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 tree sitter spans not being aligned with text after saving #65

Merged
merged 2 commits into from
Jul 24, 2022

Conversation

mcobzarenco
Copy link
Collaborator

@mcobzarenco mcobzarenco commented Jul 23, 2022

This PR ensures that after saving a buffer, we recreate a fresh copy of the tree sitter parser.

Context: on save, we strip white-space which causes the tree sitter AST spans to not be aligned with the text any more. This is the bug reported in #42.

We are typically able to compute diffs incrementally as the buffer is edited and update the tree sitter parser accordingly. We could probably compute the diffs for the strip_trailing_whitespace function. However, in general it's expected that on save a linter or similar would be run. This makes updating the tree sitter parsers impractical.

The solution in this PR is to compute a fresh copy of the AST. This means we can run any arbitrary transformation of the text on save, without having to create a diff to update the parsers. However, the downside is that it means we need to remove the existing tree on save.

IMPORTANT: As the new tree is computed async-ly, there's (a short) period when there's no available tree. This causes some flickering on save which is not ideal.

@kevinmatthes @iainh Thank you for your investigations and patience.

Fixes #42
Based on #64

@mcobzarenco mcobzarenco self-assigned this Jul 23, 2022
@mcobzarenco mcobzarenco changed the title Remove stale AST and reparse from scratch on save (fix #42) Remove stale AST and reparse from scratch on save Jul 23, 2022
@mcobzarenco mcobzarenco changed the title Remove stale AST and reparse from scratch on save Fix tree sitter spans not being aligned with text after saving Jul 23, 2022
Copy link
Contributor

@kevinmatthes kevinmatthes left a comment

Choose a reason for hiding this comment

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

👍

@kevinmatthes
Copy link
Contributor

By the way: testing against zee/src/main.rs to trigger the assertion shows that the flickering is not visible in any case as the time during which there is no tree is sometimes to short to even visualise it. Everything works fine!

And finally, this can be also considered a visual feedback that the saving is in progress.

@iainh, when saving large files (#58), is there a tree during the saving process? If not so, we could declare this a feature to warn the users that their files might be lost when quitting without syntax highlighting.

@mcobzarenco mcobzarenco merged commit 2053b21 into master Jul 24, 2022
@mcobzarenco mcobzarenco deleted the fix-colour-panic branch July 24, 2022 11:08
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.

[Bug] Whitespace Cropping Triggers Colour Panic
2 participants