[style] WebIDL-reflecting upper-case enums reported as style violation
Created attachment 180875 [details] Patch
Interesting idea. Who is the best person to review this change?
Comment on attachment 180875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180875&action=review When are you seeing this? I thought the generated files weren't checked in so the style checker doesn't run on them. > Tools/Scripts/webkitpy/style/checkers/cpp.py:1219 > + self.webidl_enum = False Nit: I think is_webidl_enum would be clearer. > Tools/Scripts/webkitpy/style/checkers/cpp.py:2098 > + if match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number]): > + enum_state.webidl_enum = True I would probably do: enum_state.is_webidl_enum = True if match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number]) else False Also, it seems more common to compile regular expressions and store them as globals.
(In reply to comment #3) > (From update of attachment 180875 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180875&action=review > > When are you seeing this? I thought the generated files weren't checked in so the style checker doesn't run on them. This is the case with enums in WebCore, for instance in Source/WebCore/Modules/webaudio/PannerNode.h: http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/webaudio/PannerNode.h#L49 These still get checked, but are reported as violations while they're basically only reflecting IDL and are required to be upper-cased (while C++ enums are expected to be InterCaps).
Comment on attachment 180875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180875&action=review >> Tools/Scripts/webkitpy/style/checkers/cpp.py:2098 >> + enum_state.webidl_enum = True > > I would probably do: > enum_state.is_webidl_enum = True if match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number]) else False > > Also, it seems more common to compile regular expressions and store them as globals. Addressing the last line, many one-time expressions are written in-line and are then compiled and cached by match, sub, search and other similar methods. Simply justifying the current approach, I can still change it if you have a strong opinion about it.
Created attachment 181157 [details] Patch
Comment on attachment 181157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181157&action=review > Tools/Scripts/webkitpy/style/checkers/cpp.py:2097 > + enum_state.is_webidl_enum = bool(match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number])) if not enum_state.is_webidl_enum else True Nit: I would probably do: enum_state.is_webidl_enum |= bool(match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number]))
(In reply to comment #7) > (From update of attachment 181157 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181157&action=review > > > Tools/Scripts/webkitpy/style/checkers/cpp.py:2097 > > + enum_state.is_webidl_enum = bool(match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number])) if not enum_state.is_webidl_enum else True > > Nit: I would probably do: > enum_state.is_webidl_enum |= bool(match(r'\s*// Web(?:Kit)?IDL enum\s*$', clean_lines.raw_lines[line_number])) Looks better. Adjusted and committed as r138719. http://trac.webkit.org/changeset/138719