# [PATCH] git-am: ignore leading whitespace before patch 10 messages from 2011-08-02 to 2011-08-08. Participants: David Barr, Tay Ray Chuan, Sverre Rabbelier, Junio C Hamano, Jonathan Nieder. Thread: https://gitlist.dev/t/28000 ## David Barr, 2011-08-02 22:20 Subject: [PATCH] git-am: ignore leading whitespace before patch Message-ID: <1312323646-93427-1-git-send-email-davidbarr@google.com> URL: https://gitlist.dev/e/1312323646-93427-1-git-send-email-davidbarr%40google.com ``` Some web-based email clients prepend whitespace to raw message transcripts to workaround content-sniffing in some browsers. Adjust the patch format detection logic to ignore leading whitespace. Signed-off-by: David Barr --- git-am.sh | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/git-am.sh b/git-am.sh index 463c741..19b2f0f 100755 --- a/git-am.sh +++ b/git-am.sh @@ -199,7 +199,11 @@ check_patch_format () { # otherwise, check the first few lines of the first patch to try # to detect its format { - read l1 + # Start from first line containing non-whitespace + until [ -n "$l1" ] + do + read l1 + done read l2 read l3 case "$l1" in -- 1.7.6 ``` ## Tay Ray Chuan, 2011-08-03 12:28 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CALUzUxpn-vCWpTQyB7z9dsu8a%2BUBL9MPjEycOfTmyws5ndz5kA%40mail.gmail.com In-Reply-To: <1312323646-93427-1-git-send-email-davidbarr@google.com> ``` On Wed, Aug 3, 2011 at 6:20 AM, David Barr wrote: > Some web-based email clients prepend whitespace to raw message > transcripts to workaround content-sniffing in some browsers. > Adjust the patch format detection logic to ignore leading > whitespace. > > Signed-off-by: David Barr Finally, patches from GMail that play nice with git-am! Acked-by: Tay Ray Chuan -- Cheers, Ray Chuan ``` ## Sverre Rabbelier, 2011-08-03 13:21 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CAGdFq_it-QAA5uSme6S715dRzHs-s-Uj%3DMWKzBK2MOaaSdiXtg%40mail.gmail.com In-Reply-To: ``` Heya, On Wed, Aug 3, 2011 at 14:28, Tay Ray Chuan wrote: > On Wed, Aug 3, 2011 at 6:20 AM, David Barr wrote: >> Some web-based email clients prepend whitespace to raw message >> transcripts to workaround content-sniffing in some browsers. >> Adjust the patch format detection logic to ignore leading >> whitespace. >> >> Signed-off-by: David Barr > > Finally, patches from GMail that play nice with git-am! So how do you get the patches out of gmail? Do you just copy/paste the output of the "Show original" page? -- Cheers, Sverre Rabbelier ``` ## Tay Ray Chuan, 2011-08-03 13:31 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CALUzUxrubhFpLd00BomM5WwPYNwPbxCx6q7U2TG4PssaQODkZQ%40mail.gmail.com In-Reply-To: ``` On Wed, Aug 3, 2011 at 9:21 PM, Sverre Rabbelier wrote: > So how do you get the patches out of gmail? Do you just copy/paste the > output of the "Show original" page? I Ctrl-S from the "Show original" page. Using Chrome, that yields a mail.txt file. Without this patch, I used to manually remove the first line (all empty whitespace). -- Cheers, Ray Chuan ``` ## Sverre Rabbelier, 2011-08-03 13:33 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CAGdFq_jm3recWwYGow0fZgw6zgwQBurTAPuAyd_qfzHLD6zGbA%40mail.gmail.com In-Reply-To: ``` Heya, On Wed, Aug 3, 2011 at 15:31, Tay Ray Chuan wrote: > On Wed, Aug 3, 2011 at 9:21 PM, Sverre Rabbelier wrote: >> So how do you get the patches out of gmail? Do you just copy/paste the >> output of the "Show original" page? > > I Ctrl-S from the "Show original" page. Using Chrome, that yields a > mail.txt file. Ah, clever hack! Nice :) -- Cheers, Sverre Rabbelier ``` ## David Barr, 2011-08-06 01:56 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CAFfmPPMY5FP8NbZ5Q15pW-NC_qs%3Di6FY7v6Pi8jkMAhkURFTmQ%40mail.gmail.com In-Reply-To: ``` Hi Jonathan, > On Wed, Aug 3, 2011 at 6:20 AM, David Barr wrote: >> Some web-based email clients prepend whitespace to raw message >> transcripts to workaround content-sniffing in some browsers. >> Adjust the patch format detection logic to ignore leading >> whitespace. >> >> Signed-off-by: David Barr >> diff --git a/git-am.sh b/git-am.sh >> index 463c741..19b2f0f 100755 >> --- a/git-am.sh >> +++ b/git-am.sh >> @@ -199,7 +199,11 @@ check_patch_format () { >> # otherwise, check the first few lines of the first patch to try >> # to detect its format >> { >> - read l1 >> + # Start from first line containing non-whitespace >> + until [ -n "$l1" ] >> + do >> + read l1 >> + done >> read l2 >> read l3 >> case "$l1" in On Wed, Aug 3, 2011 at 10:28 PM, Tay Ray Chuan wrote: > Finally, patches from GMail that play nice with git-am! > >  Acked-by: Tay Ray Chuan Do you see any subtle issues in this tiny patch? I failed to include a test, I'll add at least one to the next version. I did check that it doesn't break any of the existing git-am tests. -- David Barr ``` ## Junio C Hamano, 2011-08-06 05:00 Subject: Re: [PATCH] git-am: ignore leading whitespace before patch Message-ID: <7vvcub16e7.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvcub16e7.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` David Barr writes: > Hi Jonathan, > ... >>> diff --git a/git-am.sh b/git-am.sh >>> index 463c741..19b2f0f 100755 >>> --- a/git-am.sh >>> +++ b/git-am.sh >>> @@ -199,7 +199,11 @@ check_patch_format () { >>> # otherwise, check the first few lines of the first patch to try >>> # to detect its format >>> { >>> - read l1 >>> + # Start from first line containing non-whitespace >>> + until [ -n "$l1" ] >>> + do >>> + read l1 >>> + done > ... > Do you see any subtle issues in this tiny patch? > I failed to include a test, I'll add at least one to the next version. > I did check that it doesn't break any of the existing git-am tests. It no longer checks "the first few lines" but can read a lot more, so the comment that precedes this block is now invalid. Also we are rather old fashioned and we never say "until [ ... ]" anywhere in our shell scripts. $ git grep -e until -- '*.sh' Personally to me this is a borderline "Meh", in the sense that I wouldn't bother to waste too much effort rejecting it, as I do not see downsides other than these minor points. Thanks. ``` ## Jonathan Nieder, 2011-08-08 02:49 Subject: [PATCH v2] am: ignore leading whitespace before patch Message-ID: <20110808024904.GF19551@elie.gateway.2wire.net> URL: https://gitlist.dev/e/20110808024904.GF19551%40elie.gateway.2wire.net In-Reply-To: <7vvcub16e7.fsf@alter.siamese.dyndns.org> ``` From: David Barr Some web-based email clients prepend whitespace to raw message transcripts to workaround content-sniffing in some browsers. Adjust the patch format detection logic to ignore leading whitespace. So now you can apply patches from GMail with "git am" in three steps: 1. choose "show original" 2. tell the browser to "save as" (for example by pressing Ctrl+S) 3. run "git am" on the saved file This fixes a regression introduced by v1.6.4-rc0~15^2~2 (git-am foreign patch support: autodetect some patch formats, 2009-05-27). GMail support was first introduced to "git am" by v1.5.4-rc0~274^2 (Make mailsplit and mailinfo strip whitespace from the start of the input, 2007-11-01). Signed-off-by: David Barr Acked-by: Tay Ray Chuan Signed-off-by: Jonathan Nieder --- Junio C Hamano wrote: > It no longer checks "the first few lines" but can read a lot more, so the > comment that precedes this block is now invalid. > > Also we are rather old fashioned and we never say "until [ ... ]" anywhere > in our shell scripts. Good ideas, thanks. While at it, let's initialize l1 to protect against any stray value it might have inherited from the environment. Looking forward to the promised test, :) Jonathan git-am.sh | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/git-am.sh b/git-am.sh index 463c741d..c8422dbe 100755 --- a/git-am.sh +++ b/git-am.sh @@ -196,10 +196,15 @@ check_patch_format () { return 0 fi - # otherwise, check the first few lines of the first patch to try - # to detect its format + # otherwise, check the first few non-blank lines of the first + # patch to try to detect its format { - read l1 + # Start from first line containing non-whitespace + l1= + while test -z "$l1" + do + read l1 + done read l2 read l3 case "$l1" in -- 1.7.6 ``` ## David Barr, 2011-08-08 05:10 Subject: RE: [PATCH v2] am: ignore leading whitespace before patch Message-ID: <1312780242-91659-1-git-send-email-davidbarr@google.com> URL: https://gitlist.dev/e/1312780242-91659-1-git-send-email-davidbarr%40google.com In-Reply-To: <20110808024904.GF19551@elie.gateway.2wire.net> ``` Add a test for GMail-style padded email files. Signed-off-by: David Barr --- t/t4150-am.sh | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 151404e..40a5a3e 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -167,6 +167,17 @@ test_expect_success 'am applies patch e-mail not in a mbox with CRLF' ' test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)" ' +test_expect_success 'am applies patch e-mail with preceding whitespace' ' + rm -fr .git/rebase-apply && + git reset --hard && + git checkout first && + printf "%256s\\n" "" >patch1-ws.eml && + cat patch1.eml >>patch1-ws.eml && + git am output.out 2>&1 && + ! test -d .git/rebase-apply && + git diff --exit-code second +' + test_expect_success 'setup: new author and committer' ' GIT_AUTHOR_NAME="Another Thor" && GIT_AUTHOR_EMAIL="a.thor@example.com" && -- 1.7.6 ``` ## David Barr, 2011-08-08 05:31 Subject: Re: [PATCH v2] am: ignore leading whitespace before patch Message-ID: URL: https://gitlist.dev/e/CAFfmPPPpYDA39U9UYKojj90fST40voe%3DdgBi3QPjQdhBT89NmQ%40mail.gmail.com In-Reply-To: <1312780242-91659-1-git-send-email-davidbarr@google.com> ``` *facepalm* This test already passes: > +       git am output.out 2>&1 && Alternatively, the following was failing: > +       git am patch1-ws.eml >output.out 2>&1 && Note that the email file is passed as an argument rather than a redirect. -- David Barr ```