Peter Jones [off-list ref] writes:
quoted hunk
This patch adds the ability for "git am" to accept patches in the format
generated by "git show". Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
git-am.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/git-am.sh b/git-am.sh
index c682d34..210e9fe 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+ "commit "*)
+ case "$l2" in
+ "Author: "*)
+ case "$l3" in
+ "Date: "*)
+ patch_format=gitshow
+ ;;
+ *)
+ ;;
+ esac
+ ;;
+ *)
+ ;;
+ esac
+ ;;
At least the inner one could become easier to read by losing one
level of nesting, e.g.
case "$l2,,$l3" in
"Author: *",,"Date: ")
found it
;;
esac
I wonder what the severity of the damage if we misidentify the patch
format in this function would be? If it is severe enough, the check
for the first line may want to become a bit more strict to avoid
misidentification (e.g. expr "$l1" : 'commit [0-9a-f]\{40\}$').
Perhaps we don't care. I dunno.
quoted hunk
@@ -321,6 +336,51 @@ split_patches () {
this=
msgnum=
;;
+ gitshow)
+ this=0
+ for patch in "$@"
+ do
So each input file is expected to be nothing but an output from "git
show" for a single commit; in other words, not concatenation of
them, nor just an e-mail message that has "git show" output
copy&pasted in the body with some other cruft, but plausibly was
delibered as a separate attachment file.
I somehow was visualizing that you were trying to accept mails I
sometimes see here like:
From: somebody
Date: someday
Hi, a long winded discussion that talks about the motivation
behind the patch comes here.
commit 4d8c4db13c8c4c79b6fc0a38ff52d85d3543aa7a
Author: A U Thor [off-list ref]
Date: Tue Sep 11 12:34:56 2012 +0900
a one liner that just says "bugfix" and nothing else
diff --git ....
and that was one of the reasons I thought (but didn't say in my
responses) "Why bother? When running 'am' on such a message you
will have to edit the message to move things around anyway".
If the target is a stand-alone "git show" output, at least we do not
have to worry about such a case.
+ this=`expr "$this" + 1`
+ msgnum=`printf "%0${prec}d" $this`
+ # The first nonemptyline after an empty line is the
+ # subject, and the body starts with the next nonempty
+ # line.
+ perl -ne 'BEGIN {
+ $diff = 0; $subject = 0; $subjtext="";
+ }
+ if ($diff == 1 || /^diff/ || /^---$/) {
+ $diff = 1 ;
+ print ;
+ } elsif ($subject > 1) {
+ s/^ // ;
+ print ;
+ } elsif ($subject == 1 && !/^\s+$/) {
+ s/^ // ;
+ $subjtext = "$subjtext $_";
+ } elsif ($subject == 1) {
+ $subject = 2 ;
+ print "Subject: ", $subjtext ;
+ s/^ // ;
+ print ;
+ } elsif ($subject) {
+ print "\n" ;
+ s/^ // ;
+ print ;
+ } elsif (/^\s+$/) { next ; }
+ elsif (/^Author:/) { s/Author/From/ ; print ;}
+ elsif (/^(From|Date)/) { print ; }
Where does "^From" come from? Should this be /^Date: / instead?
+ elsif (/^commit/) { next ; }
+ else {
+ s/^ // ;
+ $subjtext = $_;
+ $subject = 1;
+ }
+ ' < "$patch" > "$dotest/$msgnum" || clean_abort
+ done
This reminds me of another reason why I am hesitant to make "am"
take output from "git show". Unlike format-patch output, "Author:"
and "Date:" in its output, because it is meant for human
consumption, might be a fair game for i18n/l10n (the first line
"commit [0-9][a-f]{40}" is not likely to change, though).
+ echo "$this" > "$dotest/last"
+ this=
+ msgnum=
+ ;;
hg)
this=0
for hg in "$@"
On Wed, 2012-09-12 at 13:06 -0700, Junio C Hamano wrote:
Peter Jones [off-list ref] writes:
quoted
This patch adds the ability for "git am" to accept patches in the format
generated by "git show". Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
git-am.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/git-am.sh b/git-am.sh
index c682d34..210e9fe 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,21 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+ "commit "*)
+ case "$l2" in
+ "Author: "*)
+ case "$l3" in
+ "Date: "*)
+ patch_format=gitshow
+ ;;
+ *)
+ ;;
+ esac
+ ;;
+ *)
+ ;;
+ esac
+ ;;
At least the inner one could become easier to read by losing one
level of nesting, e.g.
case "$l2,,$l3" in
"Author: *",,"Date: ")
found it
;;
esac
Yeah, I can do that.
I wonder what the severity of the damage if we misidentify the patch
format in this function would be? If it is severe enough, the check
for the first line may want to become a bit more strict to avoid
misidentification (e.g. expr "$l1" : 'commit [0-9a-f]\{40\}$').
Perhaps we don't care. I dunno.
I hadn't really even considered it - are there other formats that use
commit and author which git-am.sh is supposed to support? It seems as
though if we get something wrong you'll wind up in clean_abort at some
point anyway. At worst your patch will still be there and you'll need
to do "git am --abort".
quoted
@@ -321,6 +336,51 @@ split_patches () {
this=
msgnum=
;;
+ gitshow)
+ this=0
+ for patch in "$@"
+ do
So each input file is expected to be nothing but an output from "git
show" for a single commit; in other words, not concatenation of
them, nor just an e-mail message that has "git show" output
copy&pasted in the body with some other cruft, but plausibly was
delibered as a separate attachment file.
I somehow was visualizing that you were trying to accept mails I
sometimes see here like:
From: somebody
Date: someday
Hi, a long winded discussion that talks about the motivation
behind the patch comes here.
commit 4d8c4db13c8c4c79b6fc0a38ff52d85d3543aa7a
Author: A U Thor [off-list ref]
Date: Tue Sep 11 12:34:56 2012 +0900
a one liner that just says "bugfix" and nothing else
diff --git ....
and that was one of the reasons I thought (but didn't say in my
responses) "Why bother? When running 'am' on such a message you
will have to edit the message to move things around anyway".
Yeah, that sounds like madness.
If the target is a stand-alone "git show" output, at least we do not
have to worry about such a case.
Right.
quoted
+ this=`expr "$this" + 1`
+ msgnum=`printf "%0${prec}d" $this`
+ # The first nonemptyline after an empty line is the
+ # subject, and the body starts with the next nonempty
+ # line.
+ perl -ne 'BEGIN {
+ $diff = 0; $subject = 0; $subjtext="";
+ }
+ if ($diff == 1 || /^diff/ || /^---$/) {
+ $diff = 1 ;
+ print ;
+ } elsif ($subject > 1) {
+ s/^ // ;
+ print ;
+ } elsif ($subject == 1 && !/^\s+$/) {
+ s/^ // ;
+ $subjtext = "$subjtext $_";
+ } elsif ($subject == 1) {
+ $subject = 2 ;
+ print "Subject: ", $subjtext ;
+ s/^ // ;
+ print ;
+ } elsif ($subject) {
+ print "\n" ;
+ s/^ // ;
+ print ;
+ } elsif (/^\s+$/) { next ; }
+ elsif (/^Author:/) { s/Author/From/ ; print ;}
+ elsif (/^(From|Date)/) { print ; }
Where does "^From" come from? Should this be /^Date: / instead?
Entirely copy-paste error on my part. I'll fix it for the next version.
quoted
+ elsif (/^commit/) { next ; }
+ else {
+ s/^ // ;
+ $subjtext = $_;
+ $subject = 1;
+ }
+ ' < "$patch" > "$dotest/$msgnum" || clean_abort
+ done
This reminds me of another reason why I am hesitant to make "am"
take output from "git show". Unlike format-patch output, "Author:"
and "Date:" in its output, because it is meant for human
consumption, might be a fair game for i18n/l10n (the first line
"commit [0-9][a-f]{40}" is not likely to change, though).
Well, if that happens, maybe we could regexp match on
"[[:alnum:]_-]+: /someexprthatlookslikeanemailaddress/" ? But we could
also just wait to cross that bridge until we get to it? Even if that
does get translated later, the current patch would continue to work for
English, so even in that case it's not totally worthless.
--
Peter
This patch adds the ability for "git am" to accept patches in the format
generated by "git show". Some people erroneously use "git show" instead
of "git format-patch", and it's nice as a maintainer to be able to
easily take their patch rather than going back and forth with them to
get a "correctly" formatted patch containing exactly the same actual
information.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
git-am.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/git-am.sh b/git-am.sh
index c682d34..d20f249 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -216,6 +216,18 @@ check_patch_format () {
read l2
read l3
case "$l1" in
+ "commit "*)
+ case "$l2,,$l3" in
+ "Author: "*,,"Date: "*)
+ if expr "$l1" : 'commit [0-9a-f]\{40\}$' \
+ >/dev/null ; then
+ patch_format=gitshow
+ fi
+ ;;
+ *)
+ ;;
+ esac
+ ;;
"From "* | "From: "*)
patch_format=mbox
;;@@ -321,6 +333,51 @@ split_patches () {
this=
msgnum=
;;
+ gitshow)
+ this=0
+ for patch in "$@"
+ do
+ this=`expr "$this" + 1`
+ msgnum=`printf "%0${prec}d" $this`
+ # The first nonemptyline after an empty line is the
+ # subject, and the body starts with the next nonempty
+ # line.
+ perl -ne 'BEGIN {
+ $diff = 0; $subject = 0; $subjtext="";
+ }
+ if ($diff == 1 || /^diff/ || /^---$/) {
+ $diff = 1 ;
+ print ;
+ } elsif ($subject > 1) {
+ s/^ // ;
+ print ;
+ } elsif ($subject == 1 && !/^\s+$/) {
+ s/^ // ;
+ $subjtext = "$subjtext $_";
+ } elsif ($subject == 1) {
+ $subject = 2 ;
+ print "Subject: ", $subjtext ;
+ s/^ // ;
+ print ;
+ } elsif ($subject) {
+ print "\n" ;
+ s/^ // ;
+ print ;
+ } elsif (/^\s+$/) { next ; }
+ elsif (/^Author:/) { s/Author/From/ ; print ;}
+ elsif (/^Date:/) { print ; }
+ elsif (/^commit/) { next ; }
+ else {
+ s/^ // ;
+ $subjtext = $_;
+ $subject = 1;
+ }
+ ' < "$patch" > "$dotest/$msgnum" || clean_abort
+ done
+ echo "$this" > "$dotest/last"
+ this=
+ msgnum=
+ ;;
hg)
this=0
for hg in "$@"--
1.7.11.4