Philippe Blain found and reported a couple issues with the output of --remerge-diff[1]. After digging in, I think one of them actually counts as two separate issues, so here's a series with three patches to fix these issues. Each includes testcases to keep us from regressing. Changes since v1: * Added a new diff_filepair_is_phoney() function to make the code more self-documenting, as suggested by Junio. * Note: Patch 2, as called out in its commit message, disables TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any memory leaks, but because I add new testcases invoking additional parts of the code (pickaxe stuff) which already had pre-existing leaks. This is not a change since v1, but this somehow accidentally got munged out of Junio's application of my v1. Note: The issue fixed by the third commit for --remerge-diff is also an issue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of --cc). However, as far as I can tell the causes are different and come from separate codepaths; this series focuses on --remerge-diff and hence makes no attempt to fix independent (even if similar) --cc or -c issues. [1] https://lore.kernel.org/git/43cf2a1d-058a-fd79-befe-7d9bc62581ed@xxxxxxxxx/ Elijah Newren (3): diff: have submodule_format logic avoid additional diff headers diff: fix filtering of additional headers under --remerge-diff diff: fix filtering of merge commits under --remerge-diff diff.c | 17 ++++++++++++++--- t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) base-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1342 Range-diff vs v1: 1: e4414cf630d ! 1: aea0bbc1c6a diff: have submodule_format logic avoid additional diff headers @@ Commit message Signed-off-by: Elijah Newren <newren@xxxxxxxxx> ## diff.c ## +@@ diff.c: static void add_formatted_headers(struct strbuf *msg, + line_prefix, meta, reset); + } + ++static int diff_filepair_is_phoney(struct diff_filespec *one, ++ struct diff_filespec *two) ++{ ++ return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two); ++} ++ + static void builtin_diff(const char *name_a, + const char *name_b, + struct diff_filespec *one, @@ diff.c: static void builtin_diff(const char *name_a, if (o->submodule_format == DIFF_SUBMODULE_LOG && (!one->mode || S_ISGITLINK(one->mode)) && - (!two->mode || S_ISGITLINK(two->mode))) { + (!two->mode || S_ISGITLINK(two->mode)) && -+ (one->mode || two->mode)) { ++ (!diff_filepair_is_phoney(one, two))) { show_submodule_diff_summary(o, one->path ? one->path : two->path, &one->oid, &two->oid, two->dirty_submodule); @@ diff.c: static void builtin_diff(const char *name_a, (!one->mode || S_ISGITLINK(one->mode)) && - (!two->mode || S_ISGITLINK(two->mode))) { + (!two->mode || S_ISGITLINK(two->mode)) && -+ (one->mode || two->mode)) { ++ (!diff_filepair_is_phoney(one, two))) { show_submodule_inline_diff(o, one->path ? one->path : two->path, &one->oid, &two->oid, two->dirty_submodule); +@@ diff.c: static void builtin_diff(const char *name_a, + b_two = quote_two(b_prefix, name_b + (*name_b == '/')); + lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; + lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; +- if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) { ++ if (diff_filepair_is_phoney(one, two)) { + /* + * We should only reach this point for pairs from + * create_filepairs_for_header_only_notifications(). For ## t/t4069-remerge-diff.sh ## @@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif 2: feac9749460 = 2: 3bd622d5b45 diff: fix filtering of additional headers under --remerge-diff 3: 46ea0d7dd65 = 3: 7903f8380dc diff: fix filtering of merge commits under --remerge-diff -- gitgitgadget
