Re: [RFC/PATCH] commit notes workflow

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [RFC/PATCH] commit notes workflow

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:42

Sverre Rabbelier [off-list ref] writes:
Can someone explain why -v does not output it's data prefixed by a
'#'? If someone really wanted to include it in their commit message
they can column-select-delete it, and if they don't, it just gets
deleted by the cleanup code?
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.

Re: [RFC/PATCH] commit notes workflow

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:50:42

Heya,

On Thu, Mar 3, 2011 at 04:50, Junio C Hamano [off-list ref] 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?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] commit, status: #comment diff output in verbose mode

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:50:42

Heya,

On Thu, Mar 3, 2011 at 12:23, Ian Ward Comfort [off-list ref] wrote:
How about something like this?
Yup, that's what I meant :)

-- 
Cheers,

Sverre Rabbelier

[PATCH] commit, status: #comment diff output in verbose mode

From: Ian Ward Comfort <hidden>
Date: 2016-06-15 22:50:42

On 3 Mar 2011, at 3:12 AM, Sverre Rabbelier wrote:
On Thu, Mar 3, 2011 at 04:50, Junio C Hamano [off-list ref] wrote:
quoted
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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help