git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/5] format-patch: Add --{to,cc}-cmd support
@ 2023-01-19 22:38 Zev Weiss
  2023-01-19 22:38 ` [PATCH 1/5] t4014: Add test checking cover-letter To header Zev Weiss
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git
  Cc: Zev Weiss, Junio C Hamano, Eric Sunshine, brian m. carlson,
	Ævar Arnfjörð Bjarmason, Patrick Hemmer,
	René Scharfe, Denton Liu, Emma Brooks, Hamza Mahfooz

Hello,

On more than one occasion I've found myself wishing that git's
format-patch command supported the --to-cmd/--cc-cmd flags that
send-email does; this patch series is my attempt at implementing it.

This is my first foray into the git codebase, so odds are good I've
bungled something or missed the existence of some useful API, but it
passes the existing test suite (and the tests added for the new
feature), and I'm giving it its inaugural dogfooding on this very
series with `contrib/contacts/git-contacts`.

The first patch adds a test I found myself wanting when I realized I
had accidentally broken something that wasn't covered by the existing
tests.  The next two do a bit of refactoring and rearrangement, the
fourth adds a bit of library infrastructure to pretty.c, and the final
patch implements the feature itself.

There are a few points where I wasn't sure if the approach I chose was
really the best:

 - The 'name_and_address_only' pretty_print_context flag added in the
   fourth patch is kind of clunky, but I didn't see any other obvious
   great way to reuse the bits of pp_user_info() that I wanted.

 - Fork/exec-ing another internal format-patch command to generate the
   temporary patch file to run the commands on -- my attempts to
   recurse back into show_log() to do it more directly didn't go well.

 - As currently written any commands specified are only run on the
   patches themselves, not on the cover letter.  The slight
   inconsistency with git-send-email is a bit unfortunate, but I
   figure it's probably not often all that useful in that context
   anyway (and the implementation looked like it would be just
   non-trivial enough that laziness may have played a role as well).

Suggestions on better alternatives there (or elsewhere) are of course
welcome.


Thanks,
Zev Weiss

Zev Weiss (5):
  t4014: Add test checking cover-letter To header
  log: Refactor duplicated code to headerize recipient lists
  log: Push to/cc handling down into show_log()
  pretty: Add name_and_address_only parameter
  format-patch: Add support for --{to,cc}-cmd flags

 builtin/log.c           |  31 +++---
 log-tree.c              | 208 +++++++++++++++++++++++++++++++++++++++-
 log-tree.h              |   1 +
 pretty.c                |  15 ++-
 pretty.h                |   1 +
 revision.h              |   4 +
 t/t4014-format-patch.sh |  26 +++++
 7 files changed, 262 insertions(+), 24 deletions(-)

-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] t4014: Add test checking cover-letter To header
  2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
@ 2023-01-19 22:38 ` Zev Weiss
  2023-01-19 22:38 ` [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists Zev Weiss
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git; +Cc: Zev Weiss, Eric Sunshine, Junio C Hamano

I at first inadvertently broke this in the process of adding
--{to,cc}-cmd support to format-patch and didn't immediately notice
because there wasn't a test for it, so let's add one now.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 t/t4014-format-patch.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 012f155e10ae..ba5fd0efe2ae 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -2368,4 +2368,11 @@ test_expect_success 'interdiff: solo-patch' '
 	test_cmp expect actual
 '
 
+test_expect_success 'cover-letter gets To: header' '
+	rm -fr patches &&
+	git config --unset-all format.to &&
+	git format-patch -o patches --cover-letter --to "R E Cipient <rcipient@example.com>" main..side >list &&
+	grep "^To: R E Cipient <rcipient@example.com>\$" patches/0000-cover-letter.patch
+'
+
 test_done
-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists
  2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
  2023-01-19 22:38 ` [PATCH 1/5] t4014: Add test checking cover-letter To header Zev Weiss
@ 2023-01-19 22:38 ` Zev Weiss
  2023-01-25  2:11   ` Junio C Hamano
  2023-01-19 22:38 ` [PATCH 3/5] log: Push to/cc handling down into show_log() Zev Weiss
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git
  Cc: Zev Weiss, brian m. carlson,
	Ævar Arnfjörð Bjarmason, Junio C Hamano,
	Patrick Hemmer

The To and Cc headers are handled identically (the only difference is
the header name tag), so we might as well reuse the same code for both
instead of duplicating it.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 builtin/log.c | 23 ++---------------------
 log-tree.c    | 15 +++++++++++++++
 log-tree.h    |  2 ++
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 057e299c245c..ad9d7ebc6d73 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2028,27 +2028,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		strbuf_addch(&buf, '\n');
 	}
 
-	if (extra_to.nr)
-		strbuf_addstr(&buf, "To: ");
-	for (i = 0; i < extra_to.nr; i++) {
-		if (i)
-			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_to.items[i].string);
-		if (i + 1 < extra_to.nr)
-			strbuf_addch(&buf, ',');
-		strbuf_addch(&buf, '\n');
-	}
-
-	if (extra_cc.nr)
-		strbuf_addstr(&buf, "Cc: ");
-	for (i = 0; i < extra_cc.nr; i++) {
-		if (i)
-			strbuf_addstr(&buf, "    ");
-		strbuf_addstr(&buf, extra_cc.items[i].string);
-		if (i + 1 < extra_cc.nr)
-			strbuf_addch(&buf, ',');
-		strbuf_addch(&buf, '\n');
-	}
+	recipients_to_header_buf("To", &buf, &extra_to);
+	recipients_to_header_buf("Cc", &buf, &extra_cc);
 
 	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);
 
diff --git a/log-tree.c b/log-tree.c
index 1dd5fcbf7be4..0e8863fe545a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -426,6 +426,21 @@ 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)
+{
+	for (int i = 0; i < recipients->nr; i++) {
+		if (!i)
+			strbuf_addf(buf, "%s: ", hdr);
+		else
+			strbuf_addstr(buf, "    ");
+		strbuf_addstr(buf, recipients->items[i].string);
+		if (i + 1 < recipients->nr)
+			strbuf_addch(buf, ',');
+		strbuf_addch(buf, '\n');
+	}
+}
+
 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/log-tree.h b/log-tree.h
index e7e4641cf83c..227edc564121 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -25,6 +25,8 @@ 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 log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p,
-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] log: Push to/cc handling down into show_log()
  2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
  2023-01-19 22:38 ` [PATCH 1/5] t4014: Add test checking cover-letter To header Zev Weiss
  2023-01-19 22:38 ` [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists Zev Weiss
@ 2023-01-19 22:38 ` Zev Weiss
  2023-01-20  0:33   ` Junio C Hamano
  2023-01-19 22:38 ` [PATCH 4/5] pretty: Add name_and_address_only parameter Zev Weiss
  2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss
  4 siblings, 1 reply; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git
  Cc: Zev Weiss, brian m. carlson,
	Ævar Arnfjörð Bjarmason, René Scharfe,
	Denton Liu, Emma Brooks, Eric Sunshine, Junio C Hamano,
	Patrick Hemmer

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 <zev@bewilderbeest.net>
---
 builtin/log.c |  6 +++---
 log-tree.c    | 22 +++++++++++++++++++---
 log-tree.h    |  3 +--
 revision.h    |  2 ++
 4 files changed, 25 insertions(+), 8 deletions(-)

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;
-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] pretty: Add name_and_address_only parameter
  2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
                   ` (2 preceding siblings ...)
  2023-01-19 22:38 ` [PATCH 3/5] log: Push to/cc handling down into show_log() Zev Weiss
@ 2023-01-19 22:38 ` Zev Weiss
  2023-01-25  2:23   ` Junio C Hamano
  2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss
  4 siblings, 1 reply; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git; +Cc: Zev Weiss, Emma Brooks, Hamza Mahfooz, Junio C Hamano

This is meant to be used with pp_user_info() when using it to format
email recipients generated by --to-cmd/--cc-cmd.  When set it omits
the leading 'From: ', trailing linefeed, and the date suffix, and
additionally will return the input string unmodified if
split_ident_line() can't parse it (e.g. for a bare email address).

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 pretty.c | 15 ++++++++++++---
 pretty.h |  1 +
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/pretty.c b/pretty.c
index 1e1e21878c83..e6798fadc107 100644
--- a/pretty.c
+++ b/pretty.c
@@ -509,8 +509,11 @@ void pp_user_info(struct pretty_print_context *pp,
 		return;
 
 	line_end = strchrnul(line, '\n');
-	if (split_ident_line(&ident, line, line_end - line))
+	if (split_ident_line(&ident, line, line_end - line)) {
+		if (pp->name_and_address_only)
+			strbuf_addstr(sb, line);
 		return;
+	}
 
 	mailbuf = ident.mail_begin;
 	maillen = ident.mail_end - ident.mail_begin;
@@ -538,7 +541,8 @@ void pp_user_info(struct pretty_print_context *pp,
 			namelen = pp->from_ident->name_end - namebuf;
 		}
 
-		strbuf_addstr(sb, "From: ");
+		if (!pp->name_and_address_only)
+			strbuf_addstr(sb, "From: ");
 		if (pp->encode_email_headers &&
 		    needs_rfc2047_encoding(namebuf, namelen)) {
 			add_rfc2047(sb, namebuf, namelen,
@@ -558,7 +562,9 @@ void pp_user_info(struct pretty_print_context *pp,
 		if (max_length <
 		    last_line_length(sb) + strlen(" <") + maillen + strlen(">"))
 			strbuf_addch(sb, '\n');
-		strbuf_addf(sb, " <%.*s>\n", (int)maillen, mailbuf);
+		strbuf_addf(sb, " <%.*s>", (int)maillen, mailbuf);
+		if (!pp->name_and_address_only)
+			strbuf_addch(sb, '\n');
 	} else {
 		struct strbuf id = STRBUF_INIT;
 		enum grep_header_field field = GREP_HEADER_FIELD_MAX;
@@ -582,6 +588,9 @@ void pp_user_info(struct pretty_print_context *pp,
 		strbuf_release(&id);
 	}
 
+	if (pp->name_and_address_only)
+		return;
+
 	switch (pp->fmt) {
 	case CMIT_FMT_MEDIUM:
 		strbuf_addf(sb, "Date:   %s\n",
diff --git a/pretty.h b/pretty.h
index f34e24c53a49..306666e99294 100644
--- a/pretty.h
+++ b/pretty.h
@@ -39,6 +39,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned name_and_address_only:1;
 	int print_email_subject;
 	int expand_tabs_in_log;
 	int need_8bit_cte;
-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags
  2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
                   ` (3 preceding siblings ...)
  2023-01-19 22:38 ` [PATCH 4/5] pretty: Add name_and_address_only parameter Zev Weiss
@ 2023-01-19 22:38 ` Zev Weiss
  4 siblings, 0 replies; 9+ messages in thread
From: Zev Weiss @ 2023-01-19 22:38 UTC (permalink / raw)
  To: git; +Cc: Zev Weiss, Denton Liu, Eric Sunshine, Junio C Hamano

Having these flags available for format-patch instead of only on
send-email makes it much easier to use an automated command to do the
bulk of the recipient-selection work but still manually adjust it if
desired.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 builtin/log.c           |  10 +++
 log-tree.c              | 179 +++++++++++++++++++++++++++++++++++++++-
 revision.h              |   2 +
 t/t4014-format-patch.sh |  19 +++++
 4 files changed, 208 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index c0c7b8544d73..da3edb2a8299 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1877,6 +1877,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
+	char *to_cmd_arg = NULL;
+	char *cc_cmd_arg = NULL;
 
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
@@ -1929,6 +1931,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("add email header"), header_callback),
 		OPT_CALLBACK(0, "to", NULL, N_("email"), N_("add To: header"), to_callback),
 		OPT_CALLBACK(0, "cc", NULL, N_("email"), N_("add Cc: header"), cc_callback),
+		OPT_STRING(0, "to-cmd", &to_cmd_arg, N_("command"),
+		           N_("command to generate To: addresses for a patch")),
+		OPT_STRING(0, "cc-cmd", &cc_cmd_arg, N_("command"),
+		           N_("command to generate Cc: addresses for a patch")),
 		OPT_CALLBACK_F(0, "from", &from, N_("ident"),
 			    N_("set From address to <ident> (or committer ident if absent)"),
 			    PARSE_OPT_OPTARG, from_callback),
@@ -2031,6 +2037,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	rev.to_recipients = &extra_to;
 	rev.cc_recipients = &extra_cc;
+
+	rev.to_cmd = to_cmd_arg;
+	rev.cc_cmd = cc_cmd_arg;
+
 	rev.extra_headers = to_free = strbuf_detach(&buf, NULL);
 
 	if (from) {
diff --git a/log-tree.c b/log-tree.c
index 7aa2841dd803..6fdc3a3ffb7f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -20,6 +20,7 @@
 #include "help.h"
 #include "range-diff.h"
 #include "strmap.h"
+#include "run-command.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -652,11 +653,175 @@ static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)
 	opt->shown_dashes = 1;
 }
 
+/*
+ * Aims to mirror git-send-email.perl's function of the same name, returning a
+ * malloc()ed string that the caller must free().
+ */
+static char *sanitize_address(const struct rev_info *opt, const char *entry)
+{
+	int escaped = 0;
+	const char *inp;
+	char *tmp, *outp;
+	struct strbuf addr = STRBUF_INIT;
+	struct pretty_print_context ctx = {0};
+
+	/* Skip over any leading whitespace */
+	while (isspace(*entry))
+		entry++;
+
+	outp = tmp = xmalloc(strlen(entry) + 1);
+
+	/* Remove non-escaped quotes */
+	for (inp = entry; *inp; inp++) {
+		if (escaped) {
+			escaped = 0;
+		} else {
+			switch (*inp) {
+			case '\\':
+				escaped = 1;
+				/* fallthrough */
+			case '"':
+				continue;
+			}
+		}
+		*outp++ = *inp;
+	}
+	*outp = '\0';
+
+	ctx.fmt = opt->commit_format;
+	ctx.mailmap = opt->mailmap;
+	ctx.encode_email_headers = opt->encode_email_headers;
+	ctx.output_encoding = get_log_output_encoding();
+	ctx.name_and_address_only = 1;
+
+	pp_user_info(&ctx, NULL, &addr, tmp, get_log_output_encoding());
+
+	free(tmp);
+
+	return strbuf_detach(&addr, NULL);
+}
+
+/*
+ * Given 'text' as the output of a --to-cmd or --cc-cmd command, add each
+ * entry to 'list'.
+ */
+static void ingest_recipients_to_list(const char *text, const struct rev_info *opt,
+				      struct string_list *list)
+{
+	struct string_list_item *item;
+	struct string_list lines = STRING_LIST_INIT_DUP;
+
+	string_list_split(&lines, text, '\n', -1);
+
+	for_each_string_list_item(item, &lines) {
+		char *addr = sanitize_address(opt, item->string);
+		if (*addr)
+			string_list_append_nodup(list, addr);
+		else
+			free(addr);
+	}
+
+	string_list_clear(&lines, 0);
+}
+
+/*
+ * Generate a temporary patch file for the given commit, returning its path as
+ * a malloc()ed string the caller must free (and should unlink when finished
+ * with it).
+ */
+static char *generate_temp_patch(struct commit *commit)
+{
+	char path[PATH_MAX];
+	char *diff_output_arg;
+	struct strbuf diff_output_arg_buf = STRBUF_INIT;
+	struct child_process diffproc = CHILD_PROCESS_INIT;
+
+	xsnprintf(path, sizeof(path), ".git-temp-diff.XXXXXX");
+	close(xmkstemp(path));
+
+	strbuf_addf(&diff_output_arg_buf, "--output=%s", path);
+	diff_output_arg = strbuf_detach(&diff_output_arg_buf, NULL);
+
+	diffproc.git_cmd = 1;
+	strvec_push(&diffproc.args, "format-patch");
+	strvec_push(&diffproc.args, "-1");
+	strvec_push(&diffproc.args, diff_output_arg);
+	strvec_push(&diffproc.args, oid_to_hex(&commit->object.oid));
+
+	if (run_command(&diffproc))
+		die(_("Error generating temporary diff"));
+
+	free(diff_output_arg);
+
+	return xstrdup(path);
+}
+
+/*
+ * Execute a --to-cmd or --cc-cmd command on a temporary patch file, adding
+ * each recipient produced to 'list'.
+ */
+static void run_recipients_command(const char *cmd, const char *tmpdiff,
+				   const struct rev_info *opt,
+				   struct string_list *list)
+{
+	char *full_cmd;
+	char *cmd_output;
+	struct strbuf cmd_output_buf = STRBUF_INIT;
+	struct strbuf cmd_buf = STRBUF_INIT;
+	struct child_process cmdproc = CHILD_PROCESS_INIT;
+
+	strbuf_addf(&cmd_buf, "%s %s", cmd, tmpdiff);
+	full_cmd = strbuf_detach(&cmd_buf, NULL);
+
+	cmdproc.use_shell = 1;
+	strvec_push(&cmdproc.args, full_cmd);
+	if (capture_command(&cmdproc, &cmd_output_buf, 1024))
+		die(_("Error generating recipients list: command failed"));
+
+	cmd_output = strbuf_detach(&cmd_output_buf, NULL);
+	ingest_recipients_to_list(cmd_output, opt, list);
+
+	free(cmd_output);
+	free(full_cmd);
+}
+
+/*
+ * Generate a To or Cc header into 'buf', where 'header' is "To" or "Cc",
+ * 'fixed' is the list of fixed recipients (e.g. those specified by --to or
+ * --cc, optional), 'cmd' is the (optional) --to-cmd or --cc-cmd argument to
+ * generate recipients, and 'tmpdiff' is the path of a temp file containing a
+ * patch file to run 'cmd' on (mandatory if 'cmd' is non-NULL).
+ */
+static void headerize_recipients(const char *header, const char *tmpdiff,
+				 const struct rev_info *opt,
+				 const struct string_list *fixed,
+				 const char *cmd, struct strbuf *buf)
+{
+	struct string_list_item *item;
+	struct string_list recipients = STRING_LIST_INIT_DUP;
+
+	if (fixed) {
+		for_each_string_list_item(item, fixed)
+			string_list_append(&recipients, item->string);
+	}
+
+	if (cmd)
+		run_recipients_command(cmd, tmpdiff, opt, &recipients);
+
+	string_list_sort(&recipients);
+	string_list_remove_duplicates(&recipients, 0);
+
+	recipients_to_header_buf(header, buf, &recipients);
+
+	string_list_clear(&recipients, 0);
+}
+
 void show_log(struct rev_info *opt)
 {
 	struct strbuf msgbuf = STRBUF_INIT;
 	struct strbuf hdrbuf = STRBUF_INIT;
 	struct log_info *log = opt->loginfo;
+	char *tmpdiff = NULL;
 	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;
@@ -715,6 +880,18 @@ void show_log(struct rev_info *opt)
 	 */
 	graph_show_commit(opt->graph);
 
+	/* Generate dynamic To/Cc lists as needed */
+	if (opt->to_cmd || opt->cc_cmd)
+		tmpdiff = generate_temp_patch(commit);
+
+	headerize_recipients("To", tmpdiff, opt, opt->to_recipients, opt->to_cmd, &hdrbuf);
+	headerize_recipients("Cc", tmpdiff, opt, opt->cc_recipients, opt->cc_cmd, &hdrbuf);
+
+	if (tmpdiff) {
+		unlink_or_warn(tmpdiff);
+		free(tmpdiff);
+	}
+
 	/*
 	 * Print header line of header..
 	 */
@@ -780,8 +957,6 @@ 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);
 
diff --git a/revision.h b/revision.h
index 330d351b2e4c..9611309ae496 100644
--- a/revision.h
+++ b/revision.h
@@ -285,6 +285,8 @@ struct rev_info {
 	int		add_signoff;
 	struct string_list *to_recipients;
 	struct string_list *cc_recipients;
+	const char	*to_cmd;
+	const char	*cc_cmd;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ba5fd0efe2ae..2387bda4f272 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -228,6 +228,25 @@ test_expect_failure 'configuration To: header (rfc2047)' '
 	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
 '
 
+test_expect_success 'dynamic To: header (ascii)' '
+	git config --unset-all format.to &&
+	git format-patch --to-cmd="echo \"R E Cipient <rcipient@example.com>\" #" --stdout main..side >patch10 &&
+	sed -e "/^\$/q" patch10 >hdrs10 &&
+	grep "^To: R E Cipient <rcipient@example.com>\$" hdrs10
+'
+
+test_expect_success 'dynamic To: header (rfc822)' '
+	git format-patch --to-cmd="echo \"R. E. Cipient <rcipient@example.com>\" #" --stdout main..side >patch10 &&
+	sed -e "/^\$/q" patch10 >hdrs10 &&
+	grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" hdrs10
+'
+
+test_expect_success 'dynamic To: header (rfc2047)' '
+	git format-patch --to-cmd="echo \"R Ä Cipient <rcipient@example.com>\" #" --stdout main..side >patch10 &&
+	sed -e "/^\$/q" patch10 >hdrs10 &&
+	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs10
+'
+
 # check_patch <patch>: Verify that <patch> looks like a half-sane
 # patch email to avoid a false positive with !grep
 check_patch () {
-- 
2.39.1.236.ga8a28b9eace8


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] log: Push to/cc handling down into show_log()
  2023-01-19 22:38 ` [PATCH 3/5] log: Push to/cc handling down into show_log() Zev Weiss
@ 2023-01-20  0:33   ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2023-01-20  0:33 UTC (permalink / raw)
  To: Zev Weiss
  Cc: git, brian m. carlson, Ævar Arnfjörð Bjarmason,
	René Scharfe, Denton Liu, Emma Brooks, Eric Sunshine,
	Patrick Hemmer

Zev Weiss <zev@bewilderbeest.net> 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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists
  2023-01-19 22:38 ` [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists Zev Weiss
@ 2023-01-25  2:11   ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2023-01-25  2:11 UTC (permalink / raw)
  To: Zev Weiss
  Cc: git, brian m. carlson, Ævar Arnfjörð Bjarmason,
	Patrick Hemmer

Zev Weiss <zev@bewilderbeest.net> writes:

> Subject: Re: [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists

Style: "log: Refactor" -> "log: refactor"

cf. Documentation/SubmittingPatches[[summary-section]]

> The To and Cc headers are handled identically (the only difference is
> the header name tag), so we might as well reuse the same code for both
> instead of duplicating it.

Makes tons of sense.  But seeing this one ...

> +	recipients_to_header_buf("To", &buf, &extra_to);
> +	recipients_to_header_buf("Cc", &buf, &extra_cc);

... the parameters to the function probably should be ...

> +void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
> +			      const struct string_list *recipients);

... in this order, instead:

    format_recipients(&buf, "To", &extra_to);

That is, "To" and &extra_to are much closely related to each other
than they are to &buf in the sense that they are both input to the
helper function to work on, while &buf is an output buffer, and we
tend to place closer things together, next to each other.

Other than that, removal of repetition is very good.

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/5] pretty: Add name_and_address_only parameter
  2023-01-19 22:38 ` [PATCH 4/5] pretty: Add name_and_address_only parameter Zev Weiss
@ 2023-01-25  2:23   ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2023-01-25  2:23 UTC (permalink / raw)
  To: Zev Weiss; +Cc: git, Emma Brooks, Hamza Mahfooz

Zev Weiss <zev@bewilderbeest.net> writes:

> This is meant to be used with pp_user_info() when using it to format
> email recipients generated by --to-cmd/--cc-cmd.  When set it omits
> the leading 'From: ', trailing linefeed, and the date suffix, and
> additionally will return the input string unmodified if
> split_ident_line() can't parse it (e.g. for a bare email address).

It is somewhat disturbing to see that this only takes effect when
cmit_fmt_is_mail(pp->fmt) and completely ignored otherwise.  Seeing
pp->fmt and pp->name_and_address_only sitting next to each other, it
looks like a layering error.

I wonder if you instead want a new value for pp->fmt that
cmit_fmt_is_mail() considers an e-mail format but is different from
what we used for usual e-mail format?

> diff --git a/pretty.c b/pretty.c
> index 1e1e21878c83..e6798fadc107 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -509,8 +509,11 @@ void pp_user_info(struct pretty_print_context *pp,
>  		return;
>  
>  	line_end = strchrnul(line, '\n');
> -	if (split_ident_line(&ident, line, line_end - line))
> +	if (split_ident_line(&ident, line, line_end - line)) {
> +		if (pp->name_and_address_only)
> +			strbuf_addstr(sb, line);
>  		return;
> +	}

This error handling is also disturbing.  What makes it correct to
parrot the original input that does not parse correctly as an ident
line to the output, only when name_and_address_only bit is on?  It
does not make any sense to do so when cmit_fmt_is_mail(pp->fmt) is
false, especially.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-25  2:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
2023-01-19 22:38 ` [PATCH 1/5] t4014: Add test checking cover-letter To header Zev Weiss
2023-01-19 22:38 ` [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists Zev Weiss
2023-01-25  2:11   ` Junio C Hamano
2023-01-19 22:38 ` [PATCH 3/5] log: Push to/cc handling down into show_log() Zev Weiss
2023-01-20  0:33   ` Junio C Hamano
2023-01-19 22:38 ` [PATCH 4/5] pretty: Add name_and_address_only parameter Zev Weiss
2023-01-25  2:23   ` Junio C Hamano
2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).