diff mbox series

[4/5] pretty: Add name_and_address_only parameter

Message ID 20230119223858.29262-5-zev@bewilderbeest.net (mailing list archive)
State New, archived
Headers show
Series format-patch: Add --{to,cc}-cmd support | expand

Commit Message

Zev Weiss Jan. 19, 2023, 10:38 p.m. UTC
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(-)

Comments

Junio C Hamano Jan. 25, 2023, 2:23 a.m. UTC | #1
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.
Calvin Wan Feb. 1, 2023, 11:52 p.m. UTC | #2
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.
diff mbox series

Patch

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;