My takeaways from working with this grammar
This is a pull request of a few fixes I had to apply while adapting this grammar for my text editor. Here's some reasoning behind each commit, from oldest to newest:
- The first commit slashes a dozen individual queries to match function parameter names into just one. This closes #199.
- Classifying assignment statements as expression statements is not only wrong (Python 3, Python 2) but also introduces an unnecessary level of indirection when querying for class attribute declarations.
as_patternalready has analiasfield. This change brings_as_patterninto line.- Free stuff. Discriminating member identifiers is necessary to ensure
lambda foo: foo == obj.fooproduces the correct result. The only other solution is to captureexpressionaslocal.referenceinstead ofidentifierbut that produces a lot of useless matches. - Self-explanatory.
Hi @digitcrusher,
I'm not a maintainer here, but want to thank you for your changes as they address issues I've also encountered with this grammar (when using it in https://mergiraf.org/). Let's hope it will get reviewed and merged eventually.
Classifying assignment statements as expression statements is not only wrong (Python 3, Python 2) but also introduces an unnecessary level of indirection when querying for class attribute declarations.
I'm wondering if this level of indirection is useful at all - I would be tempted to hide the expression_statement node completely, and just expose its child (be it assignment or something else). In our context this would be helpful for us to associate doc strings to the elements they document.
I've integrated some of your changes to this fork. The remaining changes are left as open PRs because of failing tests.
lambda parameters are now valid in contexts they previously weren't
Do you mean the parentheses? I moved them out of the parameters node into function_definition/function_definition_scope. And a lambda parameter is a subset of a normal parameter, as far as I can tell, so idk.
I don't understand the addition of the definition scopes in the 4th commit (and it wasn't explained in the PR description) - if they're solely for the locals queries, I'd revert that.
The definition scopes are indeed only there, so that functions definitions would actually exist in the scope they are defined in. I've already learned from others and experienced myself that locals queries are not without their downsides and have limited them to function parameters (which don't need these new nodes) in my own fork. I could do some commit reordering to get this into this PR, if you want it.
The member identifier changes look fine if that's what it's called in the Python spec.
The member identifiers are necessary for locals queries, but other than that they are not given special attention in the language reference and are mere workarounds around the rigidness of Treesitter's query language, which greatly simplify highlighting any kind of members. Ugly but handy.
I don't think we want to remove the node for the parameter list, which includes the parens. If you wanted to add another node for the parameters within the list, I'd be open to that.