* [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-02-01 23:52 ` Calvin Wan
2023-01-19 22:38 ` [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists Zev Weiss
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ 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] 14+ 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
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ 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] 14+ 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; 14+ 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] 14+ 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-02-01 23:52 ` Calvin Wan
2023-01-19 22:38 ` [PATCH 4/5] pretty: Add name_and_address_only parameter Zev Weiss
` (2 subsequent siblings)
5 siblings, 2 replies; 14+ 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(¬ebuf, 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] 14+ 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
2023-02-01 23:52 ` Calvin Wan
1 sibling, 0 replies; 14+ 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] 14+ 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
@ 2023-02-01 23:52 ` Calvin Wan
1 sibling, 0 replies; 14+ messages in thread
From: Calvin Wan @ 2023-02-01 23:52 UTC (permalink / raw)
To: Zev Weiss
Cc: Calvin Wan, git, brian m. carlson,
Ævar Arnfjörð Bjarmason, René Scharfe,
Denton Liu, Emma Brooks, Eric Sunshine, Junio C Hamano,
Patrick Hemmer
> 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(¬ebuf, 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?
^ permalink raw reply [flat|nested] 14+ 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-02-01 23:52 ` Calvin Wan
2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss
2023-02-01 23:52 ` [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Calvin Wan
5 siblings, 2 replies; 14+ 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] 14+ 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
2023-02-01 23:52 ` Calvin Wan
1 sibling, 0 replies; 14+ 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] 14+ 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
@ 2023-02-01 23:52 ` Calvin Wan
1 sibling, 0 replies; 14+ messages in thread
From: Calvin Wan @ 2023-02-01 23:52 UTC (permalink / raw)
To: Zev Weiss; +Cc: Calvin Wan, git, Emma Brooks, Hamza Mahfooz, Junio C Hamano
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).
I think the ideal solution, instead of choosing this hacky approach, is
to refactor out the common functionality you need from pp_user_info()
and to call that instead in your next patch.
^ permalink raw reply [flat|nested] 14+ 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
2023-02-01 23:52 ` Calvin Wan
2023-02-01 23:52 ` [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Calvin Wan
5 siblings, 1 reply; 14+ 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(¬ebuf, 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] 14+ messages in thread
* Re: [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags
2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss
@ 2023-02-01 23:52 ` Calvin Wan
0 siblings, 0 replies; 14+ messages in thread
From: Calvin Wan @ 2023-02-01 23:52 UTC (permalink / raw)
To: Zev Weiss; +Cc: Calvin Wan, git, Denton Liu, Eric Sunshine, Junio C Hamano
Zev Weiss <zev@bewilderbeest.net> writes:
> 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.
Commit message could use some more explanation to make blame diving
easier later on. In particular, why is creating temp files necessary?
Missing update to Documentation/git-format-patch.txt
This patch also needs many more tests
- tests for --cc-cmd
- tests for commands that run on the patches
- similar tests in t9001-send-email.sh
Will wait for reroll to dive deeper into the implementation here, but
overall a well-intentioned v1!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] format-patch: Add --{to,cc}-cmd support
2023-01-19 22:38 [PATCH 0/5] format-patch: Add --{to,cc}-cmd support Zev Weiss
` (4 preceding siblings ...)
2023-01-19 22:38 ` [PATCH 5/5] format-patch: Add support for --{to,cc}-cmd flags Zev Weiss
@ 2023-02-01 23:52 ` Calvin Wan
5 siblings, 0 replies; 14+ messages in thread
From: Calvin Wan @ 2023-02-01 23:52 UTC (permalink / raw)
To: Zev Weiss
Cc: Calvin Wan, git, Junio C Hamano, Eric Sunshine, brian m. carlson,
Ævar Arnfjörð Bjarmason, Patrick Hemmer,
René Scharfe, Denton Liu, Emma Brooks, Hamza Mahfooz
Hi Zev! Thanks for the patch series. I agree that these flags are a very
good idea to add. I appreciate the well written cover letter -- it does
well describing the need for this series and calling out design
questions. I'll leave the rest of my comments in the particular patches,
looking forward to the reroll!
> - 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).
It is also inconsistent with git-format-patch --to and --cc. I strongly
believe that your flags should also work with the cover letter, since
users should expect equivalency across similar flags. git-send-email has
config options, sendemail.tocmd and sendemail.cccmd, that I also believe
should be implemented.
^ permalink raw reply [flat|nested] 14+ messages in thread