[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