Skip to content

Commit

Permalink
Fix a bug in the translation from a gitignore pattern to a glob.
Browse files Browse the repository at this point in the history
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 BurntSushi#16, BurntSushi#49, BurntSushi#50, BurntSushi#65
  • Loading branch information
amsharma91 committed Sep 20, 2016
1 parent 92f728c commit 88ed846
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 6 deletions.
20 changes: 14 additions & 6 deletions src/gitignore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down
49 changes: 49 additions & 0 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "\
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 88ed846

Please sign in to comment.