Bug 34787 - Allow underscored identifiers in CSSParser.cpp
Summary: Allow underscored identifiers in CSSParser.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuzo Fujishima
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-10 01:27 PST by Yuzo Fujishima
Modified: 2010-02-11 21:12 PST (History)
4 users (show)

See Also:


Attachments
Allow underscored identifiers in CSSParser.cpp (1.39 KB, patch)
2010-02-10 01:34 PST, Yuzo Fujishima
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuzo Fujishima 2010-02-10 01:27:20 PST
Flex (http://flex.sourceforge.net/) uses identifiers named as yy_*.
WebCore/css/CSSParser.cpp needs to handle some such identifiers.

We should relax the style rule for the file to allow underscored identifiers.
Comment 1 Yuzo Fujishima 2010-02-10 01:34:38 PST
Created attachment 48474 [details]
Allow underscored identifiers in CSSParser.cpp
Comment 2 Darin Adler 2010-02-10 10:24:45 PST
Comment on attachment 48474 [details]
Allow underscored identifiers in CSSParser.cpp

Seems unfortunate to allow any underscored names in those files when only the yy ones need to be used.
Comment 3 Yuzo Fujishima 2010-02-11 17:29:38 PST
Committed r54690: <http://trac.webkit.org/changeset/54690>
Comment 4 Shinichiro Hamaji 2010-02-11 20:28:24 PST
(In reply to comment #2)
> (From update of attachment 48474 [details])
> Seems unfortunate to allow any underscored names in those files when only the
> yy ones need to be used.

Yeah. I think adding an exception like following would be better (or you can even check if the filename is CSSParser.cpp here). What do you guys think?

--- a/WebKitTools/Scripts/webkitpy/style/processors/cpp.py
+++ b/WebKitTools/Scripts/webkitpy/style/processors/cpp.py
@@ -2482,6 +2482,7 @@ def check_identifier_name_in_declaration(filename, line_nu
mber, line, error):
             # Various exceptions to the rule: JavaScript op codes functions, co
nst_iterator.
             if (not (filename.find('JavaScriptCore') >= 0 and modified_identifi
er.find('_op_') >= 0)
                 and not modified_identifier.startswith('tst_')
+                and not modified_identifier.startswith('yy_')
                 and not modified_identifier.startswith('webkit_dom_object_')
                 and not modified_identifier.startswith('qt_')
                 and not modified_identifier.find('::qt_') >= 0
Comment 5 Yuzo Fujishima 2010-02-11 21:12:09 PST
On second though, I should have just marked the lines using yy_* with // NOLINT.

As to whether to change the rule, I think:
- only very common names (e.g., m_* and s_*?) should be exempted
   from checking in cpp.py
- file/path name based exemption should be done in one place, i.e., in
   checker.py, Checking file/path in multiple places will lead to confusion.
- It'd be nice if NOLINT mechanism is extended such that
    - we can disable subset of tests rather than the whole, e.g.,
       NOLINT:readability/naming
    - we can specify the range of NOLINT rather than put NOLINT all the lines, e.g.,
       NOLINT_START, NOLINT_END