Bug 165561 - WebCore::Pasteboard Two separate functions would be clearer than one function with an argument.
Summary: WebCore::Pasteboard Two separate functions would be clearer than one function...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-07 16:17 PST by Karim
Modified: 2020-05-30 19:25 PDT (History)
3 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Karim 2016-12-07 16:17:48 PST 
Two separate functions would be clearer than one function with an argument.
Comment 1 Karim 2016-12-07 16:58:19 PST 
Created attachment 296443 [details]
Patch
Comment 2 Darin Adler 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.
Comment 3 Darin Adler 2016-12-22 09:51:39 PST 
Thanks for tackling this.
Comment 4 Karim 2017-02-09 17:50:43 PST 
Created attachment 301115 [details]
Patch
Comment 5 Michael Catanzaro 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.
Comment 6 Karim 2017-02-11 23:11:16 PST 
Created attachment 301292 [details]
Patch
Comment 7 Karim 2017-06-05 13:32:50 PDT 
Created attachment 312027 [details]
Patch
Comment 8 Maciej Stachowiak 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.