[Webkit-unassigned] [Bug 40724] Check for extra spacing on a variable declaration line.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 16 23:03:49 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=40724
Shinichiro Hamaji <hamaji at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #58910|review? |review-
Flag| |
--- Comment #2 from Shinichiro Hamaji <hamaji at chromium.org> 2010-06-16 23:03:49 PST ---
(From update of attachment 58910)
Thanks for this patch! This feature is very useful, but I think we can improve this.
Two general comments:
- You may have already done this, but please check your patch with existing code and check if there are new false positives. For example, I often run ./WebKitTools/Scripts/run-webkit-style WebCore/*/*.cpp | grep <the new error message> to see the change.
- Please create a patch to the newest tree. I think cpp.py was moved from processors directory to checkers directory.
WebKitTools/Scripts/webkitpy/style/processors/cpp.py:1365
+ matched = search(r'^\s*(?P<type>\w+)\s\s+(?P<name>\w+)', line)
This regexp will catch "else if" as well, and this is useful warning. So, I think we should change <type> and <name> to <token1> and <token2> or something like this.
WebKitTools/Scripts/webkitpy/style/processors/cpp.py:1365
+ matched = search(r'^\s*(?P<type>\w+)\s\s+(?P<name>\w+)', line)
I think it would be nice if we can check "int* a", "int& a", "if (", etc.
WebKitTools/Scripts/webkitpy/style/processors/cpp.py:1368
+ 'Declaration has space between %s and %s ' % (matched.group('type'), matched.group('name')) )
Having one space is OK so this sentence sounds a bit strange to me. Also, we don't need the trailing space.
So, I would say "Extra space between '%s' and '%s'" or something like this.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list