Fix handling of "empty" language injections with Tree-sitter by maxbrunsfeld · Pull Request #21753 · atom/atom · GitHub
Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Fix handling of "empty" language injections with Tree-sitter - #21753

Merged
darangi merged 3 commits into
masterfrom
mb-fix-tree-sitter-injection
Dec 10, 2020
Merged

darangi merged 3 commits into
masterfrom
mb-fix-tree-sitter-injection

Conversation

@maxbrunsfeld

@maxbrunsfeld maxbrunsfeld commented Nov 24, 2020 •

Copy link
Copy Markdown
Contributor

Identify the Bug

Fixes #21093
Fixes #20884
Fixes #20897
Fixes atom/language-javascript#676

  1. The Rust grammar had a bug that caused poor error recovery.

  2. There was a bug would occur in documents with language injections, where the injected document is empty. Examples include:

    • An empty string assigned to a node's innerHTML property in JavaScript. Usually, we highlight the contents of these strings as HTML.
      node.innerHTML = "";
    • An empty script tag in an HTML document. Usually, we would parse the tag's contents this script tag as JavaScript:
      <div>
        <script></script>
      </div>
    • In Ruby or JavaScript, a regular expression literal whose entire contents consist of escape sequences. Usually we highlight the contents of the regular expression using regex syntax:
      s.search /\n/

The bug would cause us to accidentally skip highlighting large sections of the document.

Description of the Change

These injected syntax trees are each associated with a Marker in the LanguageMode's injectionsMarkerLayer. This PR ensures that, when a language injection is empty, its marker is destroyed.

I also upgraded to the latest version of tree-sitter-rust in order to fix the last two issues mentioned in #21093, which were. unrelated, but easy to address.

Verification Process

Create files containing all of the code examples in #21093. See that the syntax highlighting is all correct. Edit this files in a variety of ways and see that syntax highlighting stays correct.

Release Notes

Fixed a syntax highlighting bug that caused inconsistent and missing highlighting in several languages.

@sadick254

sadick254 commented Nov 24, 2020 •

Copy link
Copy Markdown
Contributor

@sadick254
sadick254 force-pushed the mb-fix-tree-sitter-injection branch from d19528a to 847e787 Compare November 24, 2020 14:54
@maxbrunsfeld

Copy link
Copy Markdown
Contributor Author

Do we need to bump tree-sitter-rust?

Yeah. I mentioned in the body that the problems described in the linked issue have two different causes. One of the problems was just a bug in tree-sitter-rust that has already been fixed. I think this was the Tree-sitter-rust commit that fixed it.

@darangi

darangi commented Dec 10, 2020

Copy link
Copy Markdown
Contributor

Thanks 🙇🏾‍♂️ @maxbrunsfeld

@darangi
darangi merged commit 30ba00a into master Dec 10, 2020
@maxbrunsfeld
maxbrunsfeld deleted the mb-fix-tree-sitter-injection branch December 10, 2020 13:59
@maxbrunsfeld

Copy link
Copy Markdown
Contributor Author

Thanks for getting this merged @darangi!

@Rua

Rua commented Jan 2, 2021

Copy link
Copy Markdown

When will this appear in stable Atom?

@odedharth

Copy link
Copy Markdown

Was this merged into 1.54.0? because this issue is not fixed in that version.

@darangi

darangi commented Jan 26, 2021 •

Copy link
Copy Markdown
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

5 participants