Skip to content

Commit

Permalink
globset: fix recursive suffix over matching
Browse files Browse the repository at this point in the history
Previous, 'foo/**' would match 'foo', but it shouldn't have. In this
case, not matching 'foo' is what is documented and also seems consistent
with other recursive globbing implementations (like that in zsh).

This also updates the prefix extractor to pull 'foo/' out of 'foo/**'.

Closes #1756
  • Loading branch information
zertosh authored and BurntSushi committed Jun 1, 2021
1 parent a28e664 commit 7534d51
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ Bug fixes:
Clarify that CLI invocation must always be valid, regardless of config file.
* [BUG #1741](https://github.com/BurntSushi/ripgrep/issues/1741):
Fix stdin detection when using PowerShell in UNIX environments.
* [BUG #1756](https://github.com/BurntSushi/ripgrep/pull/1756):
Fix bug where `foo/**` would match `foo`, but it shouldn't.
* [BUG #1765](https://github.com/BurntSushi/ripgrep/issues/1765):
Fix panic when `--crlf` is used in some cases.
* [BUG #1638](https://github.com/BurntSushi/ripgrep/issues/1638):
Expand Down
31 changes: 19 additions & 12 deletions crates/globset/src/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ impl Glob {
if self.opts.case_insensitive {
return None;
}
let end = match self.tokens.last() {
let (end, need_sep) = match self.tokens.last() {
Some(&Token::ZeroOrMore) => {
if self.opts.literal_separator {
// If a trailing `*` can't match a `/`, then we can't
Expand All @@ -414,9 +414,10 @@ impl Glob {
// literal prefix.
return None;
}
self.tokens.len() - 1
(self.tokens.len() - 1, false)
}
_ => self.tokens.len(),
Some(&Token::RecursiveSuffix) => (self.tokens.len() - 1, true),
_ => (self.tokens.len(), false),
};
let mut lit = String::new();
for t in &self.tokens[0..end] {
Expand All @@ -425,6 +426,9 @@ impl Glob {
_ => return None,
}
}
if need_sep {
lit.push('/');
}
if lit.is_empty() {
None
} else {
Expand Down Expand Up @@ -683,7 +687,7 @@ impl Tokens {
re.push_str("(?:/?|.*/)");
}
Token::RecursiveSuffix => {
re.push_str("(?:/?|/.*)");
re.push_str("/.*");
}
Token::RecursiveZeroOrMore => {
re.push_str("(?:/|/.*/)");
Expand Down Expand Up @@ -1222,9 +1226,9 @@ mod tests {
toregex!(re16, "**/**/*", r"^(?:/?|.*/).*$");
toregex!(re17, "**/**/**", r"^.*$");
toregex!(re18, "**/**/**/*", r"^(?:/?|.*/).*$");
toregex!(re19, "a/**", r"^a(?:/?|/.*)$");
toregex!(re20, "a/**/**", r"^a(?:/?|/.*)$");
toregex!(re21, "a/**/**/**", r"^a(?:/?|/.*)$");
toregex!(re19, "a/**", r"^a/.*$");
toregex!(re20, "a/**/**", r"^a/.*$");
toregex!(re21, "a/**/**/**", r"^a/.*$");
toregex!(re22, "a/**/b", r"^a(?:/|/.*/)b$");
toregex!(re23, "a/**/**/b", r"^a(?:/|/.*/)b$");
toregex!(re24, "a/**/**/**/b", r"^a(?:/|/.*/)b$");
Expand Down Expand Up @@ -1270,11 +1274,12 @@ mod tests {
matches!(matchrec18, "/**/test", "/test");
matches!(matchrec19, "**/.*", ".abc");
matches!(matchrec20, "**/.*", "abc/.abc");
matches!(matchrec21, ".*/**", ".abc");
matches!(matchrec21, "**/foo/bar", "foo/bar");
matches!(matchrec22, ".*/**", ".abc/abc");
matches!(matchrec23, "foo/**", "foo");
matches!(matchrec24, "**/foo/bar", "foo/bar");
matches!(matchrec25, "some/*/needle.txt", "some/one/needle.txt");
matches!(matchrec23, "test/**", "test/");
matches!(matchrec24, "test/**", "test/one");
matches!(matchrec25, "test/**", "test/one/two");
matches!(matchrec26, "some/*/needle.txt", "some/one/needle.txt");

matches!(matchrange1, "a[0-9]b", "a0b");
matches!(matchrange2, "a[0-9]b", "a9b");
Expand Down Expand Up @@ -1400,6 +1405,8 @@ mod tests {
"some/one/two/three/needle.txt",
SLASHLIT
);
nmatches!(matchrec33, ".*/**", ".abc");
nmatches!(matchrec34, "foo/**", "foo");

macro_rules! extract {
($which:ident, $name:ident, $pat:expr, $expect:expr) => {
Expand Down Expand Up @@ -1504,7 +1511,7 @@ mod tests {
prefix!(extract_prefix1, "/foo", Some(s("/foo")));
prefix!(extract_prefix2, "/foo/*", Some(s("/foo/")));
prefix!(extract_prefix3, "**/foo", None);
prefix!(extract_prefix4, "foo/**", None);
prefix!(extract_prefix4, "foo/**", Some(s("foo/")));

suffix!(extract_suffix1, "**/foo/bar", Some((s("/foo/bar"), true)));
suffix!(extract_suffix2, "*/foo/bar", Some((s("/foo/bar"), false)));
Expand Down

0 comments on commit 7534d51

Please sign in to comment.