From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sun, 24 Jul 2016 07:37:57 GMT
Subject: Re: [PATCH 2/2] format-patch: escape "From " lines recognized by mailsplit
Message-ID: <alpine.DEB.2.20.1607240926500.14111@virtualbox>
In-Reply-To: <20160724031543.GC32480@starla>

Hi Eric,

On Sun, 24 Jul 2016, Eric Wong wrote:

> @@ -1745,9 +1746,18 @@ void pp_remainder(struct pretty_print_context *pp,
>  			strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
>  					     line, linelen);
>  		else {
> -			if (pp->fmt == CMIT_FMT_MBOXRD &&
> -					is_mboxrd_from(line, linelen))
> -				strbuf_addch(sb, '>');
> +			switch (pp->fmt) {
> +			case CMIT_FMT_EMAIL:
> +				if (is_from_line(line, linelen))
> +					strbuf_addch(sb, '>');
> +				break;
> +			case CMIT_FMT_MBOXRD:
> +				if (is_mboxrd_from(line, linelen))
> +					strbuf_addch(sb, '>');
> +				break;
> +			default:
> +				break;
> +			}

Sorry to be nitpicking once again; I think this would be conciser (and
easier to read at least for me) as:

-			if (pp->fmt == CMIT_FMT_MBOXRD &&
-					is_mboxrd_from(line, linelen))
+			if ((pp->fmt == CMIT_FMT_MBOXRD &&
+			     is_mboxrd_from(line, linelen)) ||
+			    (pp->fmt == CMIT_FMT_EMAIL &&
+			     is_from_line(line, linelen)))
 				strbuf_addch(sb, '>');

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 1206c48..8fa3982 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1606,4 +1606,18 @@ test_expect_success 'format-patch --pretty=mboxrd' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'format-patch From escaping' '
> +	cat >msg <<-INPUT_END &&
> +	somebody pasted format-patch output into a body
> +
> +	From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> +	INPUT_END
> +
> +	C=$(git commit-tree HEAD^^{tree} -p HEAD <msg) &&

The double caret makes this a bit hard to read. Maybe this instead?

+	C=$(git commit-tree HEAD: -p HEAD^ <msg) &&

> +	git format-patch --stdout -1 $C~1..$C >patch &&

Either "-1 $C" or "$C~1..$C", not both...

> +	git grep -h --no-index \
> +		">From 0000000000000000000000000000000000000000 " \
> +		patch
> +'
> +
>  test_done
> -- 
> EW

Heh, that's a nice Git version ;-)

Ciao,
Dscho