git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: kristofferhaugsbakk@fastmail.com
To: git@vger.kernel.org
Cc: Kristoffer Haugsbakk <code@khaugsbakk.name>,
	Denton Liu <liu.denton@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 2/3] revision: add rdiff_log_arg to rev_info
Date: Thu, 25 Sep 2025 19:07:35 +0200	[thread overview]
Message-ID: <d9419743773.1758819879.git.code@khaugsbakk.name> (raw)
In-Reply-To: <v2-cover.1758819879.git.code@khaugsbakk.name>

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

git-format-patch(1) supports Git notes by showing them beneath the
patch/commit message, similar to git-log(1). The command also supports
showing those same notes ref names in the range diff output.

Note *the same* ref names; any Git notes options or configuration
variables need to be handed off to the range-diff machinery. This works
correctly in the case when the range diff is on the cover letter. But it
does not work correctly when the output is a single patch with an
embedded range diff.

Concretely, git-format-patch(1) needs to pass `--[no-]notes` options
on to the range-diff subprocess in `range-diff.c`. This is handled in
`builtin/log.c` by the local variable `log_arg` in the case of mul-
tiple commits, but not in the single commit case where there is no
cover letter and the range diff is embedded in the patch output; the
range diff is then made in `log-tree.c`, whither `log_arg` has not
been propagated. This means that the range-diff subprocess reverts
to its default behavior, which is to act like git-log(1) w.r.t. notes.

We need to fix this. But first lay the groundwork by converting
`log_arg` to a struct member; next we can simply use that member
in `log-tree.c` without having to thread it from `builtin/log.c`.

No functional changes.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    v2:
    
    Rewrite the commit message.  The message jumps into the problem without
    setting the stage.  I think the problem should be presented for a future
    reader with only some basic prerequisite knowledge of this area; they
    might use git-format-patch(1), but they might not use Git notes *with*
    format-patch.[1]  For that reason, start with two paragraphs that pre-
    sent how notes are handled.  Then continue with the story in a format
    similar to v1.
    
    Note that the different “range diff” and “range-diff” spellings are
    intentional.  “Range diff” here refers to the diff output while
    “range-diff” refers to the machinery that creates that output.
    
    † 1: I don’t really see much Git notes use on patches on this mailing
         list.  And even less with non-default namespaces.  But in any case:
         the stage should be set properly even if regulars here *did* use
         format-patch notes a lot.
    
    Also fix handling of struct-in-struct (helped by Junio).
    
    • Rewrite the commit message
    • Fix struct-in-struct
    • (And) Reset author date.  I started this in June but the time
      investment is mostly from these last days.
    
    v1:
    
    There is also `other_arg` in `builtin/range-diff.c` but `rev_info` does
    not seem to be involved.

 builtin/log.c | 7 +++----
 revision.h    | 2 ++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 131512ac1af..9eff62ce111 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1400,13 +1400,12 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		 * can be added later if deemed desirable.
 		 */
 		struct diff_options opts;
-		struct strvec log_arg = STRVEC_INIT;
 		struct range_diff_options range_diff_opts = {
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 			.diffopt = &opts,
-			.log_arg = &log_arg
+			.log_arg = &rev->rdiff_log_arg
 		};
 
 		repo_diff_setup(the_repository, &opts);
@@ -1414,9 +1413,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		opts.use_color = rev->diffopt.use_color;
 		diff_setup_done(&opts);
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
-		get_notes_args(&log_arg, rev);
 		show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
-		strvec_clear(&log_arg);
 	}
 }
 
@@ -2328,6 +2325,7 @@ int cmd_format_patch(int argc,
 		rev.rdiff_title = diff_title(&rdiff_title, reroll_count,
 					     _("Range-diff:"),
 					     _("Range-diff against v%d:"));
+		get_notes_args(&(rev.rdiff_log_arg), &rev);
 	}
 
 	/*
@@ -2487,6 +2485,7 @@ int cmd_format_patch(int argc,
 	rev.diffopt.no_free = 0;
 	release_revisions(&rev);
 	format_config_release(&cfg);
+	strvec_clear(&rev.rdiff_log_arg);
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index 21e288c5baa..ce30570d86a 100644
--- a/revision.h
+++ b/revision.h
@@ -334,6 +334,7 @@ struct rev_info {
 	/* range-diff */
 	const char *rdiff1;
 	const char *rdiff2;
+	struct strvec rdiff_log_arg;
 	int creation_factor;
 	const char *rdiff_title;
 
@@ -410,6 +411,7 @@ struct rev_info {
 	.expand_tabs_in_log = -1, \
 	.commit_format = CMIT_FMT_DEFAULT, \
 	.expand_tabs_in_log_default = 8, \
+	.rdiff_log_arg = STRVEC_INIT, \
 }
 
 /**
-- 
2.51.0.311.g9b2318464ce


  parent reply	other threads:[~2025-09-25 17:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22 21:10 [PATCH 0/2] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk
2025-09-22 21:10 ` [PATCH 1/2] revision: add rdiff_other_arg to rev_info kristofferhaugsbakk
2025-09-22 21:58   ` Junio C Hamano
2025-09-23 15:53     ` Kristoffer Haugsbakk
2025-09-23 17:35       ` Junio C Hamano
2025-09-23 17:47         ` Kristoffer Haugsbakk
2025-09-23 21:18           ` Junio C Hamano
2025-09-22 21:10 ` [PATCH 2/2] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk
2025-09-22 22:01   ` Junio C Hamano
2025-09-23 16:26     ` Kristoffer Haugsbakk
2025-09-23 21:20       ` Junio C Hamano
2025-09-25 17:07 ` [PATCH v2 0/3] " kristofferhaugsbakk
2025-09-25 17:07   ` [PATCH v2 1/3] range-diff: rename other_arg to log_arg kristofferhaugsbakk
2025-09-25 17:07   ` kristofferhaugsbakk [this message]
2025-09-25 17:07   ` [PATCH v2 3/3] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk

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=d9419743773.1758819879.git.code@khaugsbakk.name \
    --to=kristofferhaugsbakk@fastmail.com \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).