git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/3] diff-filter: be more careful when looking for negative bits

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 28, 2022, 12:02 UTC
Message-ID
<f1f027ad61beb1bd0dee73acbffdee5c0f967e9a.1643371370.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1127.v3.git.1643371370.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `--diff-filter=<bits>` option allows to filter the diff by certain criteria, for example `R` to only show renamed files. It also supports negating a filter via a down-cased letter, i.e. `r` to show _everything but_ renamed files.

However, the code is a bit overzealous when trying to figure out whether `git diff` should start with all diff-filters turned on because the user provided a lower-case letter: if the `--diff-filter` argument starts with an upper-case letter, we must not start with all bits turned on.

Even worse, it is possible to specify the diff filters in multiple, separate options, e.g. `--diff-filter=AM [...] --diff-filter=m`.

Let's accumulate the include/exclude filters independently, and only special-case the "only exclude filters were specified" case after parsing the options altogether.

Note: The code replaced by this commit took pains to avoid setting any
unused bits of `options->filter`. That was unnecessary, though, as all
accesses happen via the `filter_bit_tst()` function using specific bits,
and setting the unused bits has no effect. Therefore, we can simplify
the code by using `~0` (or in this instance, `~<unwanted-bit>`).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 diff.c         | 23 +++++++----------------
 diff.h         |  2 +-
 t/t4202-log.sh | 13 +++++++++++++
 3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/diff.c b/diff.c
index 5081052c431..4ab4299b817 100644
--- a/diff.c
+++ b/diff.c
@@ -4720,6 +4720,12 @@ void diff_setup_done(struct diff_options *options)
 	if (!options->use_color || external_diff())
 		options->color_moved = 0;
 
+	if (options->filter_not) {
+		if (!options->filter)
+			options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];
+		options->filter &= ~options->filter_not;
+	}
+
 	FREE_AND_NULL(options->parseopts);
 }
 
@@ -4820,21 +4826,6 @@ static int diff_opt_diff_filter(const struct option *option,
 	BUG_ON_OPT_NEG(unset);
 	prepare_filter_bits();
 
-	/*
-	 * If there is a negation e.g. 'd' in the input, and we haven't
-	 * initialized the filter field with another --diff-filter, start
-	 * from full set of bits, except for AON.
-	 */
-	if (!opt->filter) {
-		for (i = 0; (optch = optarg[i]) != '\0'; i++) {
-			if (optch < 'a' || 'z' < optch)
-				continue;
-			opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
-			opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
-			break;
-		}
-	}
-
 	for (i = 0; (optch = optarg[i]) != '\0'; i++) {
 		unsigned int bit;
 		int negate;
@@ -4851,7 +4842,7 @@ static int diff_opt_diff_filter(const struct option *option,
 			return error(_("unknown change class '%c' in --diff-filter=%s"),
 				     optarg[i], optarg);
 		if (negate)
-			opt->filter &= ~bit;
+			opt->filter_not |= bit;
 		else
 			opt->filter |= bit;
 	}
diff --git a/diff.h b/diff.h
index 8ba85c5e605..a70e7c478c1 100644
--- a/diff.h
+++ b/diff.h
@@ -283,7 +283,7 @@ struct diff_options {
 	struct diff_flags flags;
 
 	/* diff-filter bits */
-	unsigned int filter;
+	unsigned int filter, filter_not;
 
 	int use_color;
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 50495598619..b25182379ff 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -142,6 +142,19 @@ test_expect_success 'diff-filter=R' '
 
 '
 
+test_expect_success 'multiple --diff-filter bits' '
+
+	git log -M --pretty="format:%s" --diff-filter=R HEAD >expect &&
+	git log -M --pretty="format:%s" --diff-filter=Ra HEAD >actual &&
+	test_cmp expect actual &&
+	git log -M --pretty="format:%s" --diff-filter=aR HEAD >actual &&
+	test_cmp expect actual &&
+	git log -M --pretty="format:%s" \
+		--diff-filter=a --diff-filter=R HEAD >actual &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'diff-filter=C' '
 
 	git log -C -C --pretty="format:%s" --diff-filter=C HEAD >actual &&
-- 
gitgitgadget
Previous: Johannes Schindelin via GitGitGadget
Message 21 of 21 in “Fix two --diff-filter bugs”
  1. 0/2 Fix two --diff-filter bugsJohannes Schindelin via GitGitGadget, Jan 25, 2022
  2. 1/2 docs(diff): lose incorrect claim about `diff-files --diff-filter=A`Johannes Schindelin via GitGitGadget, Jan 25, 2022
  3. Junio C HamanoJan 26, 2022
  4. 2/2 diff-filter: be more careful when looking for negative bitsJohannes Schindelin via GitGitGadget, Jan 25, 2022
  5. Taylor BlauJan 25, 2022
  6. Johannes SchindelinJan 28, 2022
  7. Junio C HamanoJan 26, 2022
  8. Taylor BlauJan 26, 2022
  9. Junio C HamanoJan 26, 2022
  10. Johannes SchindelinJan 28, 2022
  11. Ævar Arnfjörð BjarmasonJan 26, 2022
  12. 0/3 Fix two --diff-filter bugsJohannes Schindelin via GitGitGadget, Jan 27, 2022
  13. 1/3 docs(diff): lose incorrect claim about `diff-files --diff-filter=A`Johannes Schindelin via GitGitGadget, Jan 27, 2022
  14. 2/3 diff.c: move the diff filter bits definitions up a bitJohannes Schindelin via GitGitGadget, Jan 27, 2022
  15. 3/3 diff-filter: be more careful when looking for negative bitsJohannes Schindelin via GitGitGadget, Jan 27, 2022
  16. Junio C HamanoJan 28, 2022
  17. Johannes SchindelinJan 28, 2022
  18. 0/3 Fix two --diff-filter bugsJohannes Schindelin via GitGitGadget, Jan 28, 2022
  19. 1/3 docs(diff): lose incorrect claim about `diff-files --diff-filter=A`Johannes Schindelin via GitGitGadget, Jan 28, 2022
  20. 2/3 diff.c: move the diff filter bits definitions up a bitJohannes Schindelin via GitGitGadget, Jan 28, 2022
  21. 3/3 diff-filter: be more careful when looking for negative bitsJohannes Schindelin via GitGitGadget, Jan 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.