Skip to content

New Patterns #155

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

Merged
merged 8 commits into from
Mar 1, 2021
Merged

New Patterns #155

merged 8 commits into from
Mar 1, 2021

Conversation

BekaValentine
Copy link
Contributor

This PR refactors the subgrammar for parameters and assignment LHSes to be a distinct set of patterns, instead of just a set of expressions. This makes it possible to refer to all variables that are used as binders, distinct from variables used as bound values, e.g.

(pattern/identifier) will match the parameter x in (x) => x but not the use of x in the body of the arrow function, while dually (_expression/identifier) will match the use but not the parameter.

This enables more precise syntax highlighting, but also is required for various precise codenav things to work.

@BekaValentine BekaValentine marked this pull request as ready for review February 26, 2021 00:02
Copy link
Contributor

@maxbrunsfeld maxbrunsfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. It makes the distinction between bindings and references much more crisp ✨ .

I think that in some places in the tree, identifier nodes that represent references will not be in _expression nodes. In particular, member_expression and new_expression seem to use primary_expression directly.

I made a branch (based off of this one) where I remove the inline for primary_expression (and drop the leading underscore in its name). You could check it out and merge it into your PR if you think it makes sense. There are no test changes, but primary_expression should now be usable as a supertype in queries (e.g. (primary_expression/identifier).

grammar.js Outdated
alias($.array, $.array_pattern)
// alias($.object, $.object_pattern),
$.object_pattern,
// alias($.array, $.array_pattern)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's 🔥 these comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@maxbrunsfeld
Copy link
Contributor

@BekaValentine
Copy link
Contributor Author

ok, i've merged those branch changes into new_patterns

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.

2 participants