Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@maxbrunsfeld
Copy link
Contributor

@maxbrunsfeld maxbrunsfeld commented Nov 24, 2020

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
Copy link
Contributor

sadick254 commented Nov 24, 2020

Thank you for the fix. Do we need to bump tree-sitter-rust?

@sadick254 sadick254 force-pushed the mb-fix-tree-sitter-injection branch from d19528a to 847e787 Compare November 24, 2020 14:54
@maxbrunsfeld
Copy link
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
Copy link
Contributor

darangi commented Dec 10, 2020

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
Contributor Author

Thanks for getting this merged @darangi!

@Rua
Copy link

Rua commented Jan 2, 2021

When will this appear in stable Atom?

@odedharth
Copy link

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

@darangi
Copy link
Contributor

darangi commented Jan 26, 2021

@odedharth, it is yet to hit stable, but you can get it on nightly

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