-
Notifications
You must be signed in to change notification settings - Fork 336
SPICE-0019: Allow trailing commas in comma-separated syntax elements #1137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
if (lookahead == terminator) { | ||
return res; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra check is needed because some callers parse the element of the list themselves (while determining what construct it is) so it's possible for this function to be reached while at terminator
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits, but overall this looks good!
Would also like @stackoverflow to take a look and review the parser changes.
Do you also mind writing a quick SPICE documenting the changes here?
pkl-core/src/test/files/LanguageSnippetTests/input/classes/constraintsTrailingComma.pkl
Show resolved
Hide resolved
pkl-core/src/test/files/LanguageSnippetTests/input/lambdas/lambdaTrailingCommas.pkl
Outdated
Show resolved
Hide resolved
pkl-core/src/test/files/LanguageSnippetTests/input/types/trailingCommas.pkl
Outdated
Show resolved
Hide resolved
pkl-core/src/test/files/LanguageSnippetTests/input/types/trailingCommas.pkl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This is a nice improvement, thanks!
Implements apple/pkl-evolution#21
This changes Pkl's grammar but is not breaking. It only admits inputs that were previously invalid.
Covers these syntax elements:
parser/methodTrailingCommas.pkl
parser/methodTrailingCommas.pkl
parser/trailingCommas.pkl
parser/trailingCommas.pkl
parser/constraintsTrailingComma.pkl
parser/lambdaTrailingCommas.pkl
parser/lambdaTrailingCommas.pkl
Resolves #1132