Skip to content
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

fix: keywords not being parsed as named arguments #217

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

calebdw
Copy link
Collaborator

@calebdw calebdw commented Jan 29, 2024

Checklist

  • All tests pass in CI
  • There are enough tests for the new fix/feature
  • Grammar rules have not been renamed unless absolutely necessary (0 rules renamed)
  • The conflicts section hasn't grown too much (0 new conflicts)
  • The parser size hasn't grown too much (master: 2699, PR: 2706)
    (check the value of STATE_COUNT in src/parser.c)

@amaanq, @cfroystad

I'm not particularly fond of this solution, but I couldn't find a more elegant way to solve the problem...do you have any ideas?

The problem stems from the fact that the argument name is optional (if you make the argument name required then this issue goes away), a parse error is generated when an argument name clashes with any expression that starts with a keyword. I suspect that this is due to the keyword extraction, but I've tried adjusting the precedence to no avail.

Closes #161

@calebdw calebdw force-pushed the array_named_param branch 2 times, most recently from 712aa5c to b1d0ba7 Compare February 1, 2024 15:13
@amaanq
Copy link
Member

amaanq commented Feb 1, 2024

The reserved rule PR upstream would help with this and so many other cases downstream, I'm fine with this if you're okay with this, but it might be better to extract the literals out of boolean and null rather than reuse the rule, up to you though

@calebdw
Copy link
Collaborator Author

calebdw commented Feb 1, 2024

@amaanq, thanks I just updated, is that what you had in mind?

Also, any idea when that reserved keyword PR will land? Looks like it's been in the works for a while...

@calebdw calebdw merged commit 040079e into tree-sitter:master Feb 2, 2024
4 checks passed
@calebdw calebdw deleted the array_named_param branch February 2, 2024 15:01
@amaanq
Copy link
Member

amaanq commented Feb 2, 2024

yeah that looks good, and I really have wanted to spend some time to work out a few issues with that PR for a while now, let's hope sometime this spring/summer once some more urgent issues upstream are cleared out :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

array as a named parameter causes error
2 participants