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

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has canceled David Kilzer
(:ddkilzer) <ddkilzer at webkit.org>'s request for 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 #5 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
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?

>> 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.