All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Malehorn <bmalehorn@gmail.com>
To: gitster@pobox.com
Cc: bmalehorn@gmail.com, git@vger.kernel.org, peff@peff.net
Subject: [PATCH] interpret-trailers: honor the cut line
Date: Mon, 15 May 2017 23:06:49 -0700	[thread overview]
Message-ID: <20170516060649.29110-2-bmalehorn@gmail.com> (raw)
In-Reply-To: <20170516060649.29110-1-bmalehorn@gmail.com>

If a commit message is edited with the "verbose" option, the buffer will
have a cut line and diff after the log message, like so:

    my subject

    # ------------------------ >8 ------------------------
    # Do not touch the line above.
    # Everything below will be removed.
    diff --git a/foo.txt b/foo.txt
    index 5716ca5..7601807 100644
    --- a/foo.txt
    +++ b/foo.txt
    @@ -1 +1 @@
    -bar
    +baz

"git interpret-trailers" is unaware of the cut line, and assumes the
trailer block would be at the end of the whole thing.  This can easily
be seen with:

     $ GIT_EDITOR='git interpret-trailers --in-place --trailer Acked-by:me' \
       git commit --amend -v

Teach "git interpret-trailers" to notice the cut-line and ignore the
remainder of the input when looking for a place to add new trailer
block.  This makes it consistent with how "git commit -v -s" inserts a
new Signed-off-by: line.

This can be done by the same logic as the existing helper function,
wt_status_truncate_message_at_cut_line(), uses, but it wants the caller
to pass a strbuf to it.  Because the helper function
ignore_non_trailer() used by the command takes a <pointer, length> pair,
not a strbuf, steal the logic from
wt_status_truncate_message_at_cut_line() to create a new
wt_status_locate_end() helper function that takes <pointer, length>
pair, and make ignore_non_trailer() call it to help
"interpret-trailers".  Since there is only one caller of
wt_status_truncate_message_at_cut_line() in cmd_commit(), rewrite it to
call wt_status_locate_end() helper instead and remove the old helper
that no longer has any caller.

Signed-off-by: Brian Malehorn <bmalehorn@gmail.com>
---
 builtin/commit.c              |  2 +-
 commit.c                      | 13 +++++++------
 t/t7513-interpret-trailers.sh | 17 +++++++++++++++++
 wt-status.c                   | 11 ++++++-----
 wt-status.h                   |  2 +-
 5 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 2de5f6cc6..6c606d965 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1735,7 +1735,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	if (verbose || /* Truncate the message just before the diff, if any. */
 	    cleanup_mode == CLEANUP_SCISSORS)
-		wt_status_truncate_message_at_cut_line(&sb);
+		strbuf_setlen(&sb, wt_status_locate_end(sb.buf, sb.len));
 
 	if (cleanup_mode != CLEANUP_NONE)
 		strbuf_stripspace(&sb, cleanup_mode == CLEANUP_ALL);
diff --git a/commit.c b/commit.c
index fab826973..fa353acac 100644
--- a/commit.c
+++ b/commit.c
@@ -11,6 +11,7 @@
 #include "commit-slab.h"
 #include "prio-queue.h"
 #include "sha1-lookup.h"
+#include "wt-status.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -1649,10 +1650,9 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
 /*
  * Inspect the given string and determine the true "end" of the log message, in
  * order to find where to put a new Signed-off-by: line.  Ignored are
- * trailing comment lines and blank lines, and also the traditional
- * "Conflicts:" block that is not commented out, so that we can use
- * "git commit -s --amend" on an existing commit that forgot to remove
- * it.
+ * trailing comment lines and blank lines.  To support "git commit -s
+ * --amend" on an existing commit, we also ignore "Conflicts:".  To
+ * support "git commit -v", we truncate at cut lines.
  *
  * Returns the number of bytes from the tail to ignore, to be fed as
  * the second parameter to append_signoff().
@@ -1662,8 +1662,9 @@ int ignore_non_trailer(const char *buf, size_t len)
 	int boc = 0;
 	int bol = 0;
 	int in_old_conflicts_block = 0;
+	size_t cutoff = wt_status_locate_end(buf, len);
 
-	while (bol < len) {
+	while (bol < cutoff) {
 		const char *next_line = memchr(buf + bol, '\n', len - bol);
 
 		if (!next_line)
@@ -1689,5 +1690,5 @@ int ignore_non_trailer(const char *buf, size_t len)
 		}
 		bol = next_line - buf;
 	}
-	return boc ? len - boc : 0;
+	return boc ? len - boc : len - cutoff;
 }
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 4dd1d7c52..0c6f91c43 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -1258,4 +1258,21 @@ test_expect_success 'with no command and no key' '
 	test_cmp expected actual
 '
 
+test_expect_success 'with cut line' '
+	cat >expected <<-\EOF &&
+		my subject
+
+		review: Brian
+		sign: A U Thor <author@example.com>
+		# ------------------------ >8 ------------------------
+		ignore this
+	EOF
+	git interpret-trailers --trailer review:Brian >actual <<-\EOF &&
+		my subject
+		# ------------------------ >8 ------------------------
+		ignore this
+	EOF
+	test_cmp expected actual
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index 4bb46781c..afff86c18 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -883,17 +883,18 @@ static void wt_longstatus_print_other(struct wt_status *s,
 	status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 }
 
-void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
+size_t wt_status_locate_end(const char *s, size_t len)
 {
 	const char *p;
 	struct strbuf pattern = STRBUF_INIT;
 
 	strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line);
-	if (starts_with(buf->buf, pattern.buf + 1))
-		strbuf_setlen(buf, 0);
-	else if ((p = strstr(buf->buf, pattern.buf)))
-		strbuf_setlen(buf, p - buf->buf + 1);
+	if (starts_with(s, pattern.buf + 1))
+		len = 0;
+	else if ((p = strstr(s, pattern.buf)))
+		len = p - s + 1;
 	strbuf_release(&pattern);
+	return len;
 }
 
 void wt_status_add_cut_line(FILE *fp)
diff --git a/wt-status.h b/wt-status.h
index 54fec7703..889a8d682 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -112,7 +112,7 @@ struct wt_status_state {
 	unsigned char cherry_pick_head_sha1[20];
 };
 
-void wt_status_truncate_message_at_cut_line(struct strbuf *);
+size_t wt_status_locate_end(const char *s, size_t len);
 void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
-- 
2.12.3.9.g050893b


  reply	other threads:[~2017-05-16  6:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-12  5:03 [PATCH 0/3] interpret-trailers + commit -v bugfix Brian Malehorn
2017-05-12  5:03 ` [PATCH 1/3] mailinfo.c: is_scissors_line ends on newline Brian Malehorn
2017-05-12  8:31   ` Jeff King
2017-05-12  5:03 ` [PATCH 2/3] commit.c: add is_scissors_line Brian Malehorn
2017-05-12  8:40   ` Jeff King
2017-05-12  5:03 ` [PATCH 3/3] commit.c: skip scissors when computing trailers Brian Malehorn
2017-05-12  8:52   ` Jeff King
2017-05-12  9:00 ` [PATCH 0/3] interpret-trailers + commit -v bugfix Jeff King
2017-05-14  3:39   ` Brian Malehorn
2017-05-14  3:39     ` [PATCH] interpret-trailers: obey scissors lines Brian Malehorn
2017-05-14  3:56       ` Jeff King
2017-05-14  8:33         ` Brian Malehorn
2017-05-14  8:33           ` Brian Malehorn
2017-05-15  3:55             ` Junio C Hamano
2017-05-15  4:23               ` Junio C Hamano
2017-05-16  6:06                 ` Brian Malehorn
2017-05-16  6:06                   ` Brian Malehorn [this message]
2017-05-16  6:42                   ` Junio C Hamano
2017-05-15  3:08           ` Jeff King
2017-05-15  2:12         ` Junio C Hamano
2017-05-15  3:07           ` Jeff King
2017-05-15  3:32             ` Junio C Hamano
2017-05-15  3:33               ` Jeff King
2017-05-14  7:02       ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170516060649.29110-2-bmalehorn@gmail.com \
    --to=bmalehorn@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.