From fa1727fb210bd8d0079af124222420d46fb70f81 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 12 Feb 2013 02:17:28 -0800 Subject: sequencer.c: rework search for start of footer to improve clarity This code sequence is somewhat difficult to read. Let's rewrite it and add some comments to improve clarity. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- sequencer.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 22604902aa..776ddb1962 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1019,19 +1019,21 @@ int sequencer_pick_revisions(struct replay_opts *opts) static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) { - int ch; - int hit = 0; + char ch, prev; int i, j, k; int len = sb->len - ignore_footer; int first = 1; const char *buf = sb->buf; + prev = '\0'; for (i = len - 1; i > 0; i--) { - if (hit && buf[i] == '\n') + ch = buf[i]; + if (prev == '\n' && ch == '\n') /* paragraph break */ break; - hit = (buf[i] == '\n'); + prev = ch; } + /* advance to start of last paragraph */ while (i < len - 1 && buf[i] == '\n') i++; -- cgit v1.3 From 9b1515220912b4e436145babf0b5887ecbfb0ac4 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:29 -0800 Subject: commit, cherry-pick -s: remove broken support for multiline rfc2822 fields Starting with c1e01b0c (commit: More generous accepting of RFC-2822 footer lines, 2009-10-28), "git commit -s" carefully parses the last paragraph of each commit message to check if it consists only of RFC2822-style headers, in which case the signoff will be added as a new line in the same list: Reported-by: Reporter Signed-off-by: Author Acked-by: Lieutenant It even included support for accepting indented continuation lines for multiline fields. Unfortunately the multiline field support is broken because it checks whether buf[k] (the first character of the *next* line) instead of buf[i] is a whitespace character. The result is that any footer with a continuation line is not accepted, since the last continuation line neither starts with an RFC2822 field name nor is followed by a continuation line. That this has remained broken for so long is good evidence that nobody actually needed multiline fields. Rip out the broken continuation support. There should be no functional change. [Thanks to Jonathan Nieder for the excellent commit message] Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- sequencer.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 776ddb1962..1a80dac6c5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1022,7 +1022,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) char ch, prev; int i, j, k; int len = sb->len - ignore_footer; - int first = 1; const char *buf = sb->buf; prev = '\0'; @@ -1042,11 +1041,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - if ((buf[k] == ' ' || buf[k] == '\t') && !first) - continue; - - first = 0; - for (j = 0; i + j < len; j++) { ch = buf[i + j]; if (ch == ':') -- cgit v1.3 From cd650a4eee148dadb0943f415e88c210f18e70df Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:32 -0800 Subject: 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 [with improvements from Jonathan Nieder] Signed-off-by: Brandon Casey Acked-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- sequencer.c | 51 ++++++++++++++++++++++++++++++++------------ t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 1a80dac6c5..7889b6bc19 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) { @@ -492,7 +493,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"); } @@ -1017,16 +1018,44 @@ 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) { - char ch, prev; - int i, j, k; + int i; + + for (i = 0; i < len; i++) { + int ch = buf[i]; + if (ch == ':') + return 1; + if (!isalnum(ch) && ch != '-') + break; + } + + return 0; +} + +static int is_cherry_picked_from_line(const char *buf, int len) +{ + /* + * We only care that it looks roughly like (cherry picked from ...) + */ + return len > strlen(cherry_picked_prefix) + 1 && + !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; +} + +static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +{ + char prev; + int i, k; int len = sb->len - ignore_footer; const char *buf = sb->buf; + /* footer must end with newline */ + if (!len || buf[len - 1] != '\n') + return 0; + prev = '\0'; for (i = len - 1; i > 0; i--) { - ch = buf[i]; + char ch = buf[i]; if (prev == '\n' && ch == '\n') /* paragraph break */ break; prev = ch; @@ -1041,15 +1070,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 - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) return 0; - } } return 1; } @@ -1066,7 +1089,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 2a040b7664..73da182e1a 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 mesg-with-cherry-footer && pristine_detach initial && test_commit conflicting unrelated ' @@ -98,6 +104,19 @@ test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committe test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-footer^0` && + git cherry-pick -x -s mesg-with-footer && + cat <<-EOF >expect && + $mesg_with_footer + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' ' pristine_detach initial && git cherry-pick -s mesg-with-footer-sob && @@ -108,4 +127,40 @@ test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-footer-sob^0` && + git cherry-pick -x -s mesg-with-footer-sob && + cat <<-EOF >expect && + $mesg_with_footer_sob + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-cherry-footer^0` && + git cherry-pick -x mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + git cherry-pick -s mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_done -- cgit v1.3 From 2cdccad1600171e3c53e2e9c22e1675389a9d61a Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:33 -0800 Subject: sequencer.c: require a conforming footer to be preceded by a blank line Currently, append_signoff() performs a search for the last line of the commit buffer by searching back from the end until it hits a newline. If it reaches the beginning of the buffer without finding a newline, that means either the commit message was empty, or there was only one line in it. In this case, append_signoff will skip the call to has_conforming_footer since it already knows that it is necessary to append a newline before appending the sob. Let's perform this function inside of has_conforming_footer where it appropriately belongs and generalize it so that we require that the footer paragraph be an actual distinct paragraph separated by a blank line. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- sequencer.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 7889b6bc19..5a9c43d806 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1061,6 +1061,10 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) prev = ch; } + /* require at least one blank line */ + if (prev != '\n' || buf[i] != '\n') + return 0; + /* advance to start of last paragraph */ while (i < len - 1 && buf[i] == '\n') i++; @@ -1089,7 +1093,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 || !has_conforming_footer(msgbuf, ignore_footer)) + if (!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); } -- cgit v1.3 From b971e04f54e73a5a2af54ad62540273bb90ee7bb Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:34 -0800 Subject: sequencer.c: always separate "(cherry picked from" from commit body Start treating the "(cherry picked from" line added by cherry-pick -x the same way that the s-o-b lines are treated. Namely, separate them from the main commit message body with an empty line. Introduce tests to test this functionality. Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- sequencer.c | 128 ++++++++++++++++++++++++----------------------- t/t3511-cherry-pick-x.sh | 53 ++++++++++++++++++++ 2 files changed, 118 insertions(+), 63 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 5a9c43d806..f9f9f8a772 100644 --- a/sequencer.c +++ b/sequencer.c @@ -20,6 +20,69 @@ const char sign_off_header[] = "Signed-off-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; +static int is_rfc2822_line(const char *buf, int len) +{ + int i; + + for (i = 0; i < len; i++) { + int ch = buf[i]; + if (ch == ':') + return 1; + if (!isalnum(ch) && ch != '-') + break; + } + + return 0; +} + +static int is_cherry_picked_from_line(const char *buf, int len) +{ + /* + * We only care that it looks roughly like (cherry picked from ...) + */ + return len > strlen(cherry_picked_prefix) + 1 && + !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; +} + +static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +{ + char prev; + int i, k; + int len = sb->len - ignore_footer; + const char *buf = sb->buf; + + /* footer must end with newline */ + if (!len || buf[len - 1] != '\n') + return 0; + + prev = '\0'; + for (i = len - 1; i > 0; i--) { + char ch = buf[i]; + if (prev == '\n' && ch == '\n') /* paragraph break */ + break; + prev = ch; + } + + /* require at least one blank line */ + if (prev != '\n' || buf[i] != '\n') + return 0; + + /* advance to start of last paragraph */ + while (i < len - 1 && buf[i] == '\n') + i++; + + for (; i < len; i = k) { + for (k = i; k < len && buf[k] != '\n'; k++) + ; /* do nothing */ + k++; + + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) + return 0; + } + return 1; +} + static void remove_sequencer_state(void) { struct strbuf seq_dir = STRBUF_INIT; @@ -493,6 +556,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { + if (!has_conforming_footer(&msgbuf, 0)) + strbuf_addch(&msgbuf, '\n'); strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); strbuf_addstr(&msgbuf, ")\n"); @@ -1018,69 +1083,6 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -static int is_rfc2822_line(const char *buf, int len) -{ - int i; - - for (i = 0; i < len; i++) { - int ch = buf[i]; - if (ch == ':') - return 1; - if (!isalnum(ch) && ch != '-') - break; - } - - return 0; -} - -static int is_cherry_picked_from_line(const char *buf, int len) -{ - /* - * We only care that it looks roughly like (cherry picked from ...) - */ - return len > strlen(cherry_picked_prefix) + 1 && - !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; -} - -static int has_conforming_footer(struct strbuf *sb, int ignore_footer) -{ - char prev; - int i, k; - int len = sb->len - ignore_footer; - const char *buf = sb->buf; - - /* footer must end with newline */ - if (!len || buf[len - 1] != '\n') - return 0; - - prev = '\0'; - for (i = len - 1; i > 0; i--) { - char ch = buf[i]; - if (prev == '\n' && ch == '\n') /* paragraph break */ - break; - prev = ch; - } - - /* require at least one blank line */ - if (prev != '\n' || buf[i] != '\n') - return 0; - - /* advance to start of last paragraph */ - while (i < len - 1 && buf[i] == '\n') - i++; - - for (; i < len; i = k) { - for (k = i; k < len && buf[k] != '\n'; k++) - ; /* do nothing */ - k++; - - if (!is_rfc2822_line(buf + i, k - i - 1) && - !is_cherry_picked_from_line(buf + i, k - i - 1)) - return 0; - } - return 1; -} - void append_signoff(struct strbuf *msgbuf, int ignore_footer) { struct strbuf sob = STRBUF_INIT; diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index 73da182e1a..a845e45d40 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -57,6 +57,19 @@ test_expect_success setup ' test_commit conflicting unrelated ' +test_expect_success 'cherry-pick -x inserts blank line after one line subject' ' + pristine_detach initial && + sha1=`git rev-parse mesg-one-line^0` && + git cherry-pick -x mesg-one-line && + cat <<-EOF >expect && + $mesg_one_line + + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s inserts blank line after one line subject' ' pristine_detach initial && git cherry-pick -s mesg-one-line && @@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line after non-conforming foot test_cmp expect actual ' +test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' ' + pristine_detach initial && + sha1=`git rev-parse mesg-no-footer^0` && + git cherry-pick -x mesg-no-footer && + cat <<-EOF >expect && + $mesg_no_footer + + (cherry picked from commit $sha1) + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' ' pristine_detach initial && git cherry-pick -s mesg-no-footer && @@ -93,6 +119,20 @@ test_expect_success 'cherry-pick -s inserts blank line when conforming footer no test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer not found' ' + pristine_detach initial && + sha1=`git rev-parse mesg-no-footer^0` && + git cherry-pick -x -s mesg-no-footer && + cat <<-EOF >expect && + $mesg_no_footer + + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' ' pristine_detach initial && git cherry-pick -s mesg-with-footer && @@ -163,4 +203,17 @@ test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part test_cmp expect actual ' +test_expect_success 'cherry-pick -x -s treats "(cherry picked from..." line as part of footer' ' + pristine_detach initial && + sha1=`git rev-parse mesg-with-cherry-footer^0` && + git cherry-pick -x -s mesg-with-cherry-footer && + cat <<-EOF >expect && + $mesg_with_cherry_footer + (cherry picked from commit $sha1) + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + EOF + git log -1 --pretty=format:%B >actual && + test_cmp expect actual +' + test_done -- cgit v1.3 From bab4d1097c8be7d688a53b992232063dbf300fd4 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:17:35 -0800 Subject: sequencer.c: teach append_signoff how to detect duplicate s-o-b Teach append_signoff how to detect a duplicate s-o-b in the commit footer. This is in preparation to unify the append_signoff implementations in log-tree.c and sequencer.c. Fixes test in t3511. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- sequencer.c | 59 ++++++++++++++++++++++++++++++++++++------------ sequencer.h | 4 +++- t/t3511-cherry-pick-x.sh | 2 +- 4 files changed, 50 insertions(+), 17 deletions(-) (limited to 'sequencer.c') diff --git a/builtin/commit.c b/builtin/commit.c index 1dd2ec5e6f..7b9e2ac2eb 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, previous = eol; } - append_signoff(&sb, ignore_footer); + append_signoff(&sb, ignore_footer, 0); } if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) diff --git a/sequencer.c b/sequencer.c index f9f9f8a772..965b954baf 100644 --- a/sequencer.c +++ b/sequencer.c @@ -44,12 +44,20 @@ static int is_cherry_picked_from_line(const char *buf, int len) !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; } -static int has_conforming_footer(struct strbuf *sb, int ignore_footer) +/* + * Returns 0 for non-conforming footer + * Returns 1 for conforming footer + * Returns 2 when sob exists within conforming footer + * Returns 3 when sob exists within conforming footer as last entry + */ +static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, + int ignore_footer) { char prev; int i, k; int len = sb->len - ignore_footer; const char *buf = sb->buf; + int found_sob = 0; /* footer must end with newline */ if (!len || buf[len - 1] != '\n') @@ -72,14 +80,25 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) i++; for (; i < len; i = k) { + int found_rfc2822; + for (k = i; k < len && buf[k] != '\n'; k++) ; /* do nothing */ k++; - if (!is_rfc2822_line(buf + i, k - i - 1) && - !is_cherry_picked_from_line(buf + i, k - i - 1)) + found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1); + if (found_rfc2822 && sob && + !strncmp(buf + i, sob->buf, sob->len)) + found_sob = k; + + if (!(found_rfc2822 || + is_cherry_picked_from_line(buf + i, k - i - 1))) return 0; } + if (found_sob == i) + return 3; + if (found_sob) + return 2; return 1; } @@ -300,7 +319,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next, rollback_lock_file(&index_lock); if (opts->signoff) - append_signoff(msgbuf, 0); + append_signoff(msgbuf, 0, 0); if (!clean) { int i; @@ -556,7 +575,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) } if (opts->record_origin) { - if (!has_conforming_footer(&msgbuf, 0)) + if (!has_conforming_footer(&msgbuf, NULL, 0)) strbuf_addch(&msgbuf, '\n'); strbuf_addstr(&msgbuf, cherry_picked_prefix); strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); @@ -1083,21 +1102,33 @@ int sequencer_pick_revisions(struct replay_opts *opts) return pick_commits(todo_list, opts); } -void append_signoff(struct strbuf *msgbuf, int ignore_footer) +void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) { + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; struct strbuf sob = STRBUF_INIT; - int i; + int has_footer; strbuf_addstr(&sob, sign_off_header); strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"), getenv("GIT_COMMITTER_EMAIL"))); strbuf_addch(&sob, '\n'); - 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 (!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); - } + + /* + * If the whole message buffer is equal to the sob, pretend that we + * found a conforming footer with a matching sob + */ + if (msgbuf->len - ignore_footer == sob.len && + !strncmp(msgbuf->buf, sob.buf, sob.len)) + has_footer = 3; + else + has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); + + if (!has_footer) + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); + + if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, + sob.buf, sob.len); + strbuf_release(&sob); } diff --git a/sequencer.h b/sequencer.h index 9d57d57524..1fc22dcabe 100644 --- a/sequencer.h +++ b/sequencer.h @@ -6,6 +6,8 @@ #define SEQ_TODO_FILE "sequencer/todo" #define SEQ_OPTS_FILE "sequencer/opts" +#define APPEND_SIGNOFF_DEDUP (1u << 0) + enum replay_action { REPLAY_REVERT, REPLAY_PICK @@ -48,6 +50,6 @@ int sequencer_pick_revisions(struct replay_opts *opts); extern const char sign_off_header[]; -void append_signoff(struct strbuf *msgbuf, int ignore_footer); +void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); #endif diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index a845e45d40..f97727975b 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -82,7 +82,7 @@ test_expect_success 'cherry-pick -s inserts blank line after one line subject' ' test_cmp expect actual ' -test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' ' +test_expect_success 'cherry-pick -s inserts blank line after non-conforming footer' ' pristine_detach initial && git cherry-pick -s mesg-broken-footer && cat <<-EOF >expect && -- cgit v1.3 From 33f2f9ab4e5c343b21a9e602456e7facabd609a0 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 12 Feb 2013 02:33:42 -0800 Subject: sequencer.c: teach append_signoff to avoid adding a duplicate newline Teach append_signoff to detect whether a blank line exists at the position that the signed-off-by line will be added, and refrain from adding an additional one if one already exists. Or, add an additional line if one is needed to make sure the new footer is separated from the message body by a blank line. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- sequencer.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 965b954baf..53ee49a9b0 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1123,8 +1123,19 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) else has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); - if (!has_footer) - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1); + if (!has_footer) { + const char *append_newlines = NULL; + size_t len = msgbuf->len - ignore_footer; + + if (len && msgbuf->buf[len - 1] != '\n') + append_newlines = "\n\n"; + else if (len > 1 && msgbuf->buf[len - 2] != '\n') + append_newlines = "\n"; + + if (append_newlines) + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, + append_newlines, strlen(append_newlines)); + } if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, -- cgit v1.3 From 8c613fd5efa1c3bbd48845ed91c533192e699c08 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Fri, 22 Feb 2013 14:05:27 -0800 Subject: git-commit: populate the edit buffer with 2 blank lines before s-o-b 'commit -s' populates the edit buffer with a blank line before the Signed-off-by line, to allow the user to immediately start typing the log message. But commit 33f2f9ab removed this space, forcing the user to first push the Signed-off-by line down to open a place to type the log message. Fix this regression and let's ensure that the Signed-off-by line is preceded by two blank lines, instead of just one, to hint that something should be filled in, and that a blank line should separate it from the body and the Signed-off-by line. Add a test for this behavior. Reported-by: John Keeping Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- sequencer.c | 27 +++++++++++++++++++++++++-- t/t7502-commit.sh | 12 ++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 53ee49a9b0..a07d2d00b9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1127,10 +1127,33 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) const char *append_newlines = NULL; size_t len = msgbuf->len - ignore_footer; - if (len && msgbuf->buf[len - 1] != '\n') + if (!len) { + /* + * The buffer is completely empty. Leave foom for + * the title and body to be filled in by the user. + */ append_newlines = "\n\n"; - else if (len > 1 && msgbuf->buf[len - 2] != '\n') + } else if (msgbuf->buf[len - 1] != '\n') { + /* + * Incomplete line. Complete the line and add a + * blank one so that there is an empty line between + * the message body and the sob. + */ + append_newlines = "\n\n"; + } else if (len == 1) { + /* + * Buffer contains a single newline. Add another + * so that we leave room for the title and body. + */ + append_newlines = "\n"; + } else if (msgbuf->buf[len - 2] != '\n') { + /* + * Buffer ends with a single newline. Add another + * so that there is an empty line between the message + * body and the sob. + */ append_newlines = "\n"; + } /* else, the buffer already ends with two newlines. */ if (append_newlines) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index deb187eb7b..c06a7526c1 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -349,6 +349,18 @@ test_expect_success 'A single-liner subject with a token plus colon is not a foo ' +test_expect_success 'commit -s places sob on third line after two empty lines' ' + git commit -s --allow-empty --allow-empty-message && + cat <<-EOF >expect && + + + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> + + EOF + sed -e "/^#/d" -e "s/^:.*//" .git/COMMIT_EDITMSG >actual && + test_cmp expect actual +' + write_script .git/FAKE_EDITOR <<\EOF mv "$1" "$1.orig" ( -- cgit v1.3