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

fix: 🚑 pattern with globstar and star fails to match #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucas-labs
Copy link

This aims to address some of the issues reported in #9.

The cases reported there now work, although there are still scenarios not covered by this fix (but, well, they're failing right now too, so I guess some is better than nothing).

I must be honest and admit that if I were to explain why this change resolves some of the reported cases, I would struggle. It made sense when I was deep in ninja mode, debugging the code and tinkering with things, but after spending a day trying to grasp the entire algorithm, I still can't quite wrap my head around it. The state transitions were perhaps too much for my modest brain, haha... So, while all tests are passing on my end, including the new ones, this would benefit from a thorough review, just to be sure.

Here are some of the cases that were failing before this change and are now passing:

assert!(glob_match("/**/*a", "/a/a"));
assert!(glob_match("**/*.js", "a/b.c/c.js"));
assert!(glob_match("**/**/*.js", "a/b.c/c.js"));
assert!(glob_match("a/**/*.d", "a/b/c.d"));
assert!(glob_match("a/**/*.d", "a/.b/c.d"));

And here are some cases that were failing before this change and would continue to fail afterward:

assert!(glob_match("**/*/**", "a/b/c"));
assert!(glob_match("**/*/c.js", "a/b/c.js"));;

@shulaoda
Copy link

@devongovett

@@ -265,7 +265,21 @@ fn glob_match_internal<'a>(
state.path_index += 1;

// If this is not a separator, lock in the previous globstar.
if c != b'/' {
if c != b'/'
&& (state.globstar.glob_index <= 0
Copy link

Choose a reason for hiding this comment

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

glob_index will never less than 0

@SyMind
Copy link

SyMind commented Jul 11, 2024

@lucas-labs I have made all cases to pass in #17 .

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 this pull request may close these issues.

None yet

3 participants