[webkit-reviews] review granted: [Bug 214954] check-webkit-style should enforce acronym capitalization at start/end of an identifier : [Attachment 405543] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 30 10:27:29 PDT 2020


Jonathan Bedard <jbedard at apple.com> has granted  review:
Bug 214954: check-webkit-style should enforce acronym capitalization at
start/end of an identifier
https://bugs.webkit.org/show_bug.cgi?id=214954

Attachment 405543: Patch v1

https://bugs.webkit.org/attachment.cgi?id=405543&action=review




--- Comment #6 from Jonathan Bedard <jbedard at apple.com> ---
Comment on attachment 405543
  --> https://bugs.webkit.org/attachment.cgi?id=405543
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=405543&action=review

>> Tools/Scripts/webkitpy/style/checkers/cpp.py:1805
>> +	acronyms = '|'.join(['MIME', 'URL'])
> 
> @Jonathan: This list of acronyms could be replaced by the other list you
mentioned.  Can you tell me where the list is defined?

I was actually referring to this list here, although you explanation of the
FIXME and false-positive matches makes it more clear the cases you had in mind.

>>> Tools/Scripts/webkitpy/style/checkers/cpp.py:1820
>>> +	 # FIXME: Hard to check middle words without knowing that the word to
the left doesn't end with an acronym.
>> 
>> We already have a list of acronyms, can't we just do 'starts with' iterating
through our list?
> 
> Where is the list of acronyms?  I wasn't aware of it until you mentioned it. 
(See previous comment.)
> 
> Also, I'm not sure how a list of acronyms would help with this FIXME.  The
case I'm concerned about is something (made-up) like:
> 
>     canUnfurlFlag
>     canCurlHair
> 
> Searching the middle of a variable for "url" (in this case) might return a
false positive, which is what the comment was about.
> 
> I was thinking more about this after posting the patch, and I think a better
algorithm may be to:
> 
> - Split camelCase identifier into individual words using the start of a
capital letter followed by a lowercase letter (after the initial word which
would be all-uppercase or all-lowercase).
> - Compare each individual word in the list to a list of acronyms, and
determine whether its case is wrong whether it's first or not-first in the
list.
> 
> The algorithm wouldn't be perfect, but would probably do a better job (and be
easier to understand) than this code:
> 
> - HTMLDOMDocument => (HTMLDOM, Document)
> - canUnfurlFlag => (can, Unfurl, Flag)
> - canCurlHair => (can Curl Hair)
> - URLString => (URL, String)
> - localPathOrUrl => (local, Path, Or, Url)
> - MIMEStringForURL => (MIME, String, For, URL)
> 
> Now this algorithm assumes each word is spelled correctly, but we're not
trying to solve correct spelling of camelCase words, so that's okay.

I was referring to your above list. (seems like DOM and HTML should be in it,
though)

If we want to do the more thorough camelCase splitting, that really feels like
a different patch, since it's quite a bit more complicated then the startswith
case I suggested.


More information about the webkit-reviews mailing list