[webkit-reviews] review granted: [Bug 34173] style tool: Improve treatment of conditions and rest of the line for if, else, switch and alikes : [Attachment 47541] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 27 09:18:42 PST 2010


Shinichiro Hamaji <hamaji at chromium.org> has granted anton muhin
<antonm at chromium.org>'s request for review:
Bug 34173: style tool: Improve treatment of conditions and rest of the line for
if, else, switch and alikes
https://bugs.webkit.org/show_bug.cgi?id=34173

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

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
Thanks for fixes! Please feel free to land this patch if you agree with my
comments and fix them.

> +def up_to_unmatched_closing_paren(s):
> +    """Splits a string into two parts up to first unmatched ) (assuming
> +    that |s| is a rest right after (.
> +
> +    Returns None, None if there is no unmatched )
> +    """

Please follow conventions of docstrings. It should be like

"""Splits a string into two parts up to first unmatched ')'.

Args:
  s: a string which is a substring of line after "(".
     (e.g., "a == (b + c)) {").

Returns:
  A pair of strings (e.g., "a == (b + c))" and " {").

"""

or something like this. It would be nicer if the argument s has a better name,
I couldn't come up with one though.

> +    in_macro = match(r'\s*#define', line)

I think we can move this line to just before the check of single line
statements (line 1350).


More information about the webkit-reviews mailing list