# [PATCH v2 3/4] format-patch: update append_signoff prototype

9 messages from 2012-11-22 to 2012-12-02. Participants: Nguyễn Thái Ngọc Duy, Nguyen Thai Ngoc Duy, Junio C Hamano, Torsten Bögershausen, Brandon Casey.
Thread: https://gitlist.dev/t/32160

## Nguyen Thai Ngoc Duy, 2012-11-22 11:24

Subject: Re: Topics currently in the Stalled category
Message-ID: <CACsJy8BiJRK7N3_HZ2WXpMd1YkDSW00AxuYqiCWJgij+Kq6AiQ@mail.gmail.com>
URL: https://gitlist.dev/e/CACsJy8BiJRK7N3_HZ2WXpMd1YkDSW00AxuYqiCWJgij%2BKq6AiQ%40mail.gmail.com
In-Reply-To: <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org>

```
On Wed, Nov 21, 2012 at 7:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
>  - Unify appending signoff in format-patch, commit and sequencer
>
>  I am not sure if the logic to refrain from adding a sign-off based
>  on the existing run of sign-offs is done correctly in this change.

I'm not sure what's the right logic here. The original code in
format-patch has exactly one test just to verify that --signoff does
_something_. There's another signoff code in git-am.sh and we should
make sure at least the behavior is consistent. I guess I should start
over by writing new tests to record the current behavior, then fixes
and finally the unification. I think you can drop this for now.
-- 
Duy

```

## Nguyễn Thái Ngọc Duy, 2012-11-22 16:38

Subject: [PATCH v2 0/4] nd/unify-appending-of-s-o-b
Message-ID: <1353602289-9418-1-git-send-email-pclouds@gmail.com>
URL: https://gitlist.dev/e/1353602289-9418-1-git-send-email-pclouds%40gmail.com
In-Reply-To: <CACsJy8BiJRK7N3_HZ2WXpMd1YkDSW00AxuYqiCWJgij+Kq6AiQ@mail.gmail.com>

```
This round cherry-pick and commit only skip S-o-b if the last one is
the same while format-patch skips it if the same one appears
anywhere in S-o-b list. I don't have strong opinion about which way is
more correct. Though if we decide to change format-patch behavior, the
series may become a bit different.

Nguyễn Thái Ngọc Duy (4):
  t4014: more tests about appending s-o-b lines
  format-patch: stricter S-o-b detection
  format-patch: update append_signoff prototype
  Unify appending signoff in format-patch, commit and sequencer

 builtin/commit.c        |  10 ++-
 builtin/log.c           |  13 +---
 log-tree.c              |  66 +++++++++++++---
 log-tree.h              |   4 +
 revision.h              |   2 +-
 sequencer.c             |  65 +---------------
 sequencer.h             |   4 -
 t/t4014-format-patch.sh | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
 t/t7501-commit.sh       |   2 +-
 9 files changed, 272 insertions(+), 93 deletions(-)

-- 
1.8.0.4.g5d0415a

```

## Nguyễn Thái Ngọc Duy, 2012-11-22 16:38

Subject: [PATCH v2 1/4] t4014: more tests about appending s-o-b lines
Message-ID: <1353602289-9418-2-git-send-email-pclouds@gmail.com>
URL: https://gitlist.dev/e/1353602289-9418-2-git-send-email-pclouds%40gmail.com
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

```

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 959aa26..c8d5d29 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -891,4 +891,149 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+append_signoff()
+{
+	C=`git commit-tree HEAD^^{tree} -p HEAD` &&
+	git format-patch --stdout --signoff ${C}^..${C} |
+		tee append_signoff.patch |
+		sed -n "1,/^---$/p" |
+		grep -n -E "^Subject|Sign|^$"
+}
+
+test_expect_success 'signoff: commit with no body' '
+	append_signoff </dev/null >actual &&
+	cat <<\EOF | sed "s/EOL$//" >expected &&
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+	echo subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with NL' '
+	echo -n subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+	printf "subject\n\nbody" | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+	printf "subject\n\nSigned-off-by: C O Mitter <committer@example.com>" |
+		append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff NOT at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+12:Signed-off-by: my@house
+EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.4.g5d0415a

```

## Nguyễn Thái Ngọc Duy, 2012-11-22 16:38

Subject: [PATCH v2 2/4] format-patch: stricter S-o-b detection
Message-ID: <1353602289-9418-3-git-send-email-pclouds@gmail.com>
URL: https://gitlist.dev/e/1353602289-9418-3-git-send-email-pclouds%40gmail.com
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

```
S-o-b in the middle of a sentence, at the beginning of the sentence
but it is just because of text wrapping, or a full paragraph of valid
S-o-b in the middle of the message. All these are not S-o-b, but
detected as is. Fix them.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 log-tree.c              | 37 +++++++++++++++++++++++++++++++--
 t/t4014-format-patch.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index c894930..aff7c0a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -260,14 +260,47 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	int has_signoff = 0;
 	char *cp;
 
-	cp = sb->buf;
+	/*
+	 * Only search in the last paragrah, don't be fooled by a
+	 * paragraph full of valid S-o-b in the middle.
+	 */
+	cp = sb->buf + sb->len - 1;
+	while (cp > sb->buf) {
+		if (cp[0] == '\n' && cp[1] == '\n') {
+			cp += 2;
+			break;
+		}
+		cp--;
+	}
 
 	/* First see if we already have the sign-off by the signer */
 	while ((cp = strstr(cp, signed_off_by))) {
+		const char *s = cp;
+		cp += strlen(signed_off_by);
+
+		if (!has_signoff && s > sb->buf) {
+			/*
+			 * S-o-b in the middle of a sentence is not
+			 * really S-o-b
+			 */
+			if (s[-1] != '\n')
+				continue;
+
+			if (s - 1 > sb->buf && s[-2] == '\n')
+				; /* the first S-o-b */
+			else if (!detect_any_signoff(sb->buf, s - sb->buf))
+				/*
+				 * The previous line looks like an
+				 * S-o-b. There's still no guarantee
+				 * that it's an actual S-o-b. For that
+				 * we need to look back until we find
+				 * a blank line, which is too costly.
+				 */
+				continue;
+		}
 
 		has_signoff = 1;
 
-		cp += strlen(signed_off_by);
 		if (cp + signoff_len >= sb->buf + sb->len)
 			break;
 		if (strncmp(cp, signoff, signoff_len))
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c8d5d29..d0b3c85 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -989,6 +989,60 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'signoff: not really a signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: not really a signoff (2)' '
+	append_signoff <<\EOF >actual &&
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: valid S-o-b paragraph in the middle' '
+	append_signoff <<\EOF >actual &&
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'signoff: the same signoff at the end' '
 	append_signoff <<\EOF >actual &&
 subject
-- 
1.8.0.4.g5d0415a

```

## Nguyễn Thái Ngọc Duy, 2012-11-22 16:38

Subject: [PATCH v2 3/4] format-patch: update append_signoff prototype
Message-ID: <1353602289-9418-4-git-send-email-pclouds@gmail.com>
URL: https://gitlist.dev/e/1353602289-9418-4-git-send-email-pclouds%40gmail.com
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

```
This is a preparation step for merging with append_signoff from
sequencer.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/log.c | 13 +------------
 log-tree.c    | 21 +++++++++++++--------
 revision.h    |  2 +-
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e..f201070 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1053,7 +1053,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
-	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
@@ -1148,16 +1147,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	if (do_signoff) {
-		const char *committer;
-		const char *endpos;
-		committer = git_committer_info(IDENT_STRICT);
-		endpos = strchr(committer, '>');
-		if (!endpos)
-			die(_("bogus committer info %s"), committer);
-		add_signoff = xmemdupz(committer, endpos - committer + 1);
-	}
-
 	for (i = 0; i < extra_hdr.nr; i++) {
 		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
@@ -1348,7 +1337,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		total++;
 		start_number--;
 	}
-	rev.add_signoff = add_signoff;
+	rev.add_signoff = do_signoff;
 	while (0 <= --nr) {
 		int shown;
 		commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index aff7c0a..7e50545 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -253,9 +253,11 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int flags)
 {
-	static const char signed_off_by[] = "Signed-off-by: ";
+	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					 getenv("GIT_COMMITTER_EMAIL")));
+	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -274,9 +276,9 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	}
 
 	/* First see if we already have the sign-off by the signer */
-	while ((cp = strstr(cp, signed_off_by))) {
+	while ((cp = strstr(cp, sign_off_header))) {
 		const char *s = cp;
-		cp += strlen(signed_off_by);
+		cp += strlen(sign_off_header);
 
 		if (!has_signoff && s > sb->buf) {
 			/*
@@ -317,9 +319,10 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	if (!has_signoff)
 		strbuf_addch(sb, '\n');
 
-	strbuf_addstr(sb, signed_off_by);
+	strbuf_addstr(sb, sign_off_header);
 	strbuf_add(sb, signoff, signoff_len);
 	strbuf_addch(sb, '\n');
+	free(signoff);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -684,8 +687,10 @@ void show_log(struct rev_info *opt)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	if (ctx.need_8bit_cte >= 0)
-		ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+	if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+		ctx.need_8bit_cte =
+			has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					       getenv("GIT_COMMITTER_EMAIL")));
 	ctx.date_mode = opt->date_mode;
 	ctx.date_mode_explicit = opt->date_mode_explicit;
 	ctx.abbrev = opt->diffopt.abbrev;
@@ -696,7 +701,7 @@ void show_log(struct rev_info *opt)
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
-		append_signoff(&msgbuf, opt->add_signoff);
+		append_signoff(&msgbuf, 0);
 	if (opt->show_log_size) {
 		printf("log size %i\n", (int)msgbuf.len);
 		graph_show_oneline(opt->graph);
diff --git a/revision.h b/revision.h
index a95bd0b..af35325 100644
--- a/revision.h
+++ b/revision.h
@@ -136,7 +136,7 @@ struct rev_info {
 	int		numbered_files;
 	char		*message_id;
 	struct string_list *ref_message_ids;
-	const char	*add_signoff;
+	int              add_signoff;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
-- 
1.8.0.4.g5d0415a

```

## Nguyễn Thái Ngọc Duy, 2012-11-22 16:38

Subject: [PATCH v2 4/4] Unify appending signoff in format-patch, commit and sequencer
Message-ID: <1353602289-9418-5-git-send-email-pclouds@gmail.com>
URL: https://gitlist.dev/e/1353602289-9418-5-git-send-email-pclouds%40gmail.com
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

```
There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing. This patch removes
the sequencer.c's in favor of the format-patch's.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c  | 10 ++++++++-
 log-tree.c        | 14 ++++++++----
 log-tree.h        |  4 ++++
 sequencer.c       | 65 ++-----------------------------------------------------
 sequencer.h       |  4 ----
 t/t7501-commit.sh |  2 +-
 6 files changed, 26 insertions(+), 73 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..6d323d9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -698,7 +698,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			previous = eol;
 		}
 
-		append_signoff(&sb, ignore_footer);
+		if (ignore_footer) {
+			struct strbuf footer = STRBUF_INIT;
+			strbuf_addstr(&footer, sb.buf + sb.len - ignore_footer);
+			strbuf_setlen(&sb, sb.len - ignore_footer);
+			append_signoff(&sb, SOB_IGNORE_SAME);
+			strbuf_addstr(&sb, footer.buf);
+			strbuf_release(&footer);
+		} else
+			append_signoff(&sb, SOB_IGNORE_SAME);
 	}
 
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/log-tree.c b/log-tree.c
index 7e50545..e8d31d9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -12,6 +12,8 @@
 
 struct decoration name_decoration = { "object names" };
 
+const char sign_off_header[] = "Signed-off-by: ";
+
 enum decoration_type {
 	DECORATION_NONE = 0,
 	DECORATION_REF_LOCAL,
@@ -207,7 +209,7 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
 }
 
 /*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
+ * Search for "^[-A-Za-z0-9]+: [^@]+@" pattern. It usually matches
  * Signed-off-by: and Acked-by: lines.
  */
 static int detect_any_signoff(char *letter, int size)
@@ -243,6 +245,7 @@ static int detect_any_signoff(char *letter, int size)
 		}
 		if (('A' <= ch && ch <= 'Z') ||
 		    ('a' <= ch && ch <= 'z') ||
+		    ('0' <= ch && ch <= '9') ||
 		    ch == '-') {
 			seen_head = 1;
 			continue;
@@ -253,11 +256,10 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, int flags)
+void append_signoff(struct strbuf *sb, int flags)
 {
 	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
 					 getenv("GIT_COMMITTER_EMAIL")));
-	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -310,7 +312,11 @@ static void append_signoff(struct strbuf *sb, int flags)
 		if (!isspace(cp[signoff_len]))
 			continue;
 		/* we already have him */
-		return;
+		if (flags & SOB_IGNORE_SAME) {
+			if (cp[signoff_len + 1] == '\0')
+				return;
+		} else
+			return;
 	}
 
 	if (!has_signoff)
diff --git a/log-tree.h b/log-tree.h
index f5ac238..739f729 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -24,4 +24,8 @@ void load_ref_decorations(int flags);
 void get_patch_filename(struct commit *commit, const char *subject, int nr,
 			const char *suffix, struct strbuf *buf);
 
+#define SOB_IGNORE_SAME   1
+extern const char sign_off_header[];
+void append_signoff(struct strbuf *msgbuf, int flags);
+
 #endif
diff --git a/sequencer.c b/sequencer.c
index e3723d2..bc02a66 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,11 +14,10 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "log-tree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
-const char sign_off_header[] = "Signed-off-by: ";
-
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -236,7 +235,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, SOB_IGNORE_SAME);
 
 	if (!clean) {
 		int i;
@@ -1016,63 +1015,3 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	save_opts(opts);
 	return pick_commits(todo_list, opts);
 }
-
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
-{
-	int ch;
-	int hit = 0;
-	int i, j, k;
-	int len = sb->len - ignore_footer;
-	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;
-}
-
-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 */
-	if (prefixcmp(msgbuf->buf + i, sob.buf)) {
-		if (!i || !ends_rfc2822_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);
-	}
-	strbuf_release(&sob);
-}
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..99eb7fa 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -46,8 +46,4 @@ struct replay_opts {
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-extern const char sign_off_header[];
-
-void append_signoff(struct strbuf *msgbuf, int ignore_footer);
-
 #endif
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 195e747..6dd4580 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -353,7 +353,7 @@ test_expect_success 'signoff gap' '
 
 	echo 3 >positive &&
 	git add positive &&
-	alt="Alt-RFC-822-Header: Value" &&
+	alt="Alt-RFC-822-Header: Va@lue" &&
 	git commit -s -m "welcome
 
 $alt" &&
-- 
1.8.0.4.g5d0415a

```

## Junio C Hamano, 2012-11-24 02:05

Subject: Re: [PATCH v2 0/4] nd/unify-appending-of-s-o-b
Message-ID: <7vtxsfzrfr.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vtxsfzrfr.fsf%40alter.siamese.dyndns.org
In-Reply-To: <1353602289-9418-1-git-send-email-pclouds@gmail.com>

```
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This round cherry-pick and commit only skip S-o-b if the last one is
> the same...

That sounds sensible.  When the user asks us to add sign-off, we
should honor it, unless it is known that adding one is absolutely
unnecessary, which is "it already exists as the last one".  Ignoring
all other cases will lose information.

Among the non-merge commits in the kernel history, you see about 500
such commits where the same contributor signs off the patch more
than once, recording the flow of the patch; 106a4ee258 seems to be
the most recent example.

Thanks.

```

## Torsten Bögershausen, 2012-12-02 07:06

Subject: Re: [PATCH v2 1/4] t4014: more tests about appending s-o-b lines
Message-ID: <50BAFE04.4080100@web.de>
URL: https://gitlist.dev/e/50BAFE04.4080100%40web.de
In-Reply-To: <1353602289-9418-2-git-send-email-pclouds@gmail.com>

```
On 22.11.12 17:38, Nguyễn Thái Ngọc Duy wrote:
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
> +	echo -n subject | append_signoff >actual &&


"echo -n" is not portable, and we use printf everywhere. 
I found one "echo -n" in line  996.

Can we squeeze that in, before going to next?

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 6cfad13..f460930 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -993,7 +993,7 @@ EOF
 '
 
 test_expect_success 'signoff: commit with only subject that does not end with NL' '
-       echo -n subject | append_signoff >actual &&
+       printf subject | append_signoff >actual &&
        cat >expected <<\EOF &&
 4:Subject: [PATCH] subject
 8:

```

## Brandon Casey, 2012-12-02 08:03

Subject: Re: [PATCH v2 1/4] t4014: more tests about appending s-o-b lines
Message-ID: <CA+sFfMd-yvQpsfVEWYBk+70_05Ov5aX4xg8nNY0HPwHTVAK60Q@mail.gmail.com>
URL: https://gitlist.dev/e/CA%2BsFfMd-yvQpsfVEWYBk%2B70_05Ov5aX4xg8nNY0HPwHTVAK60Q%40mail.gmail.com
In-Reply-To: <50BAFE04.4080100@web.de>

```
On Sat, Dec 1, 2012 at 11:06 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 22.11.12 17:38, Nguyễn Thái Ngọc Duy wrote:
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 145 insertions(+)
>> +     echo -n subject | append_signoff >actual &&
>
>
> "echo -n" is not portable, and we use printf everywhere.
> I found one "echo -n" in line  996.
>
> Can we squeeze that in, before going to next?

I got it.  I'll squash it into the next series.

-Brandon

```
