Re: [PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
On Sun, Jan 27, 2013 at 6:51 PM, Jonathan Nieder wrote: > Jonathan Nieder wrote: > >> Here's the tweak I suggested last time. I think its behavior is >> slightly better in the "ends with incomplete line" case because it >> limits the characters examined by is_rfc2822_line() and >> is_cherry_picked_from_line() not to include buf[len] (which would >> presumably sometimes be '\0'). > > Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents > exiting the loop in is_rfc2822_line when the line does not contain a > colon. Here's a corrected version of the tweak, that should actually > work. :) > > diff --git i/sequencer.c w/sequencer.c > index 0b5cd18c..108ea27b 100644 > --- i/sequencer.c > +++ w/sequencer.c > @@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len) > for (i = 0; i < len; i++) { > int ch = buf[i]; > if (ch == ':') > + return 1; > + if (!isalnum(ch) && ch != '-') > break; > - if (isalnum(ch) || (ch == '-')) > - continue; > - return 0; > } > - > - return 1; > + return 0; > } > > static int is_cherry_picked_from_line(const char *buf, int len) > @@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf, > int len) > /* > * We only care that it looks roughly like (cherry picked from ...) > */ > - return !prefixcmp(buf, cherry_picked_prefix) && > - (buf[len - 1] == ')' || > -(buf[len - 1] == '\n' && buf[len - 2] == ')')); > + return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; > } > > static int has_conforming_footer(struct strbuf *sb, int ignore_footer) > @@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int > ignore_footer) > ; /* do nothing */ > k++; > > - if (!(is_rfc2822_line(buf + i, k - i) || > - is_cherry_picked_from_line(buf + i, k - i))) > + if (!is_rfc2822_line(buf + i, k - i - 1) && > + !is_cherry_picked_from_line(buf + i, k - i - 1)) > return 0; > } > return 1; Looks good to me. -Brandon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Jonathan Nieder wrote: > Here's the tweak I suggested last time. I think its behavior is > slightly better in the "ends with incomplete line" case because it > limits the characters examined by is_rfc2822_line() and > is_cherry_picked_from_line() not to include buf[len] (which would > presumably sometimes be '\0'). Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents exiting the loop in is_rfc2822_line when the line does not contain a colon. Here's a corrected version of the tweak, that should actually work. :) diff --git i/sequencer.c w/sequencer.c index 0b5cd18c..108ea27b 100644 --- i/sequencer.c +++ w/sequencer.c @@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len) for (i = 0; i < len; i++) { int ch = buf[i]; if (ch == ':') + return 1; + if (!isalnum(ch) && ch != '-') break; - if (isalnum(ch) || (ch == '-')) - continue; - return 0; } - - return 1; + return 0; } static int is_cherry_picked_from_line(const char *buf, int len) @@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf, int len) /* * We only care that it looks roughly like (cherry picked from ...) */ - return !prefixcmp(buf, cherry_picked_prefix) && - (buf[len - 1] == ')' || -(buf[len - 1] == '\n' && buf[len - 2] == ')')); + return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; } static int has_conforming_footer(struct strbuf *sb, int ignore_footer) @@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - if (!(is_rfc2822_line(buf + i, k - i) || - is_cherry_picked_from_line(buf + i, k - i))) + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) return 0; } return 1; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Brandon Casey wrote: > Let's detect "(cherry picked from...)" as part of the footer so that we > will produce this: > >Signed-off-by: A U Thor >(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) >Signed-off-by: C O Mmitter > > instead of this: > >Signed-off-by: A U Thor >(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) > >Signed-off-by: C O Mmitter Here's the tweak I suggested last time. I think its behavior is slightly better in the "ends with incomplete line" case because it limits the characters examined by is_rfc2822_line() and is_cherry_picked_from_line() not to include buf[len] (which would presumably sometimes be '\0'). diff --git i/sequencer.c w/sequencer.c index 0b5cd18c..fa29c7c5 100644 --- i/sequencer.c +++ w/sequencer.c @@ -1043,9 +1043,7 @@ static int is_cherry_picked_from_line(const char *buf, int len) /* * We only care that it looks roughly like (cherry picked from ...) */ - return !prefixcmp(buf, cherry_picked_prefix) && - (buf[len - 1] == ')' || -(buf[len - 1] == '\n' && buf[len - 2] == ')')); + return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; } static int has_conforming_footer(struct strbuf *sb, int ignore_footer) @@ -1072,8 +1070,8 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - if (!(is_rfc2822_line(buf + i, k - i) || - is_cherry_picked_from_line(buf + i, k - i))) + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) return 0; } return 1; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
When 'cherry-pick -s' is used to append a signed-off-by line to a cherry picked commit, it does not currently detect the "(cherry picked from..." that may have been appended by a previous 'cherry-pick -x' as part of the s-o-b footer and it will insert a blank line before appending a new s-o-b. Let's detect "(cherry picked from...)" as part of the footer so that we will produce this: Signed-off-by: A U Thor (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter instead of this: Signed-off-by: A U Thor (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter Signed-off-by: Brandon Casey Acked-by: Jonathan Nieder --- sequencer.c | 46 t/t3511-cherry-pick-x.sh | 55 2 files changed, 88 insertions(+), 13 deletions(-) diff --git a/sequencer.c b/sequencer.c index 39a752b..0b5cd18 100644 --- a/sequencer.c +++ b/sequencer.c @@ -18,6 +18,7 @@ #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" const char sign_off_header[] = "Signed-off-by: "; +static const char cherry_picked_prefix[] = "(cherry picked from commit "; static void remove_sequencer_state(void) { @@ -496,7 +497,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { - strbuf_addstr(&msgbuf, "(cherry picked from commit "); + strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); strbuf_addstr(&msgbuf, ")\n"); } @@ -1021,11 +1022,36 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) +static int is_rfc2822_line(const char *buf, int len) +{ + int i; + + for (i = 0; i < len; i++) { + int ch = buf[i]; + if (ch == ':') + break; + if (isalnum(ch) || (ch == '-')) + continue; + return 0; + } + + return 1; +} + +static int is_cherry_picked_from_line(const char *buf, int len) +{ + /* +* We only care that it looks roughly like (cherry picked from ...) +*/ + return !prefixcmp(buf, cherry_picked_prefix) && + (buf[len - 1] == ')' || +(buf[len - 1] == '\n' && buf[len - 2] == ')')); +} + +static int has_conforming_footer(struct strbuf *sb, int ignore_footer) { - int ch; int last_char_was_nl, this_char_is_nl; - int i, j, k; + int i, k; int len = sb->len - ignore_footer; const char *buf = sb->buf; @@ -1046,15 +1072,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - for (j = 0; i + j < len; j++) { - ch = buf[i + j]; - if (ch == ':') - break; - if (isalnum(ch) || - (ch == '-')) - continue; + if (!(is_rfc2822_line(buf + i, k - i) || + is_cherry_picked_from_line(buf + i, k - i))) return 0; - } } return 1; } @@ -1071,7 +1091,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer) for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--) ; /* do nothing */ if (prefixcmp(msgbuf->buf + i, sob.buf)) { - if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer)) + if (!i || !has_conforming_footer(msgbuf, ignore_footer)) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len); } diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index 2a040b7..73da182 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" mesg_with_footer_sob="$mesg_with_footer Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" +mesg_with_cherry_footer="$mesg_with_footer_sob +(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709) +Tested-by: C.U. Thor " + test_expect_success setup ' git config advice.detachedhead false && @@ -47,6 +51,8 @@ test_expect_success setup ' test_commit "$mesg_with_footer" foo b mesg-with-footer && git reset --hard initial && test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob && + git reset --hard initial && + test_commit "$mesg_with_cherry_footer" foo b