Allow metadata keywords in field position #1768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Metadata keywords (optional, default, etc.) are numerous and they can't clash with field names, because they don't appear in the same positions. Thus, to avoid having user to quote their fields, this commit allows unambiguously the usage of metadata keyword in field position.
yannham
commented
Jan 19, 2024
| @@ -1,141 +1,165 @@ | |||
| # test.type = 'pass' | |||
There was a problem hiding this comment.
This file has a big diff because I formatted it with Topiary. You can ignore most of the changes but the very end, where I added a test for metadata keyword in field position.
vkleen
reviewed
Jan 19, 2024
| @@ -557,6 +557,30 @@ Match: Match = { | |||
| // A default annotation in a pattern. | |||
| DefaultAnnot: RichTerm = "?" <t: Term> => t; | |||
|
|
|||
| // A metadata keyword returned as an indent. In some positions, those are | |||
There was a problem hiding this comment.
Can we do this for all (or most) of our keywords? I think even a let or rec should be unambiguous in field name positions.
There was a problem hiding this comment.
I would vote for doing this for metadata keyword only at first. Although I don't have right now any example of an existing keyword or a future extension that could cause an issue, let's just give it a bit more thinking (metadata keywords are much more restricted in where they can appear). At worst we can do that in a follow-up PR.
vkleen
approved these changes
Jan 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1765.
Metadata keywords (optional, default, etc.) are numerous and they can't clash with field names, because they don't appear in the same positions. Thus, to avoid having user to quote their fields, this PR allows unambiguously the usage of metadata keyword in field position.
I suspect we could actually proceed with even more keywords of the language, but let's start small and avoid shooting ourselves in the foot. The metadata keywords is the set which isn't small and is likely to grow, while the other keywords should stay relatively stable.
Note that currently metadata keyword are rejected within let destucturing, because destructuring binds variables, and variables which are keywords can't be accessed in the current syntax. We might change that if we ever implement raw identifiers.