Allow using path suffixes to associate language file-types

[midnightexigent]

Related:

• tree-sitter ssh client config #2443
• add tree-sitter-git-config #1426

After playing around with shellcheck, I think it's not the way to go mostly because of the path separator between linux and windows (\ vs /). Hence this PR uses globbing instead

Now it's possible to associate for example .git/config to the git-config grammar by adding the following

[[language]]
name = "git-config"
file-globs = ["*.git/config"] 

[midnightexigent]

Also, I didn't manage to get the ssh-client-config to work. Anyone has any ideas.. ?

SSH client config

[[language]]
name = "sshclientconfig"
scope = "source.sshclientconfig"
file-globs = ["*.ssh/config"] 
roots = []

[[grammar]]
name = "sshclientconfig"
source = { git = "https://github.com/metio/tree-sitter-ssh-client-config", rev = "769d7a01a2e5493b4bb5a51096c6bf4be130b024" }

[sudormrfbin]

Do we really need file globbing here? Trying to match the end of the file path to a partial path given in file-types covers most of the cases that I can think of. So something like this:

[[language]]
name = "ssh-client-config"
scope = "source.ssh-client-config"
file-types = [ ".ssh/config" ]

We will definitely lose flexibility without globbing, so dropping it might not be a good idea anyway. A suggestion is to use the current file-types key instead of introducing a new file-globs key; i.e. file-types could itself understand glob patterns.

[midnightexigent]

@sudormrfbin

I thought about those approaches before opting for globbing, here is what I ran into when considering those approaches

Trying to match the end of the file path to a partial path given in file-types covers most of the cases that I can think of. So something like this:

[[language]]
name = "ssh-client-config"
scope = "source.ssh-client-config"
file-types = [ ".ssh/config" ]

This runs into 2 problems:

• Handling of the '/' in a cross-platform way (on windows, the path of the file will be .ssh\config which won't match)
• .ssh/config is relative to what ? even if the file is opened from the home directory hx .ssh/config, the path will be its absolute path aka /home/user/.ssh/config which won't match with .ssh/config without some sort of wacky string comparison. The alternative is to do something like described in comment which matches the exact path enabling something like file-types = ["~/.ssh/config"] or file-types = ["$root/.git/config"]. But I also discarded that approach because I couldn't think of a clean way to do it in a cross-platform manner (/ vs \\)

A suggestion is to use the current file-types key instead of introducing a new file-globs key; i.e. file-types could itself understand glob patterns.

Handling each of the strings in the file-types array as a glob feels kind of awkward and might result in undesirable behavior in some cases. Handling globs separately felt more natural

[archseer]

I'm a bit wary of importing another crate for something this small too.

• Handling of the '/' in a cross-platform way (on windows, the path of the file will be .ssh\config which won't match)

We can document we expect / in these strings, then replace / with https://doc.rust-lang.org/std/path/constant.MAIN_SEPARATOR.html internally

• .ssh/config is relative to what

It doesn't have to be relative to anything. You'd take the absolute filepath and check for ends_with?

[midnightexigent]

I'm a bit wary of importing another crate for something this small too.

• Handling of the '/' in a cross-platform way (on windows, the path of the file will be .ssh\config which won't match)

We can document we expect / in these strings, then replace / with https://doc.rust-lang.org/std/path/constant.MAIN_SEPARATOR.html internally

• .ssh/config is relative to what

It doesn't have to be relative to anything. You'd take the absolute filepath and check for ends_with?

I guess that would work, the behavior sounds a bit like a subset of globbing tho. But ok, I understand your point of not wanting to add dependencies that could be easily implemented by hand.

I'll update this PR soon

[dead10ck]

It does look like all of the comments were addressed. However, I can't help but think that the behavior of the file-types option is getting confusing and hard to reason about. If I'm understanding this right, the logic of the key is now:

1. File name is an exact match
2. File extension is an exact match
3. File path contains the value as a suffix

I guess it would be alright if this were documented, but I do think that having a separate key with its own semantics would be warranted too.

[dead10ck]

I do think being explicit about the strategy would make the config a lot easier to comprehend, especially to newcomers. With this addition, I can't help but feel like you'd have to look at the source code to completely understand how the option behaves.

[the-mikedavis]

Sorry for the delay on this. This looks good as an incremental improvement to file detection. I'd like globbing in the long-run since Starlark files can also be detected by BUILD.* (like BUILD.dep1, BUILD.dep2. 🙃) but that can be tackled separately since it brings additional complexity and discussion.

As dead10ck says though, it would be a big improvement to document the precedence rules of file-types in book/src/languages.md. A blurb in after the table in the "## Language configuration" section would be ideal I think

[midnightexigent]

How about now ?

[dead10ck]

That looks good to me, thanks!

[the-mikedavis]

I think this looks great! 👍

[pickfire]

I wish there is glob for full path so it can be a prerequisite for #3249 (comment),

I stole part of this and put it there.

[pickfire]

I was doing a fallback approach on #3249 since the mechanism I originally think of can be confusing when it matches multiple records, maybe you might want to take a look at that?

[midnightexigent]

So what's blocking here ? Are we waiting for another approval or do we need to change the design/approach ?