Re: [PATCH 2/2] format-patch: --inline-single

[Jeff King <peff@peff.net>]

On Thu, Feb 21, 2013 at 12:26:22PM -0800, Junio C Hamano wrote:

> Some people may find it convenient to append a simple patch at the
> bottom of a discussion e-mail separated by a "scissors" mark, ready
> to be applied with "git am -c".  Introduce "--inline-single" option
> to format-patch to do so.  A typical usage example might be to start
> 'F'ollow-up to a discussion, write your message, conclude with "a
> patch to do so may look like this.", and
> 
>     \C-u M-! git format-patch --inline-single -1 HEAD <ENTER>
> 
> if you are an Emacs user.  Users of other MUA's may want to consult
> their manuals to find equivalent command to append output from an
> external command to the message being composed.

Interesting. I usually just do this by hand, but this could save a few
keystrokes in my workflow.

> +static int is_current_user(const struct pretty_print_context *pp,
> +			   const char *email, size_t emaillen,
> +			   const char *name, size_t namelen)
> +{
> +	const char *me = git_committer_info(0);
> +	const char *myname, *mymail;
> +	size_t mynamelen, mymaillen;
> +	struct ident_split ident;
> +
> +	if (split_ident_line(&ident, me, strlen(me)))
> +		return 0; /* play safe, as we do not know */
> +	mymail = ident.mail_begin;
> +	mymaillen = ident.mail_end - ident.mail_begin;
> +	myname = ident.name_begin;
> +	mynamelen = ident.name_end - ident.name_begin;
> +	if (pp->mailmap)
> +		map_user(pp->mailmap, &mymail, &mymaillen, &myname, &mynamelen);
> +	return (mymaillen == emaillen &&
> +		mynamelen == namelen &&
> +		!memcmp(mymail, email, emaillen) &&
> +		!memcmp(myname, name, namelen));
> +}

Nice, I'm glad you handled this case properly. I've wondered if we
should have an option to do a similar test when writing out the "real"
message format. I.e., to put the extra "From" line in the body of the
message when !is_current_user(). Traditionally we have just said "that
is the responsibility of the MUA you use", and let send-email handle it.
But it means people who do not use send-email have to reimplement the
feature themselves.

> @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
>  	if (pp->mailmap)
>  		map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
>  
> +	if (pp->inline_single && is_current_user(pp, mailbuf, maillen, namebuf, namelen))
> +		return;
> +
>  	strbuf_init(&mail, 0);
>  	strbuf_init(&name, 0);

This makes sense to suppress the user line when it is not necessary. But
we should probably always be suppressing the Date line, as it is almost
always useless.

I also wonder if we should suppress the subject-prefix in such a case,
as it is not adding anything (it is not the subject of the email, so it
does not need to grab attention there, and it will not make it into the
final commit). On the other hand, having tried it, the "Subject:" looks
a little lonely without it. Perhaps the [PATCH] is still necessary to
grab attention after the scissors line. I dunno.

Patch for both below if you want to pick up either suggestion.

diff --git a/log-tree.c b/log-tree.c
index 15c9749..8994354 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -348,7 +348,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			 digits_in_number(opt->total),
 			 opt->nr, opt->total);
 		subject = buffer;
-	} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
+	} else if (opt->total == 0 && !opt->inline_single &&
+		   opt->subject_prefix && *opt->subject_prefix) {
 		static char buffer[256];
 		snprintf(buffer, sizeof(buffer),
 			 "Subject: [%s] ",
diff --git a/pretty.c b/pretty.c
index 363b3d9..1a7352c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -490,7 +490,8 @@ void pp_user_info(const struct pretty_print_context *pp,
 		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, pp->date_mode));
 		break;
 	case CMIT_FMT_EMAIL:
-		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
+		if (!pp->inline_single)
+			strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
 		break;
 	case CMIT_FMT_FULLER:
 		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, pp->date_mode));