discover an entirely new class of exclude matches… (#301)

…which haven't been covered by tests and which show that the current implementation isn't sufficient. It also makes clear that the match styles of git-attributes and git-ignore are quite different. It's unclear why, even though the way both algorithms work are quite different. So have to take a step back and find a way to figure it out.

Sebastian Thiel committed 4 years ago

Commit f8dd5ce1 parent fe9fb4c5 files · +67 −25Browse files

Unified · Split

5 files

git-glob/src/pattern.rs +13 −10
git-glob/tests/fixtures/generated-archives/make_baseline.tar.xz +2 −2
git-glob/tests/fixtures/make_baseline.sh +19 −2
git-glob/tests/matching/mod.rs +31 −10
git-glob/tests/wildmatch/mod.rs +2 −1

git-glob/src/pattern.rs+13−10

     ///
     /// Lastly, `case` folding can be configured as well.
     ///
-    /// Note that this method uses shortcuts to accelerate simple patterns.
+    /// Note that this method uses shortcuts to accelerate simple patterns, and is specific to **exclude** style matching.
+    /// Hence this shouldn't be used for **attribute** style patterns.
     pub fn matches_repo_relative_path<'a>(
         &self,
         path: impl Into<&'a BStr>,
         basename_start_pos: Option<usize>,
-        is_dir: bool,
+        is_dir: Option<bool>,
         case: Case,
     ) -> bool {
-        if !is_dir && self.mode.contains(pattern::Mode::MUST_BE_DIR) {
+        if let Some(is_dir) = is_dir {
-            return false;
+            if !is_dir && self.mode.contains(pattern::Mode::MUST_BE_DIR) {
+                return false;
+            }
         }
         let flags = wildmatch::Mode::NO_MATCH_SLASH_LITERAL
         );
         debug_assert!(!path.starts_with(b"/"), "input path must be relative");
-        if self.mode.contains(pattern::Mode::NO_SUB_DIR) {
+        // if self.mode.contains(pattern::Mode::NO_SUB_DIR) &&  {
-            let basename = if self.mode.contains(pattern::Mode::ABSOLUTE) {
+        if self.mode.contains(pattern::Mode::NO_SUB_DIR)
-                path
+            && !self.mode.contains(pattern::Mode::ABSOLUTE)
-            } else {
+            && !self.mode.contains(pattern::Mode::MUST_BE_DIR)
-                &path[basename_start_pos.unwrap_or_default()..]
+        {
-            };
+            let basename = &path[basename_start_pos.unwrap_or_default()..];
             self.matches(basename, flags)
         } else {
             self.matches(path, flags)

git-glob/tests/fixtures/generated-archives/make_baseline.tar.xz+2−2

@@ -1,3 +1,3 @@
1	version https://git-lfs.github.com/spec/v1	1	version https://git-lfs.github.com/spec/v1
2	oid sha256:cdade0fe7f3df3ac737130a101b077c82f9de1dcb3d59d5964e5ffd920405e8d	2	oid sha256:3f40b5dc92e78e70881a1f95748b7a1848dd59aa4a287878687f8c2a48747576
3	size 10384	3	size 10520

git-glob/tests/fixtures/make_baseline.sh+19−2

@@ -10,8 +10,10 @@ while read -r pattern value; do · 9 unmodified lines
10	echo "$pattern" > .gitignore	10	echo "$pattern" > .gitignore
11	echo "$value" \| git check-ignore -vn --stdin 2>&1 \|\| :	11	echo "$value" \| git check-ignore -vn --stdin 2>&1 \|\| :
12	done <<EOF >git-baseline.nmatch	12	done <<EOF >git-baseline.nmatch
13	*/\ XXX/\	13	/*foo bam/barfoo/baz/bam
14	*/\\ XXX/\	14	/*foo bar/bam/barfoo/baz/bam
		15	foo foobaz
		16	*/\ XXX/\\
15	/*foo bar/foo	17	/*foo bar/foo
16	/*foo bar/bazfoo	18	/*foo bar/bazfoo
17	foo*bar foo/baz/bar	19	foo*bar foo/baz/bar
@@ -71,6 +73,21 @@ while read -r pattern value; do · 53 unmodified lines
71	echo "$pattern" > .gitignore	73	echo "$pattern" > .gitignore
72	echo "$value" \| git check-ignore -vn --stdin 2>&1 \|\| :	74	echo "$value" \| git check-ignore -vn --stdin 2>&1 \|\| :
73	done <<EOF >git-baseline.match	75	done <<EOF >git-baseline.match
		76	foo foo/baz
		77	foo foo/baz/bam
		78	*foo barfoo
		79	/*foo barfoo
		80	*foo barfoo/baz/bam
		81	/*foo barfoo/baz/bam
		82	*foo bam/barfoo/baz/bam
		83	*foo bar/bam/barfoo/baz/bam
		84	foo/* foo/baz
		85	foo/* foo/baz/bam
		86	foo/ barfoo/baz
		87	foo/ barfoo/baz/bam
		88	*foo barfoo/baz
		89	/*foo/ hellofoo/bar
		90	*foo/ hellofoo/bar
74	\a a	91	\a a
75	\\\[a-z] \a	92	\\\[a-z] \a
76	\\\? \a	93	\\\? \a

git-glob/tests/matching/mod.rs+31−10

 }
 #[test]
+#[ignore]
 fn compare_baseline_with_ours() {
     let dir = git_testtools::scripted_fixture_repo_read_only("make_baseline.sh").unwrap();
     let (mut total_matches, mut total_correct, mut panics) = (0, 0, 0);
             );
             match std::panic::catch_unwind(|| {
                 let pattern = pat(pattern);
-                pattern.matches_repo_relative_path(
+                pattern.matches_repo_relative_path(value, basename_start_pos(value), None, *case)
-                    value,
-                    basename_start_pos(value),
-                    false, // TODO: does it make sense to pretend it is a dir and see what happens?
-                    *case,
-                )
             }) {
                 Ok(actual_match) => {
                     if actual_match == is_match {
     );
     let path = "hello";
     assert!(
-        !pattern.matches_repo_relative_path(path, None, false /* is-dir */, Case::Sensitive),
+        !pattern.matches_repo_relative_path(path, None, false.into() /* is-dir */, Case::Sensitive),
         "non-dirs never match a dir pattern"
     );
     assert!(
-        pattern.matches_repo_relative_path(path, None, true /* is-dir */, Case::Sensitive),
+        pattern.matches_repo_relative_path(path, None, true.into() /* is-dir */, Case::Sensitive),
         "dirs can match a dir pattern with the normal rules"
     );
 }
         "the caller checks for the negative flag and acts accordingly"
     );
 }
+#[test]
+#[ignore]
+fn names_automatically_match_entire_directories() {
+    let pattern = &pat("foo");
+    assert!(!match_file(pattern, "foobar", Case::Sensitive));
+    assert!(match_file(pattern, "foo/bar", Case::Sensitive));
+    assert!(match_file(pattern, "foo/bar/baz", Case::Sensitive));
+}
+#[test]
+#[ignore]
+fn directory_patterns_match_files_within_a_directory_as_well_like_slash_star_star() {
+    let pattern = &pat("dir/");
+    assert!(match_path(pattern, "dir/file", None, Case::Sensitive));
+    assert!(match_path(pattern, "base/dir/file", None, Case::Sensitive));
+    assert!(match_path(pattern, "base/ndir/file", None, Case::Sensitive));
+    assert!(match_path(pattern, "Dir/File", None, Case::Fold));
+    assert!(match_path(pattern, "Base/Dir/File", None, Case::Fold));
+    assert!(!match_path(pattern, "dir2/file", None, Case::Sensitive));
+    let pattern = &pat("dir/sub-dir/");
+    assert!(match_path(pattern, "dir/sub-dir/file", None, Case::Sensitive));
+    assert!(match_path(pattern, "dir/Sub-dir/File", None, Case::Fold));
+    assert!(!match_path(pattern, "dir/Sub-dir2/File", None, Case::Fold));
+}
 fn pat<'a>(pattern: impl Into<&'a BStr>) -> git_glob::Pattern {
     git_glob::Pattern::from_bytes(pattern.into()).expect("parsing works")
 }
 fn match_file<'a>(pattern: &git_glob::Pattern, path: impl Into<&'a BStr>, case: Case) -> bool {
-    match_path(pattern, path, false, case)
+    match_path(pattern, path, false.into(), case)
 }
-fn match_path<'a>(pattern: &git_glob::Pattern, path: impl Into<&'a BStr>, is_dir: bool, case: Case) -> bool {
+fn match_path<'a>(pattern: &git_glob::Pattern, path: impl Into<&'a BStr>, is_dir: Option<bool>, case: Case) -> bool {
     let path = path.into();
     pattern.matches_repo_relative_path(path, basename_start_pos(path), is_dir, case)
 }

git-glob/tests/wildmatch/mod.rs+2−1

         }
     }
+    dbg!(&failures);
     assert_eq!(failures.len(), 0);
     assert_eq!(at_least_one_panic, 0, "not a single panic in any invocation");
 }
 fn match_file_path(pattern: &git_glob::Pattern, path: &str, case: Case) -> bool {
-    pattern.matches_repo_relative_path(path, basename_of(path), false /* is_dir */, case)
+    pattern.matches_repo_relative_path(path, basename_of(path), false.into() /* is_dir */, case)
 }
 fn basename_of(path: &str) -> Option<usize> {
     path.rfind('/').map(|pos| pos + 1)

discover an entirely new class of exclude matches… (#301)

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code