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: Incorrect string interpolation highlighting group #196

Merged
merged 3 commits into from
Mar 16, 2023

Conversation

ghostbuster91
Copy link
Contributor

I noticed that values from interpolated strings are not highlighted correctly:

Screenshot from 2023-03-12 22-11-18

In order to have correct hl (highlight) group assigned I had to introduce scope tracking feature from the treesitter. After that some hl tests started failing.

Since some of the changes might be controversial I breakdown them here:

  1. in self: X => self had a type hl group previously. I dunno why. I changed it to parameter

  2. in export scanUnit.scan scanUnit was a namespace but it is declared few lines above as a variable hence it should has the same hl group as its definition.

    According to the treesitter documentation:

    Good syntax highlighting helps the reader to quickly distinguish between the different types of entities in their code. Ideally, if a given entity appears in multiple places, it should be colored the same in each place.

  3. (interpolation) @none was too wide and I couldn't get it to work with the rest of the changes, so I narrowed it.

@@ -141,9 +141,9 @@ class Copier:

export scanUnit.scan
// ^ include
// ^namespace
// ^variable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice.

@@ -57,7 +57,7 @@ object Hello {
// ^ keyword
// ^ type
self: X =>
// ^type
// ^parameter
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also great.

Copy link
Collaborator

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

overall LGTM

CI is failing though:

  ✗ basics.scala
    Failure - row: 58, column: 4, expected highlight 'parameter', actual highlights: 'type'

@ghostbuster91
Copy link
Contributor Author

ghostbuster91 commented Mar 13, 2023

CI is failing though:

  ✗ basics.scala
    Failure - row: 58, column: 4, expected highlight 'parameter', actual highlights: 'type'

Indeed, that new query file had to be added in couple different places.

But the build still fails 🤔

@ckipp01
Copy link
Collaborator

ckipp01 commented Mar 13, 2023

But the build still fails 🤔

This is because we have a check to make sure our queries match the queries in nvim-treesitter just to warn us if they go out of sync. Since you're introducing more queries here that's sort of expected to fail. No worries on that. After we merge this I'll sync the queries here with those.

@ghostbuster91
Copy link
Contributor Author

ghostbuster91 commented Mar 13, 2023

@ckipp01 hmm, I modified slightly the build process and it passes now :D

@eed3si9n eed3si9n merged commit 7d348f5 into tree-sitter:master Mar 16, 2023
ckipp01 added a commit to ckipp01/nvim-treesitter that referenced this pull request Mar 27, 2023
ckipp01 added a commit to ckipp01/nvim-treesitter that referenced this pull request Mar 27, 2023
This syncs the queries that were updated in tree-sitter/tree-sitter-scala#196.

Co-authored-by: ghostbuster91 <[email protected]>
amaanq pushed a commit to ckipp01/nvim-treesitter that referenced this pull request Mar 27, 2023
This syncs the queries that were updated in tree-sitter/tree-sitter-scala#196.

Co-authored-by: ghostbuster91 <[email protected]>
amaanq pushed a commit to nvim-treesitter/nvim-treesitter that referenced this pull request Mar 27, 2023
This syncs the queries that were updated in tree-sitter/tree-sitter-scala#196.

Co-authored-by: ghostbuster91 <[email protected]>
@ghostbuster91 ghostbuster91 deleted the fix/interpolation branch June 11, 2023 11:37
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.

3 participants