-
Notifications
You must be signed in to change notification settings - Fork 34
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
Updated the grammar rules to allow for optional trailing commas. #1510
Conversation
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.
We should add a test for this, such as adding some usages of all new forms of trailing commas to SuperSpec.qnt
- and then updating SuperSpec.json
by running npm run update-fixtures
min
function in superspec.map.json
to demonstrate this change while keeping the add
function unchanged for reference. - Added an import statement: import Proto(N = N,) as Inst1
. ### Details - The min
function now accepts parameters in the following formats: - (x: int, y: int,)
- (x: int, y: int)
- The import statement is included to utilize the Proto
module with an optional trailing comma. ### Reason for Changes - Allowing an optional trailing comma makes the syntax more flexible and consistent with common programming practices. ### Test Cases - Added test cases for both scenarios in superspec.map.json
to ensure correct parsing of functions with optional trailing commas.
min
function in superspec.map.json
to demonstrate this change while keeping the add
function unchanged for reference. - Added an import statement: import Proto(N = N,) as Inst1
. ### Details - The min
function now accepts parameters in the following formats: - (x: int, y: int,)
- (x: int, y: int)
- The import statement is included to utilize the Proto
module with an optional trailing comma. ### Reason for Changes - Allowing an optional trailing comma makes the syntax more flexible and consistent with common programming practices. ### Test Cases - Added test cases for both scenarios in superspec.map.json
to ensure correct parsing of functions with optional trailing commas.
Summary of Changes
Details
Reason for Changes
Test Cases
|
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 great! Before merging it, can you add "Closes #1383" to the PR description, so the issue gets automatically closed when we merge this?
Actually, I remembered something else: we should add an entry to CHANGELOG.md saying that we now allow trailing commas in operator definitions and constant initialization - which also makes me think we might have missed one case: operator call. We should be able to call |
CHANGELOG.md
Outdated
|
||
- Updated grammar rule to allow an optional trailing comma in parameter lists: | ||
|
||
- Function calls |
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.
- Function calls | |
- Operator calls |
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 great! I've just merged another PR that touches the grammar, so you'll need to re-generate the files once merging main on this - sorry about that! Otherwise, we're good to go ✨
Co-authored-by: Gabriela Moreira <[email protected]>
CHANGELOG.md
for any new functionalityREADME.md
updated for any listed functionality