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: multiple tree-sitter highlighting bugs #89

Merged
merged 4 commits into from
Feb 14, 2025
Merged

Fix: multiple tree-sitter highlighting bugs #89

merged 4 commits into from
Feb 14, 2025

Conversation

sminez
Copy link
Owner

@sminez sminez commented Feb 13, 2025

The precedence order for neovim (and now the tree-sitter CLI as well it seems?) is to prefer the last match found rather than the first. This moves ad over to use the same behaviour but I still need to update the existing highlight queries as there are now several places where the ordering is incorrect if we want to maintain the existing highlighting.

This should make it easier to port over existing queries from neovim but I'm currently guarding and rejecting queries that use unknown custom predicates (such as neovim's #lua-match?) as the resulting highlights are all messed up if we don't correctly implement the same logic used by whichever editor or tool the query was taken from.

The tree-sitter tests can now run in CI as I've updated the setup of parsers and TS state to support using Rust crates rather than dynamic linking.

Fixes #76 and should also address the issue encountered in #88

  • The issue in Ad panics when using custom tree-sitter queries #76 looks to have been down to passing non-raw byte offsets to byte_to_char in the TextProvider implementation for GapBuffer. This has been fixed and the method has been renamed to raw_byte_to_char to make the required arguments clearer.
  • The issue in tree-sitter and lsp for python and R #88 is down to incorrect handling of overlapping query matches being returned by the tree-sitter API. The handling I had for this was a bit naive and still essentially the first pass from when I got things originally working. I've improved things now and added some test cases that use the tree_sitter_rust and tree_sitter_python crates to allow these tests to run without requiring on-disk .so files.

Still left to do

  • update existing queries to fix precedence where it is now broken
  • add a documentation page in the repo for writing and contributing tree-sitter queries, along with known gotchas and references
  • add further testing and checks for malformed tokens that could still lead to the crashes reported in Ad panics when using custom tree-sitter queries #76

The precedence order for neovim (and now the tree-sitter CLI as well it
seems?) is to prefer the last match found rather than the first. This
moves ad over to use the same behaviour but I still need to update the
existing highlight queries as there are now several places where the
ordering is incorrect if we want to maintain the existing highlighting.

This should make it easier to port over existing queries from neovim
but I'm currently guarding and rejecting queries that use unknown custom
predicates (such as neovim's #lua-match) as the resulting highlights are
all messed up if we don't correctly implement the same logic used by
whichever editor or tool the query was taken from.

The tree-sitter tests can now run in CI as I've updated the setup of
parsers and TS state to support using Rust crates rather than dynamic
linking.
@sminez sminez merged commit e0311c0 into develop Feb 14, 2025
7 checks passed
@sminez sminez deleted the issue-76 branch February 14, 2025 07:13
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.

Ad panics when using custom tree-sitter queries
1 participant