wildmatch: re-introduce WM_PATHNAME #7
Conversation
| // pathname converts patterns of the form "foo*.txt" to "foo/**/*.txt", thus | ||
| // in-effect allowing the "*" operator to match multiple directory levels at | ||
| // once. | ||
| func pathname(p string) string { |
There was a problem hiding this comment.
I think this function can be a bit tough to follow, so it might be worth adding a few comments
| func TestPathname(t *testing.T) { | ||
| for _, c := range []*PathnameCase{ | ||
| {Given: "*.txt", Expect: "**/*.txt"}, | ||
| {Given: "foo*.txt", Expect: "foo/**/*.txt"}, |
There was a problem hiding this comment.
MIght also be worth having a case like
{Given: "foo/*/*.txt", Expect: "foo/**/*/**/*.txt"}
There was a problem hiding this comment.
It would also be worth having a case involving a ** in the path somewhere to make sure that's properly preserved. Also, should a **/* pattern expand to **/**/*? If so, that might also be worth adding a test case for.
| @@ -69,6 +83,32 @@ const ( | |||
| escapes = "\\[]*?" | |||
| ) | |||
|
|
|||
| // pathname converts patterns of the form "foo*.txt" to "foo/**/*.txt", thus | |||
There was a problem hiding this comment.
Should foo*.txt be converted? I couldn't find any documentation on Git's Wildmatch code, but I know with a unix file glob foo*.txt would match foo.txt, fooabc.txt, etc., but not foo/abc.txt.
|
@PastelMobileSuit Thanks for the review, I appreciate you pouring through Your questions, in particular #7 (comment) made me think about whether or not this is the right approach. I can't recall seeing specific rules about when it is and is not OK to transition So, I consulted Git's If you wouldn't mind dropping another review on that instead, I think we should close this out and move discussion there. |
This pull request re-introduces the WM_PATHNAME option from Git's wildmatch.c, which treats '*' as able to match across directory boundaries, as if all
*were re-written as**/*'s.This option is already somewhat used in Git LFS, when translating from filepathfilter-style path matches to .gitattributes style matches in:
https://github.com/git-lfs/git-lfs/blob/5e601f8aa040e7e8c998c2c853dda844fa156789/filepathfilter/filepathfilter.go#L102
While implementing a .gitattributes parser over the gitobj package, I found it useful to have this option to translate patterns like
*.datinto patterns like**/*.datsuch that they could be used to match children at arbitrary depth./cc @git-lfs/core