Bug 165561

Summary:

WebCore::Pasteboard Two separate functions would be clearer than one function with an argument.

Product:

WebKit

Reporter:

Karim <karim>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

darin, karim, mjs

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	mjs: review-

Karim

Reported 2016-12-07 16:17:48 PST

Two separate functions would be clearer than one function with an argument.

Attachments
Patch (8.36 KB, patch) 2016-12-07 16:58 PST, Karim	no flags	Details Formatted Diff Diff
Patch (12.10 KB, patch) 2017-02-09 17:50 PST, Karim	no flags	Details Formatted Diff Diff
Patch (13.09 KB, patch) 2017-02-11 23:11 PST, Karim	no flags	Details Formatted Diff Diff
Patch (11.90 KB, patch) 2017-06-05 13:32 PDT, Karim	mjs: review-	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Karim

Comment 1 2016-12-07 16:58:19 PST

Created attachment 296443 [details] Patch

Darin Adler

Comment 2 2016-12-22 09:51:21 PST

Comment on attachment 296443 [details] Patch Need to fix GTK, EFL, and Windows builds. Not sure that writePlainTextSmartReplace is the ideal name. Need to think about how to say this in normal colloquial language. Maybe writePlainTextWithSmartPasteFlag.

Darin Adler

Comment 3 2016-12-22 09:51:39 PST

Thanks for tackling this.

Karim

Comment 4 2017-02-09 17:50:43 PST

Created attachment 301115 [details] Patch

Michael Catanzaro

Comment 5 2017-02-11 20:09:37 PST

Comment on attachment 301115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301115&action=review Thanks for working on refactorings! I think I would name this function "writePlainTextWithSmartReplace". When you upload your next patch, be sure to set the cq? Bugzilla flag to request the patch be committed. > Source/WebCore/ChangeLog:4 > + Created a second function that handle smart replaceing > + and edited the original one to handle regular replacing only. Again, this should be the title of the bug. Your explanation is good, but move it down below, between the "Reviewed by" line and the "No new tests" line.

Karim

Comment 6 2017-02-11 23:11:16 PST

Created attachment 301292 [details] Patch

Karim

Comment 7 2017-06-05 13:32:50 PDT

Created attachment 312027 [details] Patch

Maciej Stachowiak

Comment 8 2020-05-30 19:25:47 PDT

Comment on attachment 312027 [details] Patch This seems like a good refactoring. Unfortunately the patch no longer applies. Please post an updated version.

Note You need to log in before you can comment on or make changes to this bug.