[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