[PATCH v2 4/4] add -p: ignore dirty submodules

From: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date: Aug 24, 2022, 21:21 UTC
Message-ID: <540ce27c38a68bebc9504a1cb19fb4076f87ae1c.1661376112.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.1336.v2.git.1661376112.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Thanks to alwyas running `diff-index` and `diff-files` with the `--numstat` option (the latter with `--ignore-submodules=dirty`) before even generating any real diff to parse, the Perl version of `git add -p` simply ignored dirty submodules and does not even offer them up for staging.

However, the built-in variant did not use that flag because it tries to run only one `diff` command, skipping the unneeded `diff-index`/`diff-files` invocation of the Perl variant and therefore only faithfully recapitulates what the Perl code does once it _does_ generate and parse the real diff.

This causes a problem when running the built-in `add -p` with `diff-so-fancy` because that diff colorizer always inserts an empty line before the diff header to ensure that it produces 4 lines as expected by `git add -p` (the equivalent of the non-colorized `diff`, `index`, `---` and `+++` lines). But `git diff-files` does not produce any `index` line for dirty submodules.

The underlying problem is not even the discrepancy in lines, but that `git add -p` presents diffs for dirty submodules: there is nothing that _can_ be staged for those.

Let's fix that bug, and teach the built-in `add -p` to ignore dirty submodules, too. This _incidentally_ also fixes the `diff-so-fancy` problem ;-)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-patch.c                |  3 ++-
 t/t3701-add-interactive.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/add-patch.c b/add-patch.c
index 1f3f3611ee9..64807e7cdc0 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -417,7 +417,8 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 	}
 	color_arg_index = args.nr;
 	/* Use `--no-color` explicitly, just in case `diff.color = always`. */
-	strvec_pushl(&args, "--no-color", "-p", "--", NULL);
+	strvec_pushl(&args, "--no-color", "--ignore-submodules=dirty", "-p",
+		     "--", NULL);
 	for (i = 0; i < ps->nr; i++)
 		strvec_push(&args, ps->items[i].original);
 
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 9deb7a87f1e..10058b7ce42 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -941,6 +941,18 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '
 	! grep dirty-otherwise output
 '
 
+test_expect_success 'handle submodules' '
+	echo 123 >>for-submodules/dirty-otherwise/initial.t &&
+
+	force_color git -C for-submodules add -p dirty-otherwise >output 2>&1 &&
+	grep "No changes" output &&
+
+	force_color git -C for-submodules add -p dirty-head >output 2>&1 <y &&
+	git -C for-submodules ls-files --stage dirty-head >actual &&
+	rev="$(git -C for-submodules/dirty-head rev-parse HEAD)" &&
+	grep "$rev" actual
+'
+
 test_expect_success 'set up pathological context' '
 	git reset --hard &&
 	test_write_lines a a a a a a a a a a a >a &&

-- 
gitgitgadget