Bug 217667 - [WebIDL] Add support for identifiers with dashes ('-')
Summary: [WebIDL] Add support for identifiers with dashes ('-')
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks: 217623
  Show dependency treegraph
 
Reported: 2020-10-13 14:13 PDT by Sam Weinig
Modified: 2020-10-14 09:16 PDT (History)
6 users (show)

See Also:


Attachments
Patch (29.97 KB, patch)
2020-10-13 14:40 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (30.27 KB, patch)
2020-10-13 15:09 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2020-10-13 14:13:29 PDT
[WebIDL] Add support for identifiers with hyphens ('-')
Comment 1 Sam Weinig 2020-10-13 14:40:17 PDT
Created attachment 411258 [details]
Patch
Comment 2 Darin Adler 2020-10-13 15:00:07 PDT
Comment on attachment 411258 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Update identifer regular expression to match current spec which allows for dashes ('-').

identifer

> Source/WebCore/ChangeLog:19
> +        underscores ('__'), as that is not likely to ever appear in an identifier.

C++ reserves identifiers containing double underscores. https://en.cppreference.com/w/cpp/language/identifiers

Maybe "_dash_"?
Comment 3 Sam Weinig 2020-10-13 15:07:00 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 411258 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411258&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Update identifer regular expression to match current spec which allows for dashes ('-').
> 
> identifer
> 
> > Source/WebCore/ChangeLog:19
> > +        underscores ('__'), as that is not likely to ever appear in an identifier.
> 
> C++ reserves identifiers containing double underscores.
> https://en.cppreference.com/w/cpp/language/identifiers

Oh, interesting. I for some reason always thought it was double underscores at the beginning of an identifier, but no, it's anywhere.

> 
> Maybe "_dash_"?

That seems good. Will update. Thanks.
Comment 4 Sam Weinig 2020-10-13 15:09:45 PDT
Created attachment 411264 [details]
Patch
Comment 5 Darin Adler 2020-10-13 15:23:24 PDT
Unicode names the ASCII character "hyphen-minus". So we are being super colloquial calling it a "dash". But that word seems friendly enough.
Comment 6 Sam Weinig 2020-10-13 16:07:33 PDT
(In reply to Darin Adler from comment #5)
> Unicode names the ASCII character "hyphen-minus". So we are being super
> colloquial calling it a "dash". But that word seems friendly enough.

Sorry, a bit unclear here. Are you suggesting I change it from "_dash_" to something else?
Comment 7 Darin Adler 2020-10-13 17:06:42 PDT
No. Just musing on whether this character is a "dash" or a "hyphen", don’t really care which name we chose.
Comment 8 EWS 2020-10-13 17:14:19 PDT
Committed r268435: <https://trac.webkit.org/changeset/268435>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411264 [details].
Comment 9 Radar WebKit Bug Importer 2020-10-13 17:15:26 PDT
<rdar://problem/70272997>
Comment 10 Sam Weinig 2020-10-14 09:16:07 PDT
(In reply to Darin Adler from comment #7)
> No. Just musing on whether this character is a "dash" or a "hyphen", don’t
> really care which name we chose.

Heh. I clearly had the same musing as the test case still uses "hyphen" :). I went back and forth a few times.