34787 2010-02-10 01:27:20 -0800 Allow underscored identifiers in CSSParser.cpp 2010-02-11 21:12:09 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 yuzo yuzo eric hamaji hayato yuzo oldest_to_newest 189279 0 yuzo 2010-02-10 01:27:20 -0800 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. 189283 1 48474 yuzo 2010-02-10 01:34:38 -0800 Created attachment 48474 Allow underscored identifiers in CSSParser.cpp 189404 2 48474 darin 2010-02-10 10:24:45 -0800 Comment on attachment 48474 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. 189840 3 yuzo 2010-02-11 17:29:38 -0800 Committed r54690: <http://trac.webkit.org/changeset/54690> 189911 4 hamaji 2010-02-11 20:28:24 -0800 (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 189923 5 yuzo 2010-02-11 21:12:09 -0800 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 48474 2010-02-10 01:34:38 -0800 2010-02-10 10:24:44 -0800 Allow underscored identifiers in CSSParser.cpp bug-34787-20100210183436.patch text/plain 1425 yuzo ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA2NWVlMmRhLi5hNTM3NjE5IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTAtMDIt MTAgIFl1em8gRnVqaXNoaW1hICA8eXV6b0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFsbG93IHVuZGVyc2NvcmVkIGlkZW50aWZp ZXJzIGluIENTU1BhcnNlci5jcHAKKworICAgICAgICBGbGV4IChodHRwOi8vZmxleC5zb3VyY2Vm b3JnZS5uZXQvKSB1c2VzIGlkZW50aWZpZXJzIG5hbWVkIGFzIHl5XyouCisgICAgICAgIFdlYkNv cmUvY3NzL0NTU1BhcnNlci5jcHAgbmVlZHMgdG8gaGFuZGxlIHNvbWUgc3VjaCBpZGVudGlmaWVy cy4KKyAgICAgICAgV2Ugc2hvdWxkIHJlbGF4IHRoZSBzdHlsZSBydWxlIGZvciB0aGUgZmlsZSB0 byBhbGxvdyB1bmRlcnNjb3JlZCBpZGVudGlmaWVycy4KKworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzQ3ODcKKworICAgICAgICAqIFNjcmlwdHMvd2Vi a2l0cHkvc3R5bGUvY2hlY2tlci5weToKKwogMjAxMC0wMi0wOSAgQ2hhbmcgU2h1ICA8Q2hhbmcu U2h1QG5va2lhLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBMYXN6bG8gR29tYm9zLgpkaWZm IC0tZ2l0IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2VyLnB5IGIv V2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2VyLnB5CmluZGV4IDlkZTM2 MmYuLmZiZGE4Y2IgMTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvc3R5 bGUvY2hlY2tlci5weQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2No ZWNrZXIucHkKQEAgLTEwOSw2ICsxMDksOCBAQCBfUEFUSF9SVUxFU19TUEVDSUZJRVIgPSBbCiAg ICAgKFsjIFRoZSBHVEsrIEFQSXMgdXNlIEdUSysgbmFtaW5nIHN0eWxlLCB3aGljaCBpbmNsdWRl cwogICAgICAgIyBsb3dlci1jYXNlZCwgdW5kZXJzY29yZS1zZXBhcmF0ZWQgdmFsdWVzLgogICAg ICAgIldlYktpdC9ndGsvd2Via2l0LyIsCisgICAgICAjIFRoZXJlIGlzIG5vIGNsZWFuIHdheSB0 byBhdm9pZCAieXlfKiIgbmFtZXMgdXNlZCBieSBmbGV4LgorICAgICAgIldlYkNvcmUvY3NzL0NT U1BhcnNlci5jcHAiLAogICAgICAgIyBUaGVyZSBpcyBubyBjbGVhbiB3YXkgdG8gYXZvaWQgInh4 eF9kYXRhIiBtZXRob2RzIGluc2lkZQogICAgICAgIyBRdCdzIGF1dG90ZXN0cyBzaW5jZSB0aGV5 IGFyZSBjYWxsZWQgYXV0b21hdGljYWxseSBieSB0aGUKICAgICAgICMgUXRUZXN0IG1vZHVsZS4K