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

[New Tree-Sitter] In the Ruby grammar, "/regex/" only makes colors correct if the "/" is on the first column of the row #420

Closed
mauricioszabo opened this issue Mar 11, 2023 · 5 comments · Fixed by #677

Comments

@mauricioszabo
Copy link
Contributor

No description provided.

@mauricioszabo mauricioszabo converted this from a draft issue Mar 11, 2023
@mauricioszabo
Copy link
Contributor Author

On the newer tree-sitter grammar, the following code colorizes both / as the same:

/test/

But this one doesn't:

  /test/

@mauricioszabo mauricioszabo changed the title With a Ruby grammar, "/regex/" only makes colors correct if the "/" is on the first column of the row [New Tree-Sitter] In the Ruby grammar, "/regex/" only makes colors correct if the "/" is on the first column of the row Mar 11, 2023
@savetheclocktower
Copy link
Contributor

The problem is that detectCoveredScope is a really brittle heuristic.

The purpose behind it was the idea that whenever an injection covers a region of the buffer, its scopes should preempt the scopes from a shallower layer. In this case, the idea is that we want the regex injection layer to add scopes to the regex instead of however the Ruby grammar would scope them — save for at the very boundaries of the injection, where an attempt is made to apply both sets of scopes.

But it only detects situations where a shallower layer wants to open or close a scope at the same position as the deeper layer. This makes it quite easy for opening and closing scopes to get unbalanced and out of whack. This problem theoretically exists in the legacy TreeSitterLanguageMode as well.

Commenting out all calls to detectCoveredScope fixes this problem for me. But instead I'd like to move this behavior into an option, where it can be applied far more elegantly:

atom.grammars.addInjectionPoint('source.ruby', {
    type: 'regex',
    language() {
      return 'regex';
    },
    content(node) {
      return node;
    },
    includeChildren: true,
    coverShallowerScopes: false
  });

The coverShallowerScopes option would allow any injection point to opt into or out of this behavior, and would apply to boundaries within the entire injection region, instead of just boundaries that happen to align with one another. Here it's false because we don't want or need this behavior for regexes. If we don't want scopes from the Ruby grammar to show up in regexes, we can simply refrain from adding captures in the Ruby grammar's highlights.scm for tokens within regexes.

So I'm going to comment out any calls to detectCoveredScope until I'm convinced we actually need it for something, and if we do, I'll turn it into an option as described. I'm half-convinced we can get away with not needing this at all just by being thoughtful about how we do syntax highlighting for grammars that have injections.

@savetheclocktower
Copy link
Contributor

@mauricioszabo I think this is fixed. Can you still reproduce it?

@mauricioszabo
Copy link
Contributor Author

mauricioszabo commented Aug 17, 2023

Well, no... it actually made it worse - now both situations colorize the delimiters with different colors :(

image

@savetheclocktower
Copy link
Contributor

OK, the most recent commit in #677 should fix this. Syntax highlighting is now equivalent in appearance to that of the TextMate grammar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants