public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Jens Lehmann <Jens.Lehmann@web•de>
Cc: Jeff King <peff@peff•net>, Johannes Sixt <j6t@kdbg•org>,
	Ari Pollak <ari@debian•org>,
	git@vger•kernel.org
Subject: Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message
Date: Tue, 19 Nov 2013 11:42:35 -0800	[thread overview]
Message-ID: <xmqqk3g489g4.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqob5g89y0.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Tue, 19 Nov 2013 11:31:51 -0800")

Junio C Hamano <gitster@pobox•com> writes:

> Jens Lehmann <Jens.Lehmann@web•de> writes:
>
>> diff --git a/wt-status.h b/wt-status.h
>> index 6c29e6f..cd2709f 100644
>> --- a/wt-status.h
>> +++ b/wt-status.h
>> @@ -91,6 +91,8 @@ struct wt_status_state {
>>  	unsigned char cherry_pick_head_sha1[20];
>>  };
>>
>> +const char wt_status_diff_divider[];
>
> This gives me:
>
> ./wt-status.h:94:12: error: array 'wt_status_diff_divider' assumed to have one element [-Werror]
> cc1: all warnings being treated as errors
>
> which is a bit unfortunate.
>
> Regardless of that, from the API design standpoint, I think it may
> be much better not to expose this particular implementation element
> (i.e. the array) to the caller, but instead to export a helper
> function that takes a pointer to a piece of memory and let callers
> ask an "I have this line---is it the status cut mark?" question.

That is, something like this, perhaps.

 builtin/commit.c          |  9 +++------
 t/t7507-commit-verbose.sh | 15 ++++++++++++++-
 wt-status.c               | 15 +++++++++++++++
 wt-status.h               |  1 +
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..fedb45a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf author_ident = STRBUF_INIT;
 	const char *index_file, *reflog_msg;
-	char *nl, *p;
+	char *nl;
 	unsigned char sha1[20];
 	struct ref_lock *ref_lock;
 	struct commit_list *parents = NULL, **pptr = &parents;
@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	}
 
 	/* Truncate the message just before the diff, if any. */
-	if (verbose) {
-		p = strstr(sb.buf, "\ndiff --git ");
-		if (p != NULL)
-			strbuf_setlen(&sb, p - sb.buf + 1);
-	}
+	if (verbose)
+		wt_status_truncate_message_at_cut_line(&sb);
 
 	if (cleanup_mode != CLEANUP_NONE)
 		stripspace(&sb, cleanup_mode == CLEANUP_ALL);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b..09c1150 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -65,9 +65,22 @@ test_expect_success 'diff in message is retained without -v' '
 	check_message diff
 '
 
-test_expect_failure 'diff in message is retained with -v' '
+test_expect_success 'diff in message is retained with -v' '
 	git commit --amend -F diff -v &&
 	check_message diff
 '
 
+test_expect_success 'submodule log is stripped out too with -v' '
+	git config diff.submodule log &&
+	git submodule add ./. sub &&
+	git commit -m "sub added" &&
+	(
+		cd sub &&
+		echo "more" >>file &&
+		git commit -a -m "submodule commit"
+	) &&
+	GIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&
+	test_i18ngrep "Aborting commit due to empty commit message." err
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..492506a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,9 @@
 #include "column.h"
 #include "strbuf.h"
 
+static const char wt_status_cut_line[] =
+"# ------------------------ >8 ------------------------\n";
+
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
 	GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
@@ -767,6 +770,15 @@ conclude:
 	status_printf_ln(s, GIT_COLOR_NORMAL, "");
 }
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
+{
+	const char *p;
+
+	p = strstr(buf->buf, wt_status_cut_line);
+	if (p && (p == buf->buf || p[-1] == '\n'))
+		strbuf_setlen(buf, p - buf->buf);
+}
+
 static void wt_status_print_verbose(struct wt_status *s)
 {
 	struct rev_info rev;
@@ -791,6 +803,9 @@ static void wt_status_print_verbose(struct wt_status *s)
 	 */
 	if (s->fp != stdout)
 		rev.diffopt.use_color = 0;
+	fprintf(s->fp, wt_status_cut_line);
+	fprintf(s->fp, _("# Do not touch the line above.\n"));
+	fprintf(s->fp, _("# Everything below will be removed.\n"));
 	run_diff_index(&rev, 1);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 6c29e6f..30a4812 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -91,6 +91,7 @@ struct wt_status_state {
 	unsigned char cherry_pick_head_sha1[20];
 };
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);

  reply	other threads:[~2013-11-19 19:42 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-10 21:49 Bug? diff.submodule=log adds text to commit -v message Ari Pollak
2013-11-11 20:41 ` Jens Lehmann
2013-11-11 20:48   ` Ari Pollak
2013-11-11 21:29     ` Jens Lehmann
2013-11-11 21:34       ` Jens Lehmann
2013-11-11 23:24       ` Jeff King
2013-11-12  7:46       ` Johannes Sixt
2013-11-12 22:17         ` Jens Lehmann
2013-11-12 22:20           ` Junio C Hamano
2013-11-13 18:37             ` Jens Lehmann
2013-11-13 20:04               ` Junio C Hamano
2013-11-16 22:52                 ` [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message Jens Lehmann
2013-11-17  0:22                   ` Eric Sunshine
2013-11-17  8:53                     ` Jeff King
2013-11-17  9:09                   ` Jeff King
2013-11-17 12:20                     ` Jens Lehmann
2013-11-18 16:01                     ` Junio C Hamano
2013-11-19 18:47                       ` [PATCH v2 ] " Jens Lehmann
2013-11-19 19:07                         ` Junio C Hamano
2013-11-19 19:31                         ` Junio C Hamano
2013-11-19 19:42                           ` Junio C Hamano [this message]
2013-11-19 20:34                         ` Junio C Hamano
2013-11-19 21:01                           ` Jens Lehmann

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=xmqqk3g489g4.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=Jens.Lehmann@web$(echo .)de \
    --cc=ari@debian$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=j6t@kdbg$(echo .)org \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox