[webkit-reviews] review granted: [Bug 97602] C++ style checker should warn when the indentation is wrong : [Attachment 165672] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 97602: C++ style checker should warn when the indentation is wrong
https://bugs.webkit.org/show_bug.cgi?id=97602

Attachment 165672: Patch
https://bugs.webkit.org/attachment.cgi?id=165672&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=165672&action=review


> Tools/Scripts/webkitpy/style/checkers/cpp.py:2072
> +	   error(line_number, 'whitespace/indent', 3,
> +		 'Weird number of spaces at line-start.  '
> +		 'Are you using a 4-space indent?')

Nit: put this string on one line? and maybe put all the arguments on one line?
Also, isn't the violating the new style rule?

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2081
> +    previous_line_initial_spaces = 0
> +    while previous_line_initial_spaces < len(previous_line) and
previous_line[previous_line_initial_spaces] == ' ':
> +	   previous_line_initial_spaces += 1

Nit: make a helper function for this?

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2084
> +	   error(line_number, 'whitespace/indent', 3,
> +		 'When wrapping a line, only indent 4 spaces.')

Nit: put this on one line?

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2573
>      # if(match($0, " <<")) complain = 0;
>      # if(match(prev, " +for \\(")) complain = 0;
>      # if(prevodd && match(prevprev, " +for \\(")) complain = 0;

This comment seems way outdated (and related to this patch). Delete them?