From a3fc4cdded50c370698154433600ea51dca390df Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Sat, 24 Sep 2016 16:29:14 -0400 Subject: [PATCH] Fix a bug in the translation from a gitignore pattern to a glob. We were erroneously neglecting to prefix a pattern like `foo/` with `**/` (to make `**/foo/`) because it had a slash in it. In fact, the only reason to neglect a **/ prefix is if the pattern already starts with **/, or if the pattern is absolute. Fixes #16, #49, #50, #65 --- src/gitignore.rs | 20 ++++++++++++++------ tests/tests.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/gitignore.rs b/src/gitignore.rs index 2bbebd622..6e2a37213 100644 --- a/src/gitignore.rs +++ b/src/gitignore.rs @@ -284,6 +284,7 @@ impl GitignoreBuilder { }; let mut opts = glob::MatchOptions::default(); let has_slash = line.chars().any(|c| c == '/'); + let is_absolute = line.chars().nth(0).unwrap() == '/'; // If the line starts with an escaped '!', then remove the escape. // Otherwise, if it starts with an unescaped '!', then this is a // whitelist pattern. @@ -320,14 +321,19 @@ impl GitignoreBuilder { } } // If there is a literal slash, then we note that so that globbing - // doesn't let wildcards match slashes. Otherwise, we need to let - // the pattern match anywhere, so we add a `**/` prefix to achieve - // that behavior. + // doesn't let wildcards match slashes. pat.pat = line.to_string(); if has_slash { opts.require_literal_separator = true; - } else { - pat.pat = format!("**/{}", pat.pat); + } + // If there was no leading slash, then this is a pattern that must + // match the entire path name. Otherwise, we should let it match + // anywhere, so use a **/ prefix. + if !is_absolute { + // ... but only if we don't already have a **/ prefix. + if !pat.pat.starts_with("**/") { + pat.pat = format!("**/{}", pat.pat); + } } try!(self.builder.add_with(&pat.pat, &opts)); self.patterns.push(pat); @@ -393,10 +399,12 @@ mod tests { ignored!(ig24, ROOT, "target", "grep/target"); ignored!(ig25, ROOT, "Cargo.lock", "./tabwriter-bin/Cargo.lock"); ignored!(ig26, ROOT, "/foo/bar/baz", "./foo/bar/baz"); + ignored!(ig27, ROOT, "foo/", "xyz/foo", true); + ignored!(ig28, ROOT, "src/*.rs", "src/grep/src/main.rs"); not_ignored!(ignot1, ROOT, "amonths", "months"); not_ignored!(ignot2, ROOT, "monthsa", "months"); - not_ignored!(ignot3, ROOT, "src/*.rs", "src/grep/src/main.rs"); + not_ignored!(ignot3, ROOT, "/src/*.rs", "src/grep/src/main.rs"); not_ignored!(ignot4, ROOT, "/*.c", "mozilla-sha1/sha1.c"); not_ignored!(ignot5, ROOT, "/src/*.rs", "src/grep/src/main.rs"); not_ignored!(ignot6, ROOT, "*.rs\n!src/main.rs", "src/main.rs"); diff --git a/tests/tests.rs b/tests/tests.rs index 3823cee42..925eb21dc 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -34,6 +34,18 @@ macro_rules! sherlock { }; } +macro_rules! clean { + ($name:ident, $query:expr, $path:expr, $fun:expr) => { + #[test] + fn $name() { + let wd = WorkDir::new(stringify!($name)); + let mut cmd = wd.command(); + cmd.arg($query).arg($path); + $fun(wd, cmd); + } + }; +} + sherlock!(single_file, |wd: WorkDir, mut cmd| { let lines: String = wd.stdout(&mut cmd); let expected = "\ @@ -587,6 +599,43 @@ sherlock:5:12:but Doctor Watson has to have it taken out for him and dusted, assert_eq!(lines, expected); }); +// See: https://github.com/BurntSushi/ripgrep/issues/16 +clean!(regression_16, "xyz", ".", |wd: WorkDir, mut cmd: Command| { + wd.create(".gitignore", "ghi/"); + wd.create_dir("ghi"); + wd.create_dir("def/ghi"); + wd.create("ghi/toplevel.txt", "xyz"); + wd.create("def/ghi/subdir.txt", "xyz"); + wd.assert_err(&mut cmd); +}); + +// See: https://github.com/BurntSushi/ripgrep/issues/49 +clean!(regression_49, "xyz", ".", |wd: WorkDir, mut cmd: Command| { + wd.create(".gitignore", "foo/bar"); + wd.create_dir("test/foo/bar"); + wd.create("test/foo/bar/baz", "test"); + wd.assert_err(&mut cmd); +}); + +// See: https://github.com/BurntSushi/ripgrep/issues/50 +clean!(regression_50, "xyz", ".", |wd: WorkDir, mut cmd: Command| { + wd.create(".gitignore", "XXX/YYY/"); + wd.create_dir("abc/def/XXX/YYY"); + wd.create_dir("ghi/XXX/YYY"); + wd.create("abc/def/XXX/YYY/bar", "test"); + wd.create("ghi/XXX/YYY/bar", "test"); + wd.assert_err(&mut cmd); +}); + +// See: https://github.com/BurntSushi/ripgrep/issues/65 +clean!(regression_65, "xyz", ".", |wd: WorkDir, mut cmd: Command| { + wd.create(".gitignore", "a/"); + wd.create_dir("a"); + wd.create("a/foo", "xyz"); + wd.create("a/bar", "xyz"); + wd.assert_err(&mut cmd); +}); + #[test] fn binary_nosearch() { let wd = WorkDir::new("binary_nosearch");