Repository navigation
Make full-path syntax mappings work on Windows - #4113
Conversation
`--map-syntax` split its argument on every `:`, so `C:/dir/*.conf:INI` was
rejected as an invalid mapping. Split on the last `:` instead.
The pattern still could not match, because `PathAbs` and `fs::canonicalize`
return verbatim paths such as `\\?\C:\dir\file.conf` on Windows. That also kept
the built-in `${HOME}` and `${XDG_CONFIG_HOME}` mappings from ever matching.
Try the path without the verbatim prefix as well.
53c5a75 to
b843b05
Compare
keith-hall
left a comment
keith-hall
left a comment
There was a problem hiding this comment.
Hey, thanks for creating this PR and raising awareness of the problem - I definitely agree with the goal to improve Windows compatibility. I wonder if stripping those prefixes is the best way to do it though - it means that mappings containing those verbatim paths wouldn't be matched... Is there perhaps some other way we could do it, to support globs etc with and without verbatim path prefixes, for maximum compatibility?
I was also considering asking to gate the colon splitting change to just Windows but realised it wouldn't allow us to include mappings for Windows by default (unless we have different mappings per platform, and shat may not play well with Git Bash for Windows etc) so that part is fine upon reflection - as long as globs/paths valid on one system don't cause problems with others but kinda get ignored I think it should be okay.
|
Thanks for looking! It does not replace the path, it tries both:
So nothing that matched stops matching. The one gap is the fourth row: a pattern with the prefix doesn't match a path without it, same as before. I don't think that comes up for These went through And yes, the colon split isn't gated on Windows: |
keith-hall
left a comment
keith-hall
left a comment
There was a problem hiding this comment.
Looks good, thanks for the explanation and additional test case
--helpsays--map-syntaxglobs match the full path, but that doesn't work on Windows:--map-syntax "C:/projects/*.conf:INI"fails with "Invalid syntax mapping", because the argument is split on every:. It now splits on the last one.PathAbsandfs::canonicalizegive\?\C:\...on Windows. The built-in${HOME}/.config/git/configstyle mappings never matched for the same reason. The path is now also tried without that prefix.The new tests fail without the change and pass with it on Windows 11. I haven't run them on Linux or macOS.