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

Improve indent queries for python #5332

Merged
merged 1 commit into from Jan 7, 2023

Conversation

Triton171
Copy link
Contributor

This is a workaround for the python tree-sitter grammar being very sensitive to incomplete code in some cases (an try_statement without an except/finally clause sometimes even prevents the surrounding function from being recognized). I simply added a few queries that improve the indentation in common situations. Since they only match ERROR nodes, this shouldn't break anything that works correctly right now.

Should fix #763 and #5045. At least for the common scenario of writing down a try_statement with a missing except_clause/finally_clause, the indentation is now correct in all cases I tested,

where the tree-sitter completely fails to parse incomplete code.
@zyklotomic
Copy link
Contributor

zyklotomic commented Dec 29, 2022

Looks good to me! It misses the case of nested-try statements, but I understand that the PR isn't supposed to handle every case.

I do have a bit of reservation about the latter two queries being a bit too ad-hoc though.

(ERROR
  .
  "def") @indent @extend
(ERROR
  (block) @indent @extend
  (#set! "scope" "all"))

Maybe we should somehow match for a "try" in there too.

@kirawi kirawi added S-waiting-on-review Status: Awaiting review from a maintainer. A-language-support Area: Support for programming/text languages labels Dec 29, 2022
@Triton171
Copy link
Contributor Author

I think there's some value in keeping the patterns as generic as possible. This way, we also handle situations which we haven't specifically thought about. Since the patterns only match ERROR nodes, it isn't very likely to break any cases that work correctly right now. If we really want to keep the behavior as similar as possible to the previous version, we could replace the second query by something like:

(ERROR
  "try"
  (ERROR
    (block) @indent @extend
    (#set! "scope" "all"))

This would definitely not break anything but it will probably miss some situations where the grammar produces a slightly different pattern. Also, it would be very sensitive to small changes in the grammar. For example, the version of the grammar in the playground only creates a single ERROR node, while in Helix there are 2 ERROR nodes in every case I tested. The first pattern should be fine as is, since an ERROR starting with the "def" keyword cannot really be anything but a function definition.

Handling nested try_statements is probably a bit more difficult since there's a lot more cases to think about. It's definitely something we could do in the future but I think it's best to first see if there are any issues with the handling of a single try_statement.

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Nice, that's a pretty cool way of working around it. Thanks!

@archseer archseer merged commit 873434b into helix-editor:master Jan 7, 2023
kirawi pushed a commit to kirawi/helix that referenced this pull request Jan 25, 2023
where the tree-sitter completely fails to parse incomplete code.
freqmod pushed a commit to freqmod/helix that referenced this pull request Feb 8, 2023
where the tree-sitter completely fails to parse incomplete code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python file indentation is wrong
4 participants