Allow reserved words as record names w/out quotes #7873
Conversation
|
|
choptastic
force-pushed
the
reserved_word_records
branch
from
d2ed520
to
dddf0da
Compare
jhogberg
added
team:LG
|
I have run into this in the past. I believe I wanted a |
CT Test Results 3 files 106 suites 58m 59s ⏱️ For more details on these failures, see this check. Results for commit 539299e. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
|
I (and at least one more, we will have a meeting shortly) like this idea, and cannot really see any problems with extending the syntax like you suggest. I added 3 suggested commits:
-record #div{a :: integer(), b :: integer()}.
div() -> #div{a = 1, b = 2}.More test cases are needed, documentation, maybe use the feature feature to make it possible to test for this feature. |
RaimoNiskanen
force-pushed
the
reserved_word_records
branch
from
dc35533
to
5939061
Compare
|
Fixed some lint in the latest commit message |
RaimoNiskanen
force-pushed
the
reserved_word_records
branch
from
2a1a7ec
to
6ef200c
Compare
This motivation for this comes from the Nitrogen Web Frameworks's heavy
use of records, and a clash between HTML <div> element and the Erlang
div operator.
In Nitrogen, HTML elements are represented by Erlang records. For
example: the HTML `<span>` element is `#span{}` in Nitrogen.
Logically, the heavily-used HTML `<div>` element would be represented
by `#div{}` in Nitrogen, however, that specific syntax is illegal due to
`div`'s reserved word status, and must instead be represented with
`#'div'`. This syntax, however, is awkward, and has led to a workaround
that *works*, but is itself awkward (using the term `#panel{}` instead
of `#div{}` - but this in itself leads to a semantic clash, as some
frontend HTML frameworks have their own 'panel' elements that might
ideally be abstracted into a `#panel{}` element.
But, As far as I understand, there is no potential syntax clash in
allowing the syntax `#div` ito be acceptable, and have the parser
recognize that the `div` (or any reserved word) in that `context` can
only be an atom, and would never be an operator.
So this change tweaks the grammar to recognize the circumstances of:
`#reserved_word{}`.
Further, this change does not change the way the records are defined (so
the definition must still be defined with the atom properly wrapped in
quotes (e.g. `-record('div', {a,b}).`).
This PR also adds the appropriate tests in `erl_expand_records_SUITE`,
which I wasn't sure if that was appropriate place, but it seemed the
most relevant.
RaimoNiskanen
force-pushed
the
reserved_word_records
branch
from
6ef200c
to
f30c236
Compare
|
I rebased onto OTP-27.0 to get into Markdown documentation land |
RaimoNiskanen
force-pushed
the
reserved_word_records
branch
from
f30c236
to
d9429e7
Compare
|
@RaimoNiskanen: I just wanted to pop in here and say thanks so much for reviewing this, pushing your changes, and (hopefully) accepting it. The new intuitive If there's anything else you need from me, please let me know. Thanks! |
|
@choptastic: We had a language group meeting, and decided to accept this proposal, with my suggested record definition syntax extension, but need an EEP first since it is a language syntax change that also affect tools like Emacs, the language server platform, etc... So, since you asked ;-) can you write an EEP, please? Or at least begin? |
|
Sure, I think I can manage that! |
RaimoNiskanen
force-pushed
the
reserved_word_records
branch
from
2948084
to
539299e
Compare
|
Better late than never! I've posted the EEP to the EEP repo. Thanks! Please let me know if there are any issues. |
The motivation for this PR comes from the Nitrogen Web Frameworks's heavy use of records, and a clash between HTML
<div>element and Erlang'sdivoperator.In Nitrogen, HTML elements are represented by Erlang records. For example: the HTML
<span>element is#span{}in Nitrogen.Logically, the heavily-used HTML
<div>element would be represented by#div{}in Nitrogen, however, that specific syntax is illegal due todiv's reserved word status, and must instead be represented with#'div'. This syntax, however, is awkward, and has led to a workaround that works, but is itself awkward (using the term#panel{}instead of#div{}- but this in itself leads to a semantic clash, as some frontend HTML frameworks have their own 'panel' elements that might ideally be abstracted into a#panel{}element - but can't because#panel{}already exists).But, As far as I understand, there is no potential syntax clash in allowing the syntax
#divto be acceptable, and have the parser recognize that thediv(or any reserved word) in that context should be treated as an atom.So this change tweaks the grammar to recognize those conditions.
Further, this change does not change the way the records are defined (so the definition must still be defined with the atom properly wrapped in quotes (e.g.
-record('div', {a,b}).).This PR also adds the appropriate tests in
erl_expand_records_SUITE, which I wasn't sure if that was appropriate place, but it seemed the most relevant.If any revisions should be made, I'm certainly open to it. Or if this requires a much larger discussion than just a PR (I certainly don't think this is big enough to justify an EEP, but what do I know). In any case, this would help eliminate an odd quirk of Nitrogen since the very beginning. Thanks!