Summary: | Rename AtomicString to AtomString | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||
Component: | Web Template Framework | Assignee: | Darin Adler <darin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, commit-queue, darin, dbates, ews-watchlist, mcatanzaro, mjs, sam, simon.fraser, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
Michael Catanzaro
2019-03-04 07:10:56 PST
We’ll do it with the do-webcore-rename script, which makes it really easy. Although we might need to do some trial runs and fix bugs in the script. I’ve done similarly-broad giant renames with that script before. (In reply to Darin Adler from comment #2) > Although we might need to do some trial runs and fix bugs in the script. > > I’ve done similarly-broad giant renames with that script before. Cool script. Note its output when not given anything new to rename: diff --git a/Source/WTF/wtf/Expected.h b/Source/WTF/wtf/Expected.h index e9b0ec9f9e6..b64f69ecdab 100644 --- a/Source/WTF/wtf/Expected.h +++ b/Source/WTF/wtf/Expected.h @@ -214,7 +214,7 @@ template<class E> class bad_expected_access : public bad_expected_access<void> { public: explicit bad_expected_access(E val) : val(val) { } - virtual const char* what() const noexcept override { return std::exception::what(); } + const char* what() const noexcept override { return std::exception::what(); } E& error() & { return val; } const E& error() const& { return val; } E&& error() && { return std::move(val); } (A code style error.) diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h index 94037ce8e29..369d2344d98 100644 --- a/Source/WTF/wtf/Platform.h +++ b/Source/WTF/wtf/Platform.h @@ -353,7 +353,7 @@ #endif /* ==== OS() - underlying operating system; only to be used for mandated low-level services like - virtual memory, not to choose a GUI toolkit ==== */ + memory, not to choose a GUI toolkit ==== */ /* OS(AIX) - AIX */ #ifdef _AIX (A do-webcore-rename issue.) It removed the word "virtual"? One tricky, but maybe obvious, thing about do-webcore-rename -- it usually has the "old renaming" list checked in, so you have to delete that and replace it with the new set of things to rename. Created attachment 372205 [details]
Patch
Note for anyone reviewing the patch: All the changes were done by the do-webcore-rename script, except for the changes to do-webcore-rename itself, and the changes to AtomString.h to add a temporary AtomicString synonym. Still worth looking over the changes to see if you can spot any mistakes, but it's not like a person made a choice about each change. Accordingly, I didn't try to fix the style mistakes mentioned by the style checker in this patch. Comment on attachment 372205 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372205&action=review > Tools/Scripts/do-webcore-rename:270 > + $newContents =~ s/\b$from\b/$renames{$from}/g; # this " unconfuses Xcode syntax highlighting I can take out that comment, since it doesn’t apply any more. Attachment 372205 [details] did not pass style-queue:
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:43: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:44: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:68: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:69: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/dom/QualifiedName.cpp:29: l is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4]
ERROR: Source/WTF/wtf/text/AtomStringHash.h:31: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WTF/wtf/text/AtomStringHash.h:36: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/AtomString.cpp:28: You should not add a blank line before implementation file's own header. [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/AtomString.cpp:36: Consider using ASSERT_EQ instead of ASSERT_TRUE(a == b) [readability/check] [2]
ERROR: Source/WebCore/dom/Event.h:147: The parameter name "isTrusted" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/text/AtomString.cpp:29: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WTF/wtf/text/AtomString.h:79: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/text/AtomString.h:80: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/text/AtomString.h:85: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WTF/wtf/text/AtomString.h:100: Omit int when using unsigned [runtime/unsigned] [1]
ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLType.h:43: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WTF/wtf/VectorTraits.h:31: Code inside a namespace should not be indented. [whitespace/indent] [4]
ERROR: Source/WebCore/dom/PointerEvent.h:128: The parameter name "isCancelable" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/xml/XPathStep.h:58: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/xml/XPathStep.h:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebCore/dom/TouchEvent.cpp:59: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/WebCore/dom/SpaceSplitString.cpp:161: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
Total errors found: 23 in 1257 files
If any of these errors are false positives, please file a bug against check-webkit-style.
I think this is ready to land! Review, anyone? Maybe someone needs to re-create the patch using Subversion directly so the renames work correctly? Comment on attachment 372205 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372205&action=review Thanks Darin! I certainly haven't looked at the whole diff, but it passes my sanity check. I suspect the Bugzilla review tool just isn't smart enough to notice renames? Renames generally appear as renames on trac. > Source/WTF/wtf/text/AtomString.h:180 > +using AtomicString = AtomString; This deserves a FIXME to remove it when Apple is able to do that, right? (In reply to Michael Catanzaro from comment #12) > This deserves a FIXME to remove it when Apple is able to do that, right? Sure, but that's going to be really soon. (In reply to Michael Catanzaro from comment #12) > I suspect the Bugzilla review tool just isn't smart enough to notice > renames? Renames generally appear as renames on trac. At one time, the review tool did the right thing when passed a patch generated by Subversion but not a patch generated but git. I don’t know about what will or won’t work when actually applying the patch. Comment on attachment 372205 [details] Patch Clearing flags on attachment: 372205 Committed r246490: <https://trac.webkit.org/changeset/246490> All reviewed patches have been landed. Closing bug. |