All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v2 0/3] Output fixes for --remerge-diff
Date: Thu, 01 Sep 2022 07:08:04 +0000	[thread overview]
Message-ID: <pull.1342.v2.git.1662016087.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1342.git.1661926908.gitgitgadget@gmail.com>

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@gmail.com/

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@gmail.com>
      
       ## 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

  parent reply	other threads:[~2022-09-01  7:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-31  6:21 [PATCH 0/3] Output fixes for --remerge-diff Elijah Newren via GitGitGadget
2022-08-31  6:21 ` [PATCH 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-08-31 22:20   ` Junio C Hamano
2022-09-01  3:44     ` Elijah Newren
2022-08-31  6:21 ` [PATCH 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-08-31 22:26   ` Junio C Hamano
2022-09-01  3:38     ` Elijah Newren
2022-08-31  6:21 ` [PATCH 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget
2022-09-01  1:13 ` [PATCH 0/3] Output fixes for --remerge-diff Junio C Hamano
2022-09-01  3:47   ` Elijah Newren
2022-09-01  4:01     ` Elijah Newren
2022-09-01 15:24     ` Junio C Hamano
2022-09-01 18:46       ` Ævar Arnfjörð Bjarmason
2022-09-01 19:54         ` Junio C Hamano
2022-09-01  7:08 ` Elijah Newren via GitGitGadget [this message]
2022-09-01  7:08   ` [PATCH v2 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-09-01 16:30     ` Junio C Hamano
2022-09-01  7:08   ` [PATCH v2 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-09-01  7:08   ` [PATCH v2 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget
2022-09-02  3:53   ` [PATCH v3 0/3] Output fixes for --remerge-diff Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 1/3] diff: have submodule_format logic avoid additional diff headers Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 2/3] diff: fix filtering of additional headers under --remerge-diff Elijah Newren via GitGitGadget
2022-09-02  3:53     ` [PATCH v3 3/3] diff: fix filtering of merge commits " Elijah Newren via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1342.v2.git.1662016087.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.