All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Ward Comfort <icomfort@stanford.edu>
To: Sverre Rabbelier <srabbelier@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Johan Herland <johan@herland.net>, Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: [PATCH] commit, status: #comment diff output in verbose mode
Date: Thu,  3 Mar 2011 03:23:39 -0800	[thread overview]
Message-ID: <1299151419-16027-1-git-send-email-icomfort@stanford.edu> (raw)
In-Reply-To: <AANLkTikzgGY8Fryfc7n2MYiL8ZvY1Vr0cj4QStAypwBf@mail.gmail.com>

On 3 Mar 2011, at 3:12 AM, Sverre Rabbelier wrote:
> On Thu, Mar 3, 2011 at 04:50, Junio C Hamano <gitster@pobox.com> wrote:
>> Good question.  There was no reason other than "that is just a historical
>> accident".
>>
>> The intention has always been "allow people to review the change for the
>> last time while writing a log message"; there was never a feature request
>> to allow the diff to be included---it would always have been an unwelcome
>> accident it that ever happened.
>
> In that case, can we change it to be that way now if it makes this case
> easier?

How about something like this?

--8<--

By historical accident, diffs included in commit templates and status
output when the "-v" option is given are not prefixed with the # comment
character, as other advice and status information is. Stripping these
lines is thus a best-effort operation, as it is not always possible to
tell which lines were generated by "-v" and which were inserted by the
user.

Improve this situation by adding the # prefix to diff output along with
all other status output in these cases. The change is simply made thanks
to a3c158d (Add a prefix output callback to diff output, 2010-05-26). The
prefixed diff can be stripped (or not, as configured) by the standard
cleanup code, so our special verbose-mode heuristic can be removed.

Documentation and a few tests which rely on the old "-v" format are
updated to match. One known breakage is fixed in t7507.

Signed-off-by: Ian Ward Comfort <icomfort@stanford.edu>
---
 Documentation/git-commit.txt |    3 +--
 builtin/commit.c             |    7 -------
 t/t4030-diff-textconv.sh     |    2 +-
 t/t7502-commit.sh            |    4 ++--
 t/t7507-commit-verbose.sh    |    4 ++--
 wt-status.c                  |   12 ++++++++++++
 6 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 8f89f6f..792f993 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -233,8 +233,7 @@ configuration variable documented in linkgit:git-config[1].
 --verbose::
 	Show unified diff between the HEAD commit and what
 	would be committed at the bottom of the commit message
-	template.  Note that this diff output doesn't have its
-	lines prefixed with '#'.
+	template.
 
 -q::
 --quiet::
diff --git a/builtin/commit.c b/builtin/commit.c
index 355b2cb..efecac3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1381,13 +1381,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		die("could not read commit message: %s", strerror(saved_errno));
 	}
 
-	/* 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 (cleanup_mode != CLEANUP_NONE)
 		stripspace(&sb, cleanup_mode == CLEANUP_ALL);
 	if (message_is_empty(&sb) && !allow_empty_message) {
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 88c5619..b00999e 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -79,7 +79,7 @@ test_expect_success 'format-patch produces binary' '
 test_expect_success 'status -v produces text' '
 	git reset --soft HEAD^ &&
 	git status -v >diff &&
-	find_diff <diff >actual &&
+	sed -e "s/^# //" <diff | find_diff >actual &&
 	test_cmp expect.text actual &&
 	git reset --soft HEAD@{1}
 '
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 50da034..a916001 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -151,8 +151,8 @@ test_expect_success 'verbose' '
 
 	echo minus >negative &&
 	git add negative &&
-	git status -v | sed -ne "/^diff --git /p" >actual &&
-	echo "diff --git a/negative b/negative" >expect &&
+	git status -v | sed -ne "/^# diff --git /p" >actual &&
+	echo "# diff --git a/negative b/negative" >expect &&
 	test_cmp expect actual
 
 '
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b..5b21bbb 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -5,7 +5,7 @@ test_description='verbose commit template'
 
 cat >check-for-diff <<EOF
 #!$SHELL_PATH
-exec grep '^diff --git' "\$1"
+exec grep '^# diff --git' "\$1"
 EOF
 chmod +x check-for-diff
 test_set_editor "$PWD/check-for-diff"
@@ -65,7 +65,7 @@ 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
 '
diff --git a/wt-status.c b/wt-status.c
index a82b11d..fc0063e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -32,6 +32,12 @@ static const char *color(int slot, struct wt_status *s)
 	return c;
 }
 
+static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void *data)
+{
+	assert(data);
+	return (struct strbuf *)data;
+}
+
 void wt_status_prepare(struct wt_status *s)
 {
 	unsigned char sha1[20];
@@ -588,6 +594,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 {
 	struct rev_info rev;
 	struct setup_revision_opt opt;
+	struct strbuf diff_output_prefix = STRBUF_INIT;
 
 	init_revisions(&rev, NULL);
 	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
@@ -596,10 +603,14 @@ static void wt_status_print_verbose(struct wt_status *s)
 	opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
 	setup_revisions(0, NULL, &rev, &opt);
 
+	strbuf_addstr(&diff_output_prefix, "# ");
+
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.file = s->fp;
 	rev.diffopt.close_file = 0;
+	rev.diffopt.output_prefix = diff_output_prefix_callback;
+	rev.diffopt.output_prefix_data = &diff_output_prefix;
 	/*
 	 * If we're not going to stdout, then we definitely don't
 	 * want color, since we are going to the commit message
@@ -609,6 +620,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	if (s->fp != stdout)
 		DIFF_OPT_CLR(&rev.diffopt, COLOR_DIFF);
 	run_diff_index(&rev, 1);
+	strbuf_release(&diff_output_prefix);
 }
 
 static void wt_status_print_tracking(struct wt_status *s)
-- 
1.7.4.1.177.g1c06f

  reply	other threads:[~2011-03-03 11:31 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-25 13:30 [RFC/PATCH] commit notes workflow Jeff King
2011-02-25 15:58 ` Johan Herland
2011-03-01 21:59   ` Jeff King
2011-03-02  0:21     ` Johan Herland
2011-03-03  1:57       ` Sverre Rabbelier
2011-03-03  3:50         ` Junio C Hamano
2011-03-03 11:12           ` Sverre Rabbelier
2011-03-03 11:23             ` Ian Ward Comfort [this message]
2011-03-03 11:25               ` [PATCH] commit, status: #comment diff output in verbose mode Sverre Rabbelier
2011-03-07 23:39       ` [RFC/PATCH] commit notes workflow Jeff King
2011-03-07 23:39         ` [PATCH 1/2] notes: make expand_notes_ref globally accessible Jeff King
2011-03-08  8:25           ` Johan Herland
2011-03-07 23:41         ` [PATCH 2/2] commit: allow editing notes in commit message editor Jeff King
2011-03-08  9:15           ` Johan Herland
2011-03-08 12:39         ` [RFC/PATCH] commit notes workflow Michel Lespinasse
2011-03-02  7:01     ` Chris Packham
2011-03-02 12:45       ` Drew Northup
2011-03-02 16:24       ` Piotr Krukowiecki
2011-02-25 18:59 ` Junio C Hamano
2011-02-25 20:30 ` Drew Northup
2011-03-01 22:00   ` Jeff King
2011-03-01 22:18     ` Drew Northup
2011-03-01 22:23       ` Jeff King
2011-03-01 22:26         ` Drew Northup
2011-02-27 14:31 ` Michael J Gruber
2011-03-01 22:01   ` Jeff King
2011-03-09  8:13 ` Yann Dirson

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=1299151419-16027-1-git-send-email-icomfort@stanford.edu \
    --to=icomfort@stanford.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    --cc=peff@peff.net \
    --cc=srabbelier@gmail.com \
    /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.