@matheusbsbs / matheus-bagliano / commits / 7ec8125fba
Fork 0

check-ignore: fix documentation and implementation to match

check-ignore has two different modes, and neither of these modes has an implementation that matches the documentation. These modes differ in whether they just print paths or whether they also print the final pattern matched by the path. The fix is different for both modes, so I'll discuss both separately. === First (default) mode === The first mode is documented as: For each pathname given via the command-line or from a file via --stdin, check whether the file is excluded by .gitignore (or other input files to the exclude mechanism) and output the path if it is excluded. However, it fails to do this because it did not account for negated patterns. Commands other than check-ignore verify exclusion rules via calling ... -> treat_one_path() -> is_excluded() -> last_matching_pattern() while check-ignore has a call path of the form: ... -> check_ignore() -> last_matching_pattern() The fact that the latter does not include the call to is_excluded() means that it is susceptible to to messing up negated patterns (since that is the only significant thing is_excluded() adds over last_matching_pattern()). Unfortunately, we can't make it just call is_excluded(), because the same codepath is used by the verbose mode which needs to know the matched pattern in question. This brings us to... === Second (verbose) mode === The second mode, known as verbose mode, references the first in the documentation and says: Also output details about the matching pattern (if any) for each given pathname. For precedence rules within and between exclude sources, see gitignore(5). The "Also" means it will print patterns that match the exclude rules as noted for the first mode, and also print which pattern matches. Unless more information is printed than just pathname and pattern (which is not done), this definition is somewhat ill-defined and perhaps even self-contradictory for negated patterns: A path which matches a negated exclude pattern is NOT excluded and thus shouldn't be printed by the former logic, while it certainly does match one of the explicit patterns and thus should be printed by the latter logic. === Resolution == Since the second mode exists to find out which pattern matches given paths, and showing the user a pattern that begins with a '!' is sufficient for them to figure out whether the pattern is excluded, the existing behavior is desirable -- we just need to update the documentation to match the implementation (i.e. it is about printing which pattern is matched by paths, not about showing which paths are excluded). For the first or default mode, users just want to know whether a pattern is excluded. As such, the existing documentation is desirable; change the implementation to match the documented behavior. Finally, also adjust a few tests in t0008 that were caught up by this discrepancy in how negated paths were handled. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Feb 18, 2020 at 23:05 UTC 7ec8125fba96f47b00bb0cab3ed716557e81e7e6
3 files changed +35 -19
Documentation/git-check-ignore.txt
+9 -3
@@ -30,9 +30,15 @@ OPTIONS
30 valid with a single pathname.
31
32 -v, --verbose::
33 - Also output details about the matching pattern (if any)
34 - for each given pathname. For precedence rules within and
35 - between exclude sources, see linkgit:gitignore[5].
33 + Instead of printing the paths that are excluded, for each path
34 + that matches an exclude pattern, print the exclude pattern
35 + together with the path. (Matching an exclude pattern usually
36 + means the path is excluded, but if the pattern begins with '!'
37 + then it is a negated pattern and matching it means the path is
38 + NOT excluded.)
39 ++
40 +For precedence rules within and between exclude sources, see
41 +linkgit:gitignore[5].
42
43 --stdin::
44 Read pathnames from the standard input, one per line,
builtin/check-ignore.c
+3
@@ -108,6 +108,9 @@ static int check_ignore(struct dir_struct *dir,
108 int dtype = DT_UNKNOWN;
109 pattern = last_matching_pattern(dir, &the_index,
110 full_path, &dtype);
111 + if (!verbose && pattern &&
112 + pattern->flags & PATTERN_FLAG_NEGATIVE)
113 + pattern = NULL;
114 }
115 if (!quiet && (pattern || show_non_matching))
116 output_pattern(pathspec.items[i].original, pattern);
t/t0008-ignores.sh
+23 -16
@@ -424,9 +424,24 @@ test_expect_success 'local ignore inside a sub-directory with --verbose' '
424 )
425 '
426
427 -test_expect_success_multi 'nested include' \
428 - 'a/b/.gitignore:8:!on* a/b/one' '
429 - test_check_ignore "a/b/one"
427 +test_expect_success 'nested include of negated pattern' '
428 + expect "" &&
429 + test_check_ignore "a/b/one" 1
430 +'
431 +
432 +test_expect_success 'nested include of negated pattern with -q' '
433 + expect "" &&
434 + test_check_ignore "-q a/b/one" 1
435 +'
436 +
437 +test_expect_success 'nested include of negated pattern with -v' '
438 + expect "a/b/.gitignore:8:!on* a/b/one" &&
439 + test_check_ignore "-v a/b/one" 0
440 +'
441 +
442 +test_expect_success 'nested include of negated pattern with -v -n' '
443 + expect "a/b/.gitignore:8:!on* a/b/one" &&
444 + test_check_ignore "-v -n a/b/one" 0
445 '
446
447 ############################################################################
@@ -460,7 +475,6 @@ test_expect_success 'cd to ignored sub-directory' '
475 expect_from_stdin <<-\EOF &&
476 foo
477 twoooo
463 - ../one
478 seven
479 ../../one
480 EOF
@@ -543,7 +557,6 @@ test_expect_success 'global ignore' '
557 globalthree
558 a/globalthree
559 a/per-repo
546 - globaltwo
560 EOF
561 test_check_ignore "globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
562 '
@@ -586,17 +599,7 @@ EOF
599 cat <<-\EOF >expected-default
600 one
601 a/one
589 - a/b/on
590 - a/b/one
591 - a/b/one one
592 - a/b/one two
593 - "a/b/one\"three"
594 - a/b/two
602 a/b/twooo
596 - globaltwo
597 - a/globaltwo
598 - a/b/globaltwo
599 - b/globaltwo
603 EOF
604 cat <<-EOF >expected-verbose
605 .gitignore:1:one one
@@ -696,8 +699,12 @@ cat <<-EOF >expected-all
699 $global_excludes:2:!globaltwo ../b/globaltwo
700 :: c/not-ignored
701 EOF
702 +cat <<-EOF >expected-default
703 +../one
704 +one
705 +b/twooo
706 +EOF
707 grep -v '^:: ' expected-all >expected-verbose
700 -sed -e 's/.* //' expected-verbose >expected-default
708
709 broken_c_unquote stdin >stdin0
710