WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
51452
check-webkit-style shouldn't complaint about underscores in variables in objective C files.
https://bugs.webkit.org/show_bug.cgi?id=51452
Summary
check-webkit-style shouldn't complaint about underscores in variables in obje...
David Levin
Reported
2010-12-22 00:09:04 PST
See summary.
Attachments
Patch
(14.70 KB, patch)
2010-12-22 00:24 PST
,
David Levin
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
David Levin
Comment 1
2010-12-22 00:24:25 PST
Created
attachment 77194
[details]
Patch
David Levin
Comment 2
2010-12-22 00:26:55 PST
See
https://bugs.webkit.org/show_bug.cgi?id=49481#c70
David Levin
Comment 3
2010-12-22 00:28:22 PST
Perhaps, I made the check too liberal by ignoring all _ in Objective C variables. Maybe it should only allow a single leading _. (If this is so, speak up and I believe I can adjust this.)
Eric Seidel (no email)
Comment 4
2010-12-22 02:40:32 PST
I believe only leading _ is allowed for AppKit style objc.
Shinichiro Hamaji
Comment 5
2010-12-22 05:32:05 PST
Comment on
attachment 77194
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=77194&action=review
Looks good. I think it's OK to land this patch as is. If we want a stricter rule, we may be able to create another patch.
> Tools/Scripts/webkitpy/style/checkers/cpp.py:933 > + self._is_objective_c = None
I feel we want a comment here which describes we'll guess this value in is_objective_c.
David Levin
Comment 6
2010-12-22 07:35:00 PST
Comment on
attachment 77194
[details]
Patch Landed as
http://trac.webkit.org/changeset/74478
. Agreed with Shinichiro that we can make the check more strict in another patch. (It will be nice to get rid of this false positive now and I suspect the issue of underscores in variables is a less common mistake for Objective C programmers.)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug