Re: [PATCH v3] git-am: fix maildir support regression: accept email file as patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nicolas Sebrecht <nicolas.s.dev@xxxxxx> writes:

> Patch format detection introduced by a5a6755a1d4707bf2fab7752e5c974ebf63d086a
> may refuse valid patches.

This message is even worse than the previous round.

By definition what git-am does not accept is invalid, and you are
loosening that definition to include something else newly as valid.

Please describe what that new something is.

If you are claiming a5a6755 (git-am foreign patch support: introduce
patch_format, 2009-05-27) is an regression, and before it X, Y and Z
format were supported but after that only X and Y format are, then for the
same reason please specify what that Z you are resurrecting the support
for is.

That way, people who have been frustrated that their randomly formatted
files were not processible without first converting them to mbox format
will know that now their favorite format is now/again also supported.

> We keep detection on the first three lines. Emails may have:
>  - header fields in a random order;
>  - folded lines.
>
> Signed-off-by: Nicolas Sebrecht <nicolas.s.dev@xxxxxx>
> ---
>  git-am.sh |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
>

Please do not break the thread.  Make this message a response to my
message you lifted the idea from, which in turn was sent as a response in
the thread of v2 patch.


> +		for line in "$l1" "$l2" "$l3"
> +		do
> +			printf "$line" |
> +				# The line may be a folded line
> +				sed -e '/^$/q' -e '/^[ ]/d' |
> +				grep -E -e '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
> +				is_email='false'

Running three independent printf piped to two processes in a loop is
quite silly.

I think you did not understand the point of the three liner I sent you.

	sed -e '/^$/q' -e '/^[ 	]/d' "$1" |

    The point of this is not to use the silly "we only look at the first
    three lines" rule.  Instead, it ignores these l1/l2/l3, but grabs all
    the header lines, but discards second and subsequent physical lines if
    a logical line was folded.  Which means that the effect of this is to
    pipe the whole header (again, without worrying about the indented
    remainder of folded lines) to downsream, which is the grep -v below

        grep -v -E -e '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||

    This checks if there is a line that does _NOT_ match the usual
    e-mail header pattern.  If there is such a line, it means that the
    file is not an e-mail message.  If there is no such line, we say...

        patch_format=mbox

One caveat is that the above logic alone won't catch a random file that
does not have _any_ e-mail headers in it.  So you might need to do
something like:

	LF='
        '
	case "$l1$LF$l2$LF$l3" in
        *"$LF$LF"*)
        	# has a completely empty line in there?
                # that means the message has only two headers at most;
                # that cannot be an email.
		;;
	*)
                sed -e '/^$/q' -e '/^[ 	]/d' "$1" |
                grep -v -E -e '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
                patch_format=mbox
	esac

to completely replace that "for line in..." loop.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]