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

match pairs which don't form a standalone TS node #7242

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

pascalkuthe
Copy link
Member

Closes #3357
Closes #5177

These issues where caused by us requiring pairs to form a standalone TS node. This change looks at the sibling nodes (so still within the same node) to also find pairs formed by siblings. Syntax trees are usually much wider than deep, so I introduced a fairy aggressive limit on the number of examined nodes. Usually pairs form immediate siblings and looking at additional siblings is just done to handle weird grammars (and comments). The limit does lead to missing some matches but those cases are so rare that it's worth the improved performance IMO.

@pascalkuthe pascalkuthe added C-enhancement Category: Improvements A-tree-sitter Area: Tree-sitter E-medium Call for participation: Experience needed to fix: Medium / intermediate A-core Area: Helix core improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Jun 5, 2023
@pascalkuthe pascalkuthe force-pushed the better_pairs branch 2 times, most recently from ff90c6f to f5a3f99 Compare June 6, 2023 00:04
@pascalkuthe
Copy link
Member Author

also addresses #3357 (comment) (which is an unrelated bug but a small fix that makes sense to include here)

I also did wind up including a (very limited) plaintext fallback to handle text-only nodes (c preprocessor nodes for example #3357 (comment)) but to not break existing workflows this only works when absolutely no other TS match it found (and only on the brackets, and within the first named parent TS node ). I am still not sure if that's really worth having since it will case brackets inside literals to be highlighted and that feels weird to me.

helix-core/src/match_brackets.rs Outdated Show resolved Hide resolved
helix-core/src/match_brackets.rs Show resolved Hide resolved
helix-core/src/match_brackets.rs Show resolved Hide resolved
dead10ck
dead10ck previously approved these changes Jun 6, 2023
Copy link
Member

@dead10ck dead10ck left a comment

Choose a reason for hiding this comment

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

Ok this looks good to me, thanks!

alevinval
alevinval previously approved these changes Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements A-tree-sitter Area: Tree-sitter C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
4 participants