setup_revisions(): turn on diffs for all-negative diff filter

When the user gives us a diff filter like --diff-filter=D, we need to do
a tree diff even if we're not planning to show the diff result itself,
in order to decide whether to show the commit at all. So there's an
explicit check of revs->diffopt.filter in setup_revisions(), and we set
revs->diff if any bits are set.

Originally that "filter" field covered both positive capital-letter
filters (like "D") and also negative lowercase filters (like "d"), so it
was sufficient for both cases. But later, 75408ca949 (diff-filter: be
more careful when looking for negative bits, 2022-01-28) split the
negative bits out into a "filter_not" field.

We eventually fold those into "filter", but not until diff_setup_done()
is called, which happens after our explicit check. As a result, a purely
negative filter like:

  git log --diff-filter=d

failed to turn on diffs at all. But rather than fail to filter by diff,
because the filter variable is eventually set, we mistakenly show no
commits at all, thinking that the empty diffs were cases where nothing
passed through the filter.

The smallest fix here is to just have our check look for any bits in
either "filter" or "filter_not". I suspect it would also be OK to
reorder the function a bit to call diff_setup_done() earlier, but that
risks violating some other subtle ordering dependency. So I went with
the simple and safe solution here.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/revision.c b/revision.c
index 474fa1e767c8d8f90ce39190ccf8b49cdaa51f54..0f9da40d8cb8c350ea7b7fd1f256b4ad6e4fdc55 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -3112,7 +3112,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
        /* Pickaxe, diff-filter and rename following need diffs */
        if ((revs->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK) ||
-           revs->diffopt.filter ||
+           revs->diffopt.filter || revs->diffopt.filter_not ||
            revs->diffopt.flags.follow_renames)
                revs->diff = 1;
 

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 51f7beb59f88c8fba384acf7c4772e255b612dbf..fed69717a0a91cf4de7b355fb468822ad786387a 100755 (executable)
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -134,6 +134,12 @@ test_expect_success 'diff-filter=D' '
 
 '
 
+test_expect_success 'all-negative filter' '
+       git log --no-renames --format=%s --diff-filter=d HEAD >actual &&
+       printf "%s\n" fifth fourth third second initial >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'diff-filter=R' '
 
        git log -M --pretty="format:%s" --diff-filter=R HEAD >actual &&