[3/5] log: Push to/cc handling down into show_log()

Commit Message

Zev Weiss Jan. 19, 2023, 10:38 p.m. UTC

This rearrangement is a measure to facilitate adding --to-cmd/--cc-cmd
support to format-patch.  The command will need to be run separately
for each commit, so the lists of individual recipients specified via
--to and --cc (and potentially --add-header) need to be available at
the per-commit level instead of just the combined headers in a single
string as has been the case until now.

Signed-off-by: Zev Weiss <[e-mail]>
---
 builtin/log.c |  6 +++---
 log-tree.c    | 22 +++++++++++++++++++---
 log-tree.h    |  3 +--
 revision.h    |  2 ++
 4 files changed, 25 insertions(+), 8 deletions(-)

Comments

Junio C Hamano Jan. 20, 2023, 12:33 a.m. UTC | #1

Zev Weiss <[e-mail]> writes:

> Subject: Re: [PATCH 3/5] log: Push to/cc handling down into show_log()

s/Push/push/
cf. Documentation/SubmittingPatches[[summary-section]]

This is common to all these patches.

> @@ -1326,6 +1326,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  	pp.rev = rev;
>  	pp.print_email_subject = 1;
>  	pp_user_info(&pp, NULL, &sb, committer, encoding);
> +	format_recipients(rev, &sb);

This is where the two new members in the rev structure is used.

> @@ -2028,9 +2029,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		strbuf_addch(&buf, '\n');
>  	}
>  
> -	recipients_to_header_buf("To", &buf, &extra_to);
> -	recipients_to_header_buf("Cc", &buf, &extra_cc);
> -
> +	rev.to_recipients = &extra_to;
> +	rev.cc_recipients = &extra_cc;
>  	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);

And these two members point at borrowed memory.  extra_to and
extra_cc is freed after everything is done, near the end of the
cmd_format_patch() function.  We don't leak any extra memory by this
change, which is good.

Calvin Wan Feb. 1, 2023, 11:52 p.m. UTC | #2

>  void log_write_email_headers(struct rev_info *opt, struct commit *commit,
>  			     const char **extra_headers_p,
>  			     int *need_8bit_cte_p,
> @@ -647,10 +655,12 @@ static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)
>  void show_log(struct rev_info *opt)
>  {
>  	struct strbuf msgbuf = STRBUF_INIT;
> +	struct strbuf hdrbuf = STRBUF_INIT;
>  	struct log_info *log = opt->loginfo;
>  	struct commit *commit = log->commit, *parent = log->parent;
>  	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : the_hash_algo->hexsz;
>  	const char *extra_headers = opt->extra_headers;
> +	char *to_free;
>  	struct pretty_print_context ctx = {0};
>  
>  	opt->loginfo = NULL;
> @@ -770,6 +780,11 @@ void show_log(struct rev_info *opt)
>  		ctx.notes_message = strbuf_detach(&notebuf, NULL);
>  	}
>  
> +	format_recipients(opt, &hdrbuf);
> +
> +	if (extra_headers)
> +		strbuf_addstr(&hdrbuf, extra_headers);
> +
>  	/*
>  	 * And then the pretty-printed message itself
>  	 */
> @@ -779,7 +794,7 @@ void show_log(struct rev_info *opt)
>  	ctx.date_mode = opt->date_mode;
>  	ctx.date_mode_explicit = opt->date_mode_explicit;
>  	ctx.abbrev = opt->diffopt.abbrev;
> -	ctx.after_subject = extra_headers;
> +	ctx.after_subject = to_free = strbuf_detach(&hdrbuf, NULL);

Can you explain in the commit message why hdrbuf, which contains the
output from format_recipients and extra_headers, is still the same
functionality as before?

> -void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
> -			      const struct string_list *recipients);
> +void format_recipients(struct rev_info *rev, struct strbuf *sb);

What do you think about renaming this function to strbuf_add_recipients
and swapping the parameters?

Patch

diff --git a/builtin/log.c b/builtin/log.c
index ad9d7ebc6d73..c0c7b8544d73 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1326,6 +1326,7 @@  static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	pp.rev = rev;
 	pp.print_email_subject = 1;
 	pp_user_info(&pp, NULL, &sb, committer, encoding);
+	format_recipients(rev, &sb);
 	prepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);
 	fprintf(rev->diffopt.file, "%s\n", sb.buf);
 
@@ -2028,9 +2029,8 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		strbuf_addch(&buf, '\n');
 	}
 
-	recipients_to_header_buf("To", &buf, &extra_to);
-	recipients_to_header_buf("Cc", &buf, &extra_cc);
-
+	rev.to_recipients = &extra_to;
+	rev.cc_recipients = &extra_cc;
 	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);
 
 	if (from) {
diff --git a/log-tree.c b/log-tree.c
index 0e8863fe545a..7aa2841dd803 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -426,8 +426,8 @@  void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)
 	}
 }
 
-void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
-			      const struct string_list *recipients)
+static void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
+				     const struct string_list *recipients)
 {
 	for (int i = 0; i < recipients->nr; i++) {
 		if (!i)
@@ -441,6 +441,14 @@  void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
 	}
 }
 
+void format_recipients(struct rev_info *rev, struct strbuf *sb)
+{
+	if (rev->to_recipients)
+		recipients_to_header_buf("To", sb, rev->to_recipients);
+	if (rev->cc_recipients)
+		recipients_to_header_buf("Cc", sb, rev->cc_recipients);
+}
+
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p,
@@ -647,10 +655,12 @@  static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)
 void show_log(struct rev_info *opt)
 {
 	struct strbuf msgbuf = STRBUF_INIT;
+	struct strbuf hdrbuf = STRBUF_INIT;
 	struct log_info *log = opt->loginfo;
 	struct commit *commit = log->commit, *parent = log->parent;
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : the_hash_algo->hexsz;
 	const char *extra_headers = opt->extra_headers;
+	char *to_free;
 	struct pretty_print_context ctx = {0};
 
 	opt->loginfo = NULL;
@@ -770,6 +780,11 @@  void show_log(struct rev_info *opt)
 		ctx.notes_message = strbuf_detach(&notebuf, NULL);
 	}
 
+	format_recipients(opt, &hdrbuf);
+
+	if (extra_headers)
+		strbuf_addstr(&hdrbuf, extra_headers);
+
 	/*
 	 * And then the pretty-printed message itself
 	 */
@@ -779,7 +794,7 @@  void show_log(struct rev_info *opt)
 	ctx.date_mode = opt->date_mode;
 	ctx.date_mode_explicit = opt->date_mode_explicit;
 	ctx.abbrev = opt->diffopt.abbrev;
-	ctx.after_subject = extra_headers;
+	ctx.after_subject = to_free = strbuf_detach(&hdrbuf, NULL);
 	ctx.preserve_subject = opt->preserve_subject;
 	ctx.encode_email_headers = opt->encode_email_headers;
 	ctx.reflog_info = opt->reflog_info;
@@ -828,6 +843,7 @@  void show_log(struct rev_info *opt)
 
 	strbuf_release(&msgbuf);
 	free(ctx.notes_message);
+	free(to_free);
 
 	if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
 		struct diff_queue_struct dq;
diff --git a/log-tree.h b/log-tree.h
index 227edc564121..ace50dad6c83 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -25,8 +25,7 @@  void format_decorations_extended(struct strbuf *sb, const struct commit *commit,
 #define format_decorations(strbuf, commit, color) \
 			     format_decorations_extended((strbuf), (commit), (color), " (", ", ", ")")
 void show_decorations(struct rev_info *opt, struct commit *commit);
-void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
-			      const struct string_list *recipients);
+void format_recipients(struct rev_info *rev, struct strbuf *sb);
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p,
diff --git a/revision.h b/revision.h
index 30febad09a1e..330d351b2e4c 100644
--- a/revision.h
+++ b/revision.h
@@ -283,6 +283,8 @@  struct rev_info {
 	struct ident_split from_ident;
 	struct string_list *ref_message_ids;
 	int		add_signoff;
+	struct string_list *to_recipients;
+	struct string_list *cc_recipients;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;