[PATCH v6 0/4] commit: Add commit.verbose configuration

This patch allows people to set commit.verbose to implicitly send
--verbose to git-commit.

It introduces several cleanup patches to t/t7505-commit-verbose.sh to
bring it closer to the current state of the tests as they have been
explained to me, then adds the verbose config and --no-verbose flag.

Since the last version of this patch
(http://thread.gmane.org/gmane.comp.version-control.git/251486), there
have been a couple of commit message clarifications and the body of the
check-for-no-diff script in the last patch was changed to simply negate
check-for-diff.

Caleb Thompson

------------------------------------------------------


Caleb Thompson (4):
  commit test: Use test_config instead of git-config
  commit test: Use write_script
  commit test: test_set_editor in each test
  commit: Add commit.verbose configuration

 Documentation/config.txt               |  5 +++
 Documentation/git-commit.txt           |  8 ++++-
 builtin/commit.c                       |  4 +++
 contrib/completion/git-completion.bash |  1 +
 t/t7507-commit-verbose.sh              | 64 +++++++++++++++++++++++++---------
 5 files changed, 64 insertions(+), 18 deletions(-)

--
2.0.0

Use write_script from t/test-lib-functions.sh instead of cat, shebang,
and chmod. This aids in readability for creating the script by using the
named function and allows us to turn off interpolation in the heredoc of
the script body to avoid extra escaping, since $SHELL_PATH is handled
for us.

Signed-off-by: Caleb Thompson <redacted>
Reviewed-by: Jeremiah Mahler <redacted>
---
 t/t7507-commit-verbose.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 6d778ed..db09107 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh

@@ -3,11 +3,9 @@
 test_description='verbose commit template'
 . ./test-lib.sh
 
-cat >check-for-diff <<EOF
-#!$SHELL_PATH
-exec grep '^diff --git' "\$1"
+write_script check-for-diff <<-'EOF'
+	exec grep '^diff --git' "$1"
 EOF
-chmod +x check-for-diff
 test_set_editor "$PWD/check-for-diff"
 
 cat >message <<'EOF'

-- 
2.0.0

Some of the tests in t/t7507-commit-verbose.sh were still using
git-config to set configuration. Change them to use the test_config
helper.

Signed-off-by: Caleb Thompson <redacted>
Reviewed-by: Jeremiah Mahler <redacted>
---
 t/t7507-commit-verbose.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 2ddf28c..6d778ed 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh

@@ -43,7 +43,7 @@ test_expect_success 'verbose diff is stripped out' '
 '
 
 test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
-	git config diff.mnemonicprefix true &&
+	test_config diff.mnemonicprefix true &&
 	git commit --amend -v &&
 	check_message message
 '

@@ -71,7 +71,7 @@ test_expect_success 'diff in message is retained with -v' '
 '
 
 test_expect_success 'submodule log is stripped out too with -v' '
-	git config diff.submodule log &&
+	test_config diff.submodule log &&
 	git submodule add ./. sub &&
 	git commit -m "sub added" &&
 	(

-- 
2.0.0

t/t7507-commit-verbose.sh was using a global test_set_editor call to
build its environment. The $EDITOR being used was not necessary for
all tests, and was in fact circumvented using subshells in some
cases.

To improve robustness against global state changes and avoid the use of
subshells to temporarily switch the editor as some were with
GIT_EDITOR=cat, set the editor explicitly wherever it will be important.

Specifically, in tests that need to check for the presence of a diff in the
editor, make calls to set_test_editor to set $EDITOR to check-for-diff
rather than relying on that editor being configured globally. This also
helps readers grok the tests as the setup is closer to the verification.

Signed-off-by: Caleb Thompson <redacted>
Reviewed-by: Jeremiah Mahler <redacted>
---
 t/t7507-commit-verbose.sh | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index db09107..35a4d06 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh

@@ -6,7 +6,6 @@ test_description='verbose commit template'
 write_script check-for-diff <<-'EOF'
 	exec grep '^diff --git' "$1"
 EOF
-test_set_editor "$PWD/check-for-diff"
 
 cat >message <<'EOF'
 subject

@@ -21,6 +20,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'initial commit shows verbose diff' '
+	test_set_editor "$PWD/check-for-diff" &&
 	git commit --amend -v
 '
 

@@ -36,11 +36,13 @@ check_message() {
 }
 
 test_expect_success 'verbose diff is stripped out' '
+	test_set_editor "$PWD/check-for-diff" &&
 	git commit --amend -v &&
 	check_message message
 '
 
 test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
+	test_set_editor "$PWD/check-for-diff" &&
 	test_config diff.mnemonicprefix true &&
 	git commit --amend -v &&
 	check_message message

@@ -77,20 +79,14 @@ test_expect_success 'submodule log is stripped out too with -v' '
 		echo "more" >>file &&
 		git commit -a -m "submodule commit"
 	) &&
-	(
-		GIT_EDITOR=cat &&
-		export GIT_EDITOR &&
-		test_must_fail git commit -a -v 2>err
-	) &&
+	test_set_editor cat &&
+	test_must_fail git commit -a -v 2>err &&
 	test_i18ngrep "Aborting commit due to empty commit message." err
 '
 
 test_expect_success 'verbose diff is stripped out with set core.commentChar' '
-	(
-		GIT_EDITOR=cat &&
-		export GIT_EDITOR &&
-		test_must_fail git -c core.commentchar=";" commit -a -v 2>err
-	) &&
+	test_set_editor cat &&
+	test_must_fail git -c core.commentchar=";" commit -a -v 2>err &&
 	test_i18ngrep "Aborting commit due to empty commit message." err
 '
 

-- 
2.0.0

Add a new configuration variable commit.verbose to implicitly pass
--verbose to git-commit. Ensure that --no-verbose to git-commit
negates that setting.

Signed-off-by: Caleb Thompson <redacted>
---
 Documentation/config.txt               |  5 +++++
 Documentation/git-commit.txt           |  8 +++++++-
 builtin/commit.c                       |  4 ++++
 contrib/completion/git-completion.bash |  1 +
 t/t7507-commit-verbose.sh              | 36 ++++++++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9f467d3..85ef016 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1017,6 +1017,11 @@ commit.template::
 	"`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
 	specified user's home directory.
 
+commit.verbose::
+	A boolean to enable/disable inclusion of diff information in the
+	commit message template when using an editor to prepare the commit
+	message.  Defaults to false.
+
 credential.helper::
 	Specify an external helper to be called when a username or
 	password credential is needed; the helper may consult external

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..8cb3439 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -282,7 +282,13 @@ configuration variable documented in linkgit:git-config[1].
 	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 '#'.
+	lines prefixed with '#'.  The `commit.verbose` configuration
+	variable can be set to true to implicitly send this option.
+
+--no-verbose::
+	Do not show the unified diff at the bottom of the commit message
+	template.  This is the default behavior, but can be used to override
+	the `commit.verbose` configuration variable.
 
 -q::
 --quiet::

diff --git a/builtin/commit.c b/builtin/commit.c
index 5e2221c..b746dc8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -1503,6 +1503,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 		sign_commit = git_config_bool(k, v) ? "" : NULL;
 		return 0;
 	}
+	if (!strcmp(k, "commit.verbose")) {
+		verbose = git_config_bool(k, v);
+		return 0;
+	}
 
 	status = git_gpg_config(k, v, NULL);
 	if (status)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 019026e..b686985 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -1980,6 +1980,7 @@ _git_config ()
 		color.ui
 		commit.status
 		commit.template
+		commit.verbose
 		core.abbrev
 		core.askpass
 		core.attributesfile

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 35a4d06..471bd8f 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh

@@ -7,6 +7,10 @@ write_script check-for-diff <<-'EOF'
 	exec grep '^diff --git' "$1"
 EOF
 
+write_script check-for-no-diff <<-'EOF'
+	! check-for-diff
+EOF
+
 cat >message <<'EOF'
 subject
 

@@ -48,6 +52,38 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
 	check_message message
 '
 
+test_expect_success 'commit shows verbose diff with commit.verbose true' '
+	echo morecontent >>file &&
+	git add file &&
+	test_config commit.verbose true &&
+	test_set_editor "$PWD/check-for-diff" &&
+	git commit --amend
+'
+
+test_expect_success 'commit --verbose overrides commit.verbose false' '
+	echo evenmorecontent >>file &&
+	git add file &&
+	test_config commit.verbose false  &&
+	test_set_editor "$PWD/check-for-diff" &&
+	git commit --amend --verbose
+'
+
+test_expect_success 'commit does not show verbose diff with commit.verbose false' '
+	echo evenmorecontent >>file &&
+	git add file &&
+	test_config commit.verbose false &&
+	test_set_editor "$PWD/check-for-no-diff" &&
+	git commit --amend
+'
+
+test_expect_success 'commit --no-verbose overrides commit.verbose true' '
+	echo evenmorecontent >>file &&
+	git add file &&
+	test_config commit.verbose true &&
+	test_set_editor "$PWD/check-for-no-diff" &&
+	git commit --amend --no-verbose
+'
+
 cat >diff <<'EOF'
 This is an example commit message that contains a diff.
 

-- 
2.0.0

On Tue, Jun 17, 2014 at 02:39:00PM -0500, Caleb Thompson wrote:

Don't you need "$PWD/" or similar here, since check-for-diff is not in our path?

Otherwise, your tests will always pass ("! check-for-diff" is always
true, since we cannot find it). I get (running with "-v"):

expecting success: 
        echo evenmorecontent >>file &&
        git add file &&
        test_config commit.verbose true &&
        test_set_editor "$PWD/check-for-no-diff" &&
        git commit --amend --no-verbose

/home/peff/compile/git/t/trash directory.t7507-commit-verbose/check-for-no-diff: 2: /home/peff/compile/git/t/trash directory.t7507-commit-verbose/check-for-no-diff: check-for-diff: not found

Other than that, the whole series looks OK to me.

-Peff

On Tue, Jun 17, 2014 at 14:38:56 -0500, Caleb Thompson wrote:

This patch allows people to set commit.verbose to implicitly send
--verbose to git-commit.

It introduces several cleanup patches to t/t7505-commit-verbose.sh to
bring it closer to the current state of the tests as they have been
explained to me, then adds the verbose config and --no-verbose flag.

Since the last version of this patch
(http://thread.gmane.org/gmane.comp.version-control.git/251486), there
have been a couple of commit message clarifications and the body of the
check-for-no-diff script in the last patch was changed to simply negate
check-for-diff.

Hi Caleb,

Do you intend to work further on this topic? I have been using a similar
(but less polished) patch, and would like to see this included in Git.

If you don't have time to work on it, I can incorporate the comments on
this version and submit a v7.


Torstein

Hey Torstein.

I'd planned to come back with a heavily simplified version of this that
didn't include any of the related changes, just the feature and the new
test, since a lot of that seemed controversial.

That said, I haven't done so and you're welcome to take a whack at this
feedback if you'd like.

Caleb Thompson

On Fri, Feb 27, 2015 at 10:13:36AM +0100, Torstein Hegge wrote:

On Tue, Jun 17, 2014 at 14:38:56 -0500, Caleb Thompson wrote:

quoted

This patch allows people to set commit.verbose to implicitly send
--verbose to git-commit.

It introduces several cleanup patches to t/t7505-commit-verbose.sh to
bring it closer to the current state of the tests as they have been
explained to me, then adds the verbose config and --no-verbose flag.

Since the last version of this patch
(http://thread.gmane.org/gmane.comp.version-control.git/251486), there
have been a couple of commit message clarifications and the body of the
check-for-no-diff script in the last patch was changed to simply negate
check-for-diff.

Hi Caleb,

Do you intend to work further on this topic? I have been using a similar
(but less polished) patch, and would like to see this included in Git.

If you don't have time to work on it, I can incorporate the comments on
this version and submit a v7.


Torstein