[PATCH v3] cherry-pick: don't forget -s on failure

2012-09-13 Thread Miklos Vajna
In case 'git cherry-pick -s commit' failed, the user had to use 'git
commit -s' (i.e. state the -s option again), which is easy to forget
about.  Instead, write the signed-off-by line early, so plain 'git
commit' will have the same result.

Also update 'git commit -s', so that in case there is already a relevant
Signed-off-by line before the Conflicts: line, it won't add one more at
the end of the message. If there is no such line, then add it before the
the Conflicts: line.

Signed-off-by: Miklos Vajna vmik...@suse.cz
---

 This is somewhat iffy.  Shouldn't test_commit --signoff --notick
 work?

Indeed, fixed now.

 Hrm, what is this thing trying to do?  It does start scanning from
 the end (ignoring the Conflicts: thing) to see who the last person
 that signed it off was, but once it decides that it needs to add a
 new sign-off, it still adds it at the very end anyway.

Ah, I did not handle that, as the original git commit -s didn't do it 
either -- and originally I just wanted to touch git cherry-pick. 
Implemented now.

 builtin/commit.c|   79 +++---
 sequencer.c |   72 +++
 sequencer.h |4 ++
 t/t3507-cherry-pick-conflict.sh |   32 
 t/t3510-cherry-pick-sequence.sh |6 +-
 t/test-lib-functions.sh |   20 +++--
 6 files changed, 149 insertions(+), 64 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 778cf16..4d50484 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -28,6 +28,7 @@
 #include submodule.h
 #include gpg-interface.h
 #include column.h
+#include sequencer.h
 
 static const char * const builtin_commit_usage[] = {
N_(git commit [options] [--] filepattern...),
@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)
return !!(current_head-parents  current_head-parents-next);
 }
 
-static const char sign_off_header[] = Signed-off-by: ;
-
 static void export_one(const char *var, const char *s, const char *e, int hack)
 {
struct strbuf buf = STRBUF_INIT;
@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf 
*author_ident)
}
 }
 
-static int ends_rfc2822_footer(struct strbuf *sb)
-{
-   int ch;
-   int hit = 0;
-   int i, j, k;
-   int len = sb-len;
-   int first = 1;
-   const char *buf = sb-buf;
-
-   for (i = len - 1; i  0; i--) {
-   if (hit  buf[i] == '\n')
-   break;
-   hit = (buf[i] == '\n');
-   }
-
-   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 ((buf[k] == ' ' || buf[k] == '\t')  !first)
-   continue;
-
-   first = 0;
-
-   for (j = 0; i + j  len; j++) {
-   ch = buf[i + j];
-   if (ch == ':')
-   break;
-   if (isalnum(ch) ||
-   (ch == '-'))
-   continue;
-   return 0;
-   }
-   }
-   return 1;
-}
-
 static char *cut_ident_timestamp_part(char *string)
 {
char *ket = strrchr(string, '');
@@ -717,21 +675,30 @@ static int prepare_to_commit(const char *index_file, 
const char *prefix,
stripspace(sb, 0);
 
if (signoff) {
-   struct strbuf sob = STRBUF_INIT;
-   int i;
+   /*
+* See if we have a Conflicts: block at the end. If yes, count
+* its size, so we can ignore it.
+*/
+   int ignore_footer = 0;
+   int i, eol, previous = 0;
+   const char *nl;
 
-   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 = sb.len - 1; i  0  sb.buf[i - 1] != '\n'; i--)
-   ; /* do nothing */
-   if (prefixcmp(sb.buf + i, sob.buf)) {
-   if (!i || !ends_rfc2822_footer(sb))
-   strbuf_addch(sb, '\n');
-   strbuf_addbuf(sb, sob);
+   for (i = 0; i  sb.len; i++) {
+   nl = memchr(sb.buf + i, '\n', sb.len - i);
+   if (nl)
+   eol = nl - sb.buf;
+   else
+   eol = sb.len;
+   if (!prefixcmp(sb.buf + previous, \nConflicts:\n)) {
+   ignore_footer = sb.len - previous;
+   break;
+   }
+   while (i  

Re: [PATCH v3] cherry-pick: don't forget -s on failure

2012-09-13 Thread Junio C Hamano
Miklos Vajna vmik...@suse.cz writes:

 +void append_signoff(struct strbuf *msgbuf, int ignore_footer)
 +{
 + struct strbuf sob = STRBUF_INIT;
 + int i;
 +
 + 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 */
 + struct strbuf footer = STRBUF_INIT;
 + if (ignore_footer  0) {
 + strbuf_addstr(footer, msgbuf-buf + msgbuf-len - 
 ignore_footer);
 + strbuf_setlen(msgbuf, msgbuf-len - ignore_footer);
 + }

That's decl-after-stmt.

I would have expected that you can just do strbuf_splice() to add
the sob into msgbuf with the original code structure, without a
substantial rewrite of the function like this.  Perhaps I am missing
something?
--
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