Re: [PATCH v6] mailinfo: allow e-mail files as input

[Junio C Hamano <gitster@pobox.com>]

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> We traditionally allowed a mbox file or a directory name of a maildir to be
> ...
> Signed-off-by: Nicolas Sebrecht <nicolas.s.dev@gmx.fr>

Thanks.

I have one more comment on the test script, but it's something I can
locally fix (iow, there is no need to resend your patch if there is no
other issue pointed out by others, and if you agree to my suggested
improvements).

> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> index a12bf84..4c99240 100755
> --- a/t/t4150-am.sh
> +++ b/t/t4150-am.sh
> @@ -63,6 +63,53 @@ with the data reset to initial values.
>  
>  EOF
>  
> +cat >rfc2822_email <<EOF
> +Return-Path: <user@domain.name>
> +X-Flags: 0000
> +	999

Please quote that EOF if you do not mean to have any $variable substituted
in the "here document", like this:

	cat >rfc2822_email <<\EOF
        ... here document comes here ...
	EOF

Otherwise reviewers need to waste time looking for what is being replaced
in the huge here document to understand what is going on.

> +Delivered-To: delivery to user@domain.name
> +Received: (qmail invoked by alias); 16 Jul 2009 05:25:49 -0000
> +Received: from vger.knl.xyz (EHLO vger.knl.xyz) [4.3.2.1]
> +  by mx0.gmx.com (mx-us004) with SMTP; 16 Jul 2009 01:25:49 -0400
> ...
> +
> +EOF
> +

The headers look a bit too excessive to my taste, but probably you wanted
to take a real-life example.  If that is the case, I suspect the manually
added X-Flags: at the beginning defeats that purpose, though.  I'd suggest
either removing the hand-munging, or triming the Received: sequence to
make it a bit shorter.

> @@ -222,6 +269,13 @@ test_expect_success 'am takes patches from a Pine mailbox' '
>  	test -z "$(git diff master^..HEAD)"
>  '
>  
> +test_expect_success 'am takes patches from a RFC2822 formated email' '
> +	git checkout first &&
> +	cat rfc2822_email patch1 | git am &&
> +	! test -d .git/rebase-apply &&
> +	test -z "$(git diff master^..HEAD)"
> +'

These days we tend to write the last step

	git diff --exit-code master^ HEAD

which allows "sh t4150-am.sh -i -v" to be more useful when debugging.  But
the existing tests in this script are old fashioned, and you matched their
style in this patch, which is very good.

We probably would want a separate patch to modernize them after 1.6.4,
though.