[webkit-reviews] review granted: [Bug 103157] [style] Add a style-check for enum-member names : [Attachment 176022] Addressed the comments. Thanks!

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Sadrul Habib Chowdhury
<sadrul at chromium.org>'s request for review:
Bug 103157: [style] Add a style-check for enum-member names
https://bugs.webkit.org/show_bug.cgi?id=103157

Attachment 176022: Addressed the comments. Thanks!
https://bugs.webkit.org/attachment.cgi?id=176022&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176022&action=review


> Tools/Scripts/webkitpy/style/checkers/cpp.py:1222
> +	   expr_all_uppercase =
r'\s*[A-Z0-9_]+\s*(?:=\s*[0-9A-Za-z]+\s*)?,?\s*$'

I suggest we add a FIXME comment above this line to explain that this regular
expression and the regular expression for expr_enum_end only support integers
and identifiers for the value of an enumerator, but this is sufficient for our
needs at the time of writing (10/26/2012). That is, these regular expressions
don't match all constant expressions.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:1224
> +	   expr_enum_end =
r'}\s*(?:[a-zA-Z0-9]+\s*(?:=\s*[a-zA-Z0-9]+)?)?\s*;\s*'

Nit: For consistency, I suggest that we use the same ordering in characters
classes. In particular, the ordering of the second character class in
expr_enum_end ([a-zA-Z0-9]) differs from the ordering of second character class
in expr_all_uppercase ([0-9A-Za-z]). We should pick an ordering for character
classes and be consistent.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:1227
> +	       if match(r'\s*' + expr_enum_end + '$', line):

Although not necessary, you may want to consider prefixing the last string
literal in this string concatenation with 'r' to have Python treat it as a raw
string.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:1234
> +	       if match(expr_enum_start + '$', line):

Ditto.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:1237
> +		   matched = match(expr_enum_start + '(?P<members>.*)' +
expr_enum_end + '$', line)

Although not necessary, you may want to consider prefixing both sting literals
'(?P<members>.*)' and '$' with 'r' to have Python treat them as raw strings.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:3548
> +	 enum_state: A _EnumState instance which maintains an enum delcaration

Nit: delcaration => declaration