From: Elijah Newren <newren@xxxxxxxxx>
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@xxxxxxxxx>
---
check-ignore: fix handling with negated patterns
Changes since v2:
* Add missing comma, as pointed out by Junio.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-711%2Fnewren%2Ffix-check-ignore-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-711/newren/fix-check-ignore-v3
Pull-Request: https://github.com/git/git/pull/711
Range-diff vs v2:
1: 1c289cb4d9a ! 1: 99d2e466844 check-ignore: fix documentation and implementation to match
@@ -83,7 +83,7 @@
- for each given pathname. For precedence rules within and
- between exclude sources, see linkgit:gitignore[5].
+ Instead of printing the paths that are excluded, for each path
-+ that matches an exclude pattern print the exclude pattern
++ that matches an exclude pattern, print the exclude pattern
+ together with the path. (Matching an exclude pattern usually
+ means the path is excluded, but if the pattern begins with '!'
+ then it is a negated pattern and matching it means the path is
Documentation/git-check-ignore.txt | 12 ++++++---
builtin/check-ignore.c | 3 +++
t/t0008-ignores.sh | 39 ++++++++++++++++++------------
3 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt
index 8b2d49c79e1..0c3924a63d2 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -30,9 +30,15 @@ OPTIONS
valid with a single pathname.
-v, --verbose::
- Also output details about the matching pattern (if any)
- for each given pathname. For precedence rules within and
- between exclude sources, see linkgit:gitignore[5].
+ Instead of printing the paths that are excluded, for each path
+ that matches an exclude pattern, print the exclude pattern
+ together with the path. (Matching an exclude pattern usually
+ means the path is excluded, but if the pattern begins with '!'
+ then it is a negated pattern and matching it means the path is
+ NOT excluded.)
++
+For precedence rules within and between exclude sources, see
+linkgit:gitignore[5].
--stdin::
Read pathnames from the standard input, one per line,
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 5a4f92395f3..ea5d0ae3a6a 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -108,6 +108,9 @@ static int check_ignore(struct dir_struct *dir,
int dtype = DT_UNKNOWN;
pattern = last_matching_pattern(dir, &the_index,
full_path, &dtype);
+ if (!verbose && pattern &&
+ pattern->flags & PATTERN_FLAG_NEGATIVE)
+ pattern = NULL;
}
if (!quiet && (pattern || show_non_matching))
output_pattern(pathspec.items[i].original, pattern);
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 1744cee5e99..370a389e5c5 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -424,9 +424,24 @@ test_expect_success 'local ignore inside a sub-directory with --verbose' '
)
'
-test_expect_success_multi 'nested include' \
- 'a/b/.gitignore:8:!on* a/b/one' '
- test_check_ignore "a/b/one"
+test_expect_success 'nested include of negated pattern' '
+ expect "" &&
+ test_check_ignore "a/b/one" 1
+'
+
+test_expect_success 'nested include of negated pattern with -q' '
+ expect "" &&
+ test_check_ignore "-q a/b/one" 1
+'
+
+test_expect_success 'nested include of negated pattern with -v' '
+ expect "a/b/.gitignore:8:!on* a/b/one" &&
+ test_check_ignore "-v a/b/one" 0
+'
+
+test_expect_success 'nested include of negated pattern with -v -n' '
+ expect "a/b/.gitignore:8:!on* a/b/one" &&
+ test_check_ignore "-v -n a/b/one" 0
'
############################################################################
@@ -460,7 +475,6 @@ test_expect_success 'cd to ignored sub-directory' '
expect_from_stdin <<-\EOF &&
foo
twoooo
- ../one
seven
../../one
EOF
@@ -543,7 +557,6 @@ test_expect_success 'global ignore' '
globalthree
a/globalthree
a/per-repo
- globaltwo
EOF
test_check_ignore "globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
'
@@ -586,17 +599,7 @@ EOF
cat <<-\EOF >expected-default
one
a/one
- a/b/on
- a/b/one
- a/b/one one
- a/b/one two
- "a/b/one\"three"
- a/b/two
a/b/twooo
- globaltwo
- a/globaltwo
- a/b/globaltwo
- b/globaltwo
EOF
cat <<-EOF >expected-verbose
.gitignore:1:one one
@@ -696,8 +699,12 @@ cat <<-EOF >expected-all
$global_excludes:2:!globaltwo ../b/globaltwo
:: c/not-ignored
EOF
+cat <<-EOF >expected-default
+../one
+one
+b/twooo
+EOF
grep -v '^:: ' expected-all >expected-verbose
-sed -e 's/.* //' expected-verbose >expected-default
broken_c_unquote stdin >stdin0
base-commit: bfdd66e72fffd18235757bedbf355fd4176d6019
--
gitgitgadget
