Attachment 71742 [details] patch on Bug 46096, Comment #8 had a false-positive on an #if macro definition: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/assembler/MacroAssemblerARM.cpp:59: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 1 in 2 files I believe the check-webkit-style script though this was a standard if() statement, not an #if macro.
Created attachment 75440 [details] Patch
Comment on attachment 75440 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review Otherwise seems fine. > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > + in_preprocessor_directive = match(r'\s*#', line) Seems you want a ^ in that regexp to lock it to the front of the string. char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't.
(In reply to comment #2) > (From update of attachment 75440 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review > > Otherwise seems fine. > > > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > > + in_preprocessor_directive = match(r'\s*#', line) > > Seems you want a ^ in that regexp to lock it to the front of the string. > > char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't. Good call. fwiw, re.match has an implicit ^ (http://docs.python.org/library/re.html#re.match). re.search would need the ^ I wish there was only re.search (why have another function just to mean "^" + regex_given?) but match exists, so I guess it makes sense to use it.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 75440 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review > > > > Otherwise seems fine. > > > > > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > > > + in_preprocessor_directive = match(r'\s*#', line) > > > > Seems you want a ^ in that regexp to lock it to the front of the string. > > > > char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't. > Good call. > fwiw, re.match has an implicit ^ (http://docs.python.org/library/re.html#re.match). > re.search would need the ^ > > I wish there was only re.search (why have another function just to mean "^" + regex_given?) but match exists, so I guess it makes sense to use it. But I'll add test for this though.
Created attachment 75451 [details] Patch
Comment on attachment 75451 [details] Patch looks good
Comment on attachment 75451 [details] Patch Clearing flags on attachment: 75451 Committed r73248: <http://trac.webkit.org/changeset/73248>
All reviewed patches have been landed. Closing bug.