Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove let_declaration from Rust locals tracking #3212

Merged

Conversation

the-mikedavis
Copy link
Member

This fixes some edge-cases like here:

before after
before after

I don't think it ever makes sense to have a let-declaration define a local, right? We want to tag parameters as local.definition so that you can identify function/closure parameters but let-declarations always create variable bindings even when pattern matching so we can use normal highlighting for those.

@poliorcetics
Copy link
Contributor

For those looking at these and searching for the difference, look at the regex:: on the 4th line, I spent way too much time searching for it 😅

@poliorcetics
Copy link
Contributor

What is the result for later uses of the regex variable ? Does it remove some highlighting ? Could this be fixed by detecting modules first when seeing something like name:: ?

@the-mikedavis
Copy link
Member Author

What is the result for later uses of the regex variable?

Later uses have the @variable highlighting as expected. This change only removes the @variable highlighting on the regex:: instance.

Could this be fixed by detecting modules first when seeing something like name::?

Not without some changes to the syntax highlighting code. Local reference highlights are higher precedence than anything highlights from highlights.scm and you can't currently 'block' a local.reference with something like

(scoped_identifier path: (identifier) @ignore)
(identifier) @local.reference

@poliorcetics
Copy link
Contributor

Thanks for the explanation !

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Seems reasonable 👍

@archseer archseer merged commit 9ed9c3f into helix-editor:master Jul 28, 2022
@the-mikedavis the-mikedavis deleted the md-rust-locals-let-pattern branch July 28, 2022 09:57
GreasySlug pushed a commit to GreasySlug/helix that referenced this pull request Aug 2, 2022
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants