{"thread":{"id":"19773","subject":"[PATCH] git-format-patch, git-send-email: generate/handle escaped >From","startedAt":"2009-06-11T10:00:34Z","lastAt":"2009-06-28T07:52:12Z","messageCount":4,"participants":["Paolo Bonzini","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"116065","messageId":"1244714434-20794-1-git-send-email-bonzini@gnu.org","threadId":"19773","inReplyTo":null,"subject":"[PATCH] git-format-patch, git-send-email: generate/handle escaped >From","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-06-11T10:00:34Z","receivedAt":"2009-06-11T10:00:34Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"I noticed that the mbox files generated by git-format-patch (especially\nwith --stdout) are not proper in the sense that the lines starting with\n\"From \" are not escaped with a > sign.  This is unlikely to cause problems\nwith mail clients such as mutt, but many scripts designed to work on\nmbox files will fumble in this case.\n\nThis patch fixes it and dually unescapes the lines in git-send-email.\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n---\n git-send-email.perl     |    1 +\n pretty.c                |    2 ++\n t/t4014-format-patch.sh |   13 +++++++++++++\n t/t9001-send-email.sh   |   15 +++++++++++++++\n 4 files changed, 31 insertions(+), 0 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 4c795a4..de57303 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1085,6 +1085,7 @@ foreach my $t (@files) {\n \t}\n \t# Now parse the message body\n \twhile(<F>) {\n+\t\ts/^>From /From /;\n \t\t$message .=  $_;\n \t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n \t\t\tchomp;\ndiff --git a/pretty.c b/pretty.c\nindex e5328da..d7f8228 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -868,6 +868,8 @@ void pp_remainder(enum cmit_fmt fmt,\n \t\t\tmemset(sb->buf + sb->len, ' ', indent);\n \t\t\tstrbuf_setlen(sb, sb->len + indent);\n \t\t}\n+\t\tif (fmt == CMIT_FMT_EMAIL && !prefixcmp(line, \"From \"))\n+\t\t\tstrbuf_addch(sb, '>');\n \t\tstrbuf_add(sb, line, linelen);\n \t\tstrbuf_addch(sb, '\\n');\n \t}\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 922a894..8474718 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -28,6 +28,13 @@ test_expect_success setup '\n \tgit update-index file &&\n \tgit commit -m \"Side changes #3 with \\\\n backslash-n in it.\" &&\n \n+\tgit checkout -b more &&\n+\tfor i in A B C; do echo \"$i\"; done >>file &&\n+\tgit update-index file &&\n+\tgit commit -m \"More changes\n+\n+From this point on...\" &&\n+\n \tgit checkout master &&\n \tgit diff-tree -p C2 | git apply --index &&\n \tgit commit -m \"Master accepts moral equivalent of #2\"\n@@ -516,4 +523,10 @@ test_expect_success 'format-patch --signoff' '\n \tgrep \"^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n '\n \n+test_expect_success 'format-patch escapes From' '\n+\tgit format-patch more^..more --stdout > patch9 &&\n+\tgrep \"^>From this\" patch9 &&\n+\t! grep \"^From this\" patch9\n+'\n+\n test_done\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 2ce24cd..5c6f0f4 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -168,6 +168,21 @@ test_expect_success 'no patch was sent' '\n \t! test -e commandline1\n '\n \n+test_expect_success 'fix >From lines' '\n+\tclean_fake_sendmail &&\n+\tcp $patches gt-from.patch &&\n+\techo \">From abcdef\" >>gt-from.patch &&\n+\tgit send-email \\\n+\t\t--from=\"Example <nobody@example.com>\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\tgt-from.patch \\\n+\t\t2>errors &&\n+\ttest -e msgtxt1 &&\n+\t! grep \"^>From abcdef\" msgtxt1 &&\n+\tgrep \"^From abcdef\" msgtxt1\n+'\n+\n test_expect_success 'Author From: in message body' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n-- \n1.6.0.3\n"},{"id":"116067","messageId":"20090611105538.GA4409@coredump.intra.peff.net","threadId":"19773","inReplyTo":"1244714434-20794-1-git-send-email-bonzini@gnu.org","subject":"Re: [PATCH] git-format-patch, git-send-email: generate/handle escaped >From","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-11T10:55:39Z","receivedAt":"2009-06-11T10:55:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 11, 2009 at 12:00:34PM +0200, Paolo Bonzini wrote:\n\n> I noticed that the mbox files generated by git-format-patch (especially\n> with --stdout) are not proper in the sense that the lines starting with\n> \"From \" are not escaped with a > sign.  This is unlikely to cause problems\n> with mail clients such as mutt, but many scripts designed to work on\n> mbox files will fumble in this case.\n> \n> This patch fixes it and dually unescapes the lines in git-send-email.\n\nUgh. Can we please at least make this optional?  Many modern MTAs handle\nthe current situation just fine by being much more strict in their\n\"From\" matching (i.e., you would need something that really looks like a\nvalid \"From\" line to get a match), and do not do \">From\" de-quoting at\nall (mutt is such an example).\n\nSome even take it a step farther and use Content-Length headers, though\nI do not think that is a good idea here (we encourage people to munge\nthe contents while stored as an mbox).\n\n> +++ b/pretty.c\n> @@ -868,6 +868,8 @@ void pp_remainder(enum cmit_fmt fmt,\n>  \t\t\tmemset(sb->buf + sb->len, ' ', indent);\n>  \t\t\tstrbuf_setlen(sb, sb->len + indent);\n>  \t\t}\n> +\t\tif (fmt == CMIT_FMT_EMAIL && !prefixcmp(line, \"From \"))\n> +\t\t\tstrbuf_addch(sb, '>');\n\nThis is the lossy \"mboxo\" conversion. A quoted From is now\nindistinguishable from an original \">From\". To be reversible, you need\nto quote \"s/^>*From />&/\".\n\nBut of course, which conversion you want depends entirely on what you\nare going to feed it to, and which mbox they are expecting. Which is why\nit really should be configurable.\n\nYour use case looks to be feeding it to send-email; I suspect you would\nbe better served by improving the 'From ' detection in send-email,\nprobably to something like:\n\n  /^From \\S+ \\w{3} \\w{3} \\d+ \\d\\d:\\d\\d:\\d\\d \\d+/\n\nthough that may be too strict. It would probably make sense to steal one\nfrom one of the many mbox-reading CPAN modules.\n\n-Peff\n"},{"id":"116070","messageId":"4A30F16C.5040207@gmail.com","threadId":"19773","inReplyTo":"20090611105538.GA4409@coredump.intra.peff.net","subject":"Re: [PATCH] git-format-patch, git-send-email: generate/handle escaped >From","fromName":"Paolo Bonzini","fromEmail":"paolo.bonzini@gmail.com","sentAt":"2009-06-11T11:58:36Z","receivedAt":"2009-06-11T11:58:36Z","isPatch":true,"sender":{"key":"paolo.bonzini@gmail.com","avatar":"https://gravatar.com/avatar/7817ef2e168b4ef0570c5bb5bdc1d4b44f34d3075fe32b871710dd942d0a89f5?d=mp&s=160"},"body":"\n> But of course, which conversion you want depends entirely on what you\n> are going to feed it to, and which mbox they are expecting. Which is why\n> it really should be configurable.\n> \n> Your use case looks to be feeding it to send-email\n\nAlmost, :-) because git-send-email does not support sending multiple \nmessages out of one mbox file.  I have an upcoming patch to add this \nsupport, and I was just \"feeling the waters\" before posting it.\n\n> I suspect you would\n> be better served by improving the 'From ' detection in send-email,\n> probably to something like:\n> \n>   /^From \\S+ \\w{3} \\w{3} \\d+ \\d\\d:\\d\\d:\\d\\d \\d+/\n> \n> though that may be too strict. It would probably make sense to steal one\n> from one of the many mbox-reading CPAN modules.\n\nGood idea.  I'll let others speak and can make this configurable (as \nwell as use the s/^>*From />&/ conversion), but if no one speaks, I'll \nchange my git-send-email patch to use the above regex or a similar one, \nand withdraw this patch.\n\nThanks for the prompt remark!\n\nPaolo\n"},{"id":"117117","messageId":"4A47212C.5070000@gmail.com","threadId":"19773","inReplyTo":"4A30F16C.5040207@gmail.com","subject":"Re: [PATCH] git-format-patch, git-send-email: generate/handle escaped >From","fromName":"Paolo Bonzini","fromEmail":"paolo.bonzini@gmail.com","sentAt":"2009-06-28T07:52:12Z","receivedAt":"2009-06-28T07:52:12Z","isPatch":true,"sender":{"key":"paolo.bonzini@gmail.com","avatar":"https://gravatar.com/avatar/7817ef2e168b4ef0570c5bb5bdc1d4b44f34d3075fe32b871710dd942d0a89f5?d=mp&s=160"},"body":"Paolo Bonzini wrote:\n> \n>> But of course, which conversion you want depends entirely on what you\n>> are going to feed it to, and which mbox they are expecting. Which is why\n>> it really should be configurable.\n>>\n>> Your use case looks to be feeding it to send-email\n> \n> Almost, :-) because git-send-email does not support sending multiple \n> messages out of one mbox file.  I have an upcoming patch to add this \n> support, and I was just \"feeling the waters\" before posting it.\n\nI decided that it's easier to support sending a single mbox file with \nthis alias instead:\n\nsend-mbox = \"!bash -ec 'eval f=\\\\$$#; eval set -- `seq -f\\\"\\\\$%.0f\\\" 1 \n$(($#-1))`; if last=`mkdir .mboxsplit && git mailsplit -d4 -o.mboxsplit \n-b -- \\\"$f\\\"`; then echo Found $last messages in \\\"$f\\\"; git send-email \n\\\"$@\\\" .mboxsplit && rm -rf .mboxsplit; else rm -rf .mboxsplit; fi' -\"\n\nSo, patch withdrawn.  I don't think I'll post the patches in my \nmailsplit-with-sendemail branch.\n\nPaolo\n"}]}