[Webkit-unassigned] [Bug 40724] Check for extra spacing on a variable declaration line.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40724





--- Comment #4 from Sam Magnuson <smagnuson at netflix.com>  2010-06-17 11:06:01 PST ---
(In reply to comment #2)
> (From update of attachment 58910 [details])
> 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.

This seemed to pass for me, it even spotted a handful of existing errors :)

> - 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.
> 

Fair enough, I've done this - add added the 'else  if' to the unittest. 

> 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.
>

Ok, I added this and put it in the unit test as well.

> 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.

Agreed, changed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.