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

Filetype autocompletion suggests wrong filetypes #3215

Closed
dmaluka opened this issue Mar 24, 2024 · 3 comments · Fixed by #3218
Closed

Filetype autocompletion suggests wrong filetypes #3215

dmaluka opened this issue Mar 24, 2024 · 3 comments · Fixed by #3218

Comments

@dmaluka
Copy link
Collaborator

dmaluka commented Mar 24, 2024

When typing set filetype and pressing Tab to autocomplete, some of the suggested filetypes are incorrect (there is no syntax highlighting for them). For example, sh is suggested, but the actual filetype for shell scripts is shell, not sh. So the user executes set filetype sh, and as a result, the syntax is not highlighted at all.

The reason is that the autocompleter simply returns the basenames of syntax *.yaml files, but some of those file names don't match filetype values inside those files. Here is the complete list of such non-matching filetypes:

% for f in runtime/syntax/*.yaml; do fname=$(basename $f | sed 's/\.yaml//'); ftype=$(awk '/filetype:/ {print $2}' <$f); [ "$fname" = "$ftype" ] || grep -H filetype $f; done
runtime/syntax/arduino.yaml:filetype: ino
runtime/syntax/ats.yaml:filetype: ATS
runtime/syntax/bat.yaml:filetype: batch
runtime/syntax/cpp.yaml:filetype: c++
runtime/syntax/csx.yaml:filetype: csharp-script
runtime/syntax/gentoo-ebuild.yaml:filetype: ebuild
runtime/syntax/gentoo-etc-portage.yaml:filetype: etc-portage
runtime/syntax/justfile.yaml:filetype: 'justfile'
runtime/syntax/kvlang.yaml:filetype: "kvlang"
runtime/syntax/mpdconf.yaml:filetype: mpd
runtime/syntax/objc.yaml:filetype: objective-c
runtime/syntax/pkg-config.yaml:filetype: pc
runtime/syntax/PowerShell.yaml:filetype: powershell
runtime/syntax/python3.yaml:filetype: python
runtime/syntax/reST.yaml:filetype: rst
runtime/syntax/sh.yaml:filetype: shell
runtime/syntax/sls.yaml:filetype: salt

Commit hash: c2c2b2a
OS: any
Terminal: any

@dmaluka dmaluka changed the title Filetype autocompletion suggests incorrect filetypes Filetype autocompletion suggests wrong filetypes Mar 24, 2024
@JoeKar
Copy link
Collaborator

JoeKar commented Mar 25, 2024

Damn, it was me (again) with #3090.

We've different approaches to solve this (by accessing the *.FileType):

  • searching and parsing the config.RTSyntax files
    • expensive, since we've to parse the header from the *.yaml first
  • searching and parsing the config.RTSyntaxHeader files
    • less expensive, since we can parse the already created *.hdr faster
    • we need to create the *.hdr of the user custom files in the moment we add them to the runtime files
  • storing all the available filetypes in the moment we call UpdateRules() into a list, which can be accessed later on easily via the buffer
    • we don't need to parse the files again, since we already did this
    • negative side effect: we store it per buffer (maybe better global in the highlighter/parser?)

The last one seems to be the most obvious to me.

@dmaluka
Copy link
Collaborator Author

dmaluka commented Mar 25, 2024

There is also another approach, which would prefer the most, if we lived in an ideal world: align all *.yaml filenames with their filetype values, so we can match just by *.yaml filenames. Even better, throw out those filetype directives as redundant. Keep It Simple, Stupid.

But due to backward compatibility reasons etc we probably don't want to do that.

So, since we need to match by *.FileType, I think the obvious easy solution is: search and parse built-in filetypes by config.RTSyntaxHeader files (as we already have them) and user's custom filetypes by config.RTSyntax. Anything else sounds like a premature optimization at this point, IMHO.

@JoeKar
Copy link
Collaborator

JoeKar commented Mar 25, 2024

But due to backward compatibility reasons etc we probably don't want to do that.

Yep, unfortunately we (I more than you) broke more than necessary.
But yes, KISS fully hit it.

I think the obvious easy solution is: search and parse built-in filetypes by config.RTSyntaxHeader files (as we already have them) and user's custom filetypes by config.RTSyntax.

Ok, then we go with the more expensive approach and sacrifice a few more CPU cycles by parsing again.
I will prepare something to solve my premature first shot.

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 a pull request may close this issue.

2 participants