Hi Dscho
On 29/08/2022 16:11, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>
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.
I had a bit of a hard time understanding this paragraph. To me the fact
that the perl version is using --numstat is not that important here,
what is important is that it is using --ignore-submodules=dirty when it
generates its list of files to show and that information is consigned to
a parenthesized aside.
The fix itself looks good.
Best Wishes
Phillip
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@xxxxxx>
---
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 0217cdd7c4a..ee6a3d3b712 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -426,7 +426,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 39e68b6d066..a4f45fc48a0 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -965,6 +965,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 &&
