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

Highlighting is done for file named exactly a language file name extension (rs, py, …) #7029

Closed
antoyo opened this issue May 12, 2023 · 4 comments
Labels
C-bug Category: This is a bug

Comments

@antoyo
Copy link
Contributor

antoyo commented May 12, 2023

Summary

Hi.
Helix will enable the Rust syntax highlighting for a file named rs, the Python one for py, etc.
The config should include a leading dot to avoid this.
Thanks.

Reproduction Steps

I tried this:

  1. hx rs
  2. Type fn.

I expected this to happen:

No syntax highlighting.

Instead, this happened:

fn got highlighted because helix loads the Rust syntax highlighter.

Helix log

n/a

Platform

Linux

Terminal Emulator

alacritty 0.12.0 (5a728195)

Helix Version

helix 23.03

@antoyo antoyo added the C-bug Category: This is a bug label May 12, 2023
@antoyo antoyo changed the title Highlighting is do for file named exactly a language file name extension (rs, py, …) Highlighting is doing syntax highlighting for file named exactly a language file name extension (rs, py, …) May 12, 2023
@antoyo antoyo changed the title Highlighting is doing syntax highlighting for file named exactly a language file name extension (rs, py, …) Highlighting is done for file named exactly a language file name extension (rs, py, …) May 12, 2023
@pascalkuthe
Copy link
Member

pascalkuthe commented May 12, 2023

This is caused by this check: https://github.com/helix-editor/helix/blame/06d7f5d100fdcc99f4cdfda879898b2d488d8d7c/helix-core/src/syntax.rs#L643.

For the most part file types are already treated as extensions. We have the { suffix = "..." } syntax for matching the rest of the filename. I wonder if we should remove this check and simply convert all files that check the entire file-name (like poetry.lock for toml) to use suffix matching.

What do you think @the-mikedavis? I saw in the blame that you introduced that syntax.

I think /<filename> should always exactly match the filename right?

@the-mikedavis
Copy link
Member

There is some more discussion of this in #5869. Using suffix for full filenames could be a nice workaround to avoid introducing breaking changes or new file-type formats but it seems a little hacky to me: I would prefer having a { filename = "poetry.lock" } over using suffix

@pascalkuthe
Copy link
Member

pascalkuthe commented May 12, 2023

Ah I wasnt aware of the discussion there. From the issue title I wouldn't have looked there.

Using the filename syntax looks quite nice and is my favorite variant but would require a breaking change to the current behavior. If we want to avoid the breaking change an alternative could be to just add {extension = "rs" } instead and keep the current syntax unchanged.

It's not a huge deal but can cause weird bugs when a language uses a common filename (like res). Especially if the TS grammar isn't super robust. So I would Kean towards trying to fix this somehow.

@the-mikedavis
Copy link
Member

This was fixed in #8006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

3 participants