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

[Bug] Whitespace Cropping Triggers Colour Panic #42

Closed
kevinmatthes opened this issue Jul 16, 2022 · 6 comments · Fixed by #65
Closed

[Bug] Whitespace Cropping Triggers Colour Panic #42

kevinmatthes opened this issue Jul 16, 2022 · 6 comments · Fixed by #65

Comments

@kevinmatthes
Copy link
Contributor

When working on the changes I submitted with #41, I noticed an important bug.

zee is able to automatically crop obsolete whitespaces, this is, whitespaces at the end of a line without any need such as additional spaces and tab characters. When saving the current buffer, zee will enter a panic state best described with "Colour Panic": any lexemes do not need to be coloured correctly any longer and the parser will find syntax issues in lines which do not have such.

If one keeps the editor open while zee has a "Colour Panic", it might break due to it. This happened to me when editing zee/src/components/theme/base16.rs and pressing [TAB] once at the end of a line of a block comment for a field of the Base16Theme structure. To reproduce, add such an obsolete tab insertion, save, scroll completely down and up again, and repeat once. This broke my compilation after two rounds with insert_spaces_for_tab = true and theme_name = "base16-vscode-dark".

The same "Colour Panic" is achieved when only viewing this repository's README. Scroll some rounds slowly over the file and you will see that the colourisation fails. This behaviour can be reproduced not only with the VSCODE_DARK theme but also the default one. I quit the editor before it could break but the "Colour Panic" seems to be a symptom that there might be an internal bug hiding in the colourisation logic.

@mcobzarenco
Copy link
Collaborator

Thanks for the issue! I previously noticed a behaviour similar to what you describe in markdown and at the time concluded it's related to the tree sitter parser -- maybe incorrectly as that was unrelated to trimming whitespace.. While I investigate, could you see if you can reproduce the issue on this branch #29?

@kevinmatthes
Copy link
Contributor Author

With #29, head 5f51577, the following happens:

  • Markdown colouring is way more stable.
  • Cropping in zee/src/components/theme/base16.rs still breaks the editor -- but there are more cropping rounds needed. The wrong colouring is still applied after saving.

There is definitely progress regarding the stability of the colouring but whitespace cropping is still critical and should be worked on further.

@kevinmatthes
Copy link
Contributor Author

The bug originates from zee/src/syntax/parse.rs, line 163:

            assert!(tree.root_node().end_byte() <= text.len_bytes());

The application will break when:

  1. zee/src/components/theme/base16.rs is opened for editing (no other file)
  2. one tab is cropped on saving from the end of line 2
  3. one tab is cropped on saving from the end of line 668
  4. one tab is entered in line 2

@kevinmatthes
Copy link
Contributor Author

Due to the whitespace cropping, the text length expected by the assertion changes and the sole way it can react is panic. Due to the text length having changed, the Colour Panic is caused: all tokens after the respective crops are shifted regarding their colour by the amount of the cropped whitespaces.

What is this assertion relevant for? Can it be removed without causing further bugs?

@kevinmatthes
Copy link
Contributor Author

The complete solution to #59 would not only prevent the crash after whitespace cropping but also solve the colourisation failures. Where to put the update instruction for the buffer content?

@mcobzarenco
Copy link
Collaborator

I created #65 which attempts to fix this.

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 a pull request may close this issue.

2 participants