From: Junio C Hamano <gitster@pobox.com>
To: Nicolas Sebrecht <nicolas.s.dev@gmx.fr>
Cc: <git@vger.kernel.org>, Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Subject: Re: [PATCH v3] git-am: fix maildir support regression: accept email file as patch
Date: Wed, 15 Jul 2009 15:54:25 -0700 [thread overview]
Message-ID: <7v1voheevy.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1654b578a7e045b564c46df4abb6e2196422c2b2.1247696294.git.nicolas.s.dev@gmx.fr> (Nicolas Sebrecht's message of "Thu\, 16 Jul 2009 00\:19\:36 +0200")
Nicolas Sebrecht <nicolas.s.dev@gmx.fr> 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@gmx.fr>
> ---
> 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.
next prev parent reply other threads:[~2009-07-15 22:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-15 22:19 [PATCH v3] git-am: fix maildir support regression: accept email file as patch Nicolas Sebrecht
2009-07-15 22:43 ` [PATCH v3] " Nicolas Sebrecht
2009-07-15 22:54 ` Junio C Hamano [this message]
2009-07-15 23:56 ` [PATCH v3] " Junio C Hamano
2009-07-16 1:00 ` [PATCH v3] " Nicolas Sebrecht
2009-07-16 2:06 ` Nicolas Sebrecht
2009-07-16 2:30 ` Junio C Hamano
2009-07-16 2:59 ` Nicolas Sebrecht
2009-07-16 0:49 ` Nicolas Sebrecht
2009-07-16 2:41 ` Junio C Hamano
2009-07-16 4:05 ` [PATCH v4] git-am: allow e-mail file(s) as input Nicolas Sebrecht
2009-07-16 4:10 ` [PATCH v4] " Nicolas Sebrecht
2009-07-16 5:23 ` [PATCH v5] " Nicolas Sebrecht
2009-07-16 7:09 ` Stephen Boyd
2009-07-16 7:24 ` Junio C Hamano
2009-07-16 7:50 ` [PATCH v5] " Nicolas Sebrecht
2009-07-16 8:06 ` Nicolas Sebrecht
2009-07-16 8:17 ` Johannes Sixt
2009-07-16 8:12 ` Johannes Sixt
2009-07-16 17:45 ` [PATCH v6] mailinfo: allow e-mail files " Nicolas Sebrecht
2009-07-17 1:05 ` Junio C Hamano
2009-07-17 2:20 ` [PATCH v6] " Nicolas Sebrecht
2009-07-17 10:06 ` [PATCH v6] " Nanako Shiraishi
2009-07-17 19:54 ` Junio C Hamano
2009-07-17 22:04 ` [PATCH v6] " Nicolas Sebrecht
2009-08-06 17:07 ` [PATCH v7] " Nicolas Sebrecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7v1voheevy.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=giuseppe.bilotta@gmail.com \
--cc=nicolas.s.dev@gmx.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).