Skip to content

Expand git-config filename pattern to include */git/config #4025

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.

Sign up for GitHub

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

Open
injust wants to merge 2 commits into
base: master
Choose a base branch
from
+1 −1
Open

Expand git-config filename pattern to include */git/config #4025

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1a7f0d
Expand git-config filename pattern to include `*/git/config`
injust Feb 26, 2026
184dd25
Match backslash too
injust Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runtime/syntax/git-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
filetype: git-config

detect:
filename: "git(config|modules)$|\\.git/config$"
filename: "git(config|modules)$|^(.*[\\/])?\\.?git[\\/]config$"
Copy link
Contributor

@Andriamanitra Andriamanitra Feb 27, 2026

Choose a reason for hiding this comment

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

I think you need more backslashes if [\\/] is supposed to match both regular and windows-style path separators...

Suggested change
filename: "git(config|modules)$|^(.*[\\/])?\\.?git[\\/]config$"
filename: "git(config|modules)$|^(.*[\\\\/])?\\.?git[\\\\/]config$"

Copy link
Contributor Author

@injust injust Feb 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

In [\\/], I'm not sure if the backslash is treated as an escape character. But what I have now is consistent with the rest of the syntax highlighting files.

But we should really change all these strings to single-quoted strings to avoid all the backslashes.

Copy link
Contributor Author

@injust injust Feb 27, 2026

Choose a reason for hiding this comment

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

https://xkcd.com/1638/

Copy link
Member

@JoeKar JoeKar Feb 27, 2026

Choose a reason for hiding this comment

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

In [\\/], I'm not sure if the backslash is treated as an escape character.

In YAML the content of "[\\/]" already results in a interpret string of [\/] used by regexp due to its presence within []. Only the representation of the same string written in Go needs to be "[\\\\/]".
So I'd assume that the current state should work.

Copy link
Contributor

@Andriamanitra Andriamanitra Feb 28, 2026

Choose a reason for hiding this comment

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

Did you test it? At least on my machine it wouldn't recognize git\config without the extra backslashes. I'm on Linux so the \ is a part of the filename rather than a path separator but I believe the path matching should work the same regardless of how the OS treats it.

The double-quoted string in yaml eats the first escape so the regex just sees [\/], which it interprets as "character group with a literal / in it" (/ doesn't require escaping but it's still treated as an escape sequence).

Copy link
Member

@JoeKar JoeKar Feb 28, 2026

Choose a reason for hiding this comment

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

Damn, looks you're right. I tried it now too...sorry for that.
This means, that the same check in git-commit.yaml, git-rebase-todo.yaml and ruby.yaml doesn't work either.

Copy link
Member

@JoeKar JoeKar Feb 28, 2026

Choose a reason for hiding this comment

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

Damn, looks you're right. I tried it now too...sorry for that. This means, that the same check in git-commit.yaml, git-rebase-todo.yaml and ruby.yaml doesn't work either.

What makes me wonder is that it didn't work the easy (single-escaped) way, when I used single-quotes for the YAML string. 🤔

Copy link
Contributor

@Andriamanitra Andriamanitra Feb 28, 2026

Choose a reason for hiding this comment

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

What makes me wonder is that it didn't work the easy (single-escaped) way, when I used single-quotes for the YAML string. 🤔

[\\/] should work with single-quoted string. Maybe you forgot to change the \\. to \.?

Copy link
Member

@JoeKar JoeKar Feb 28, 2026

Choose a reason for hiding this comment

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

Maybe you forgot to change the \\. to \.?

Exactly, this was the case. 👍

But we should really change all these strings to single-quoted strings to avoid all the backslashes.

I agree, this would really help to simplify all of the regex strings...especially the ones where we used \\\\. and all of this double backslash escaping.


rules:
- constant: "\\<(true|false)\\>"
Expand Down