[PATCH v4 0/4] commit: support commit.verbose and --no-verbose

[PATCH v4 0/4] commit: support commit.verbose and --no-verbose

[caleb]

This patch allows people to set commit.verbose to implicitly send
--verbose to git-commit. It also introduces --no-verbose to override the
configuration setting.

This version incorporates changes suggested by Eric Sunshine, Duy
Nguyen, and Jeremiah Mahler.

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

Since the last version of this patch
(http://marc.info/?l=git&m=140113063517118&w=2), I've made the following
changes:

* Remove the $PWD => $(pwd) change, as it was a misunderstanding of
  t/README (Thank you to Johannes and David for pointing that out and
  clarifying)
* Use consistent test naming (Thanks to Jeremiah)
* Switch from <<-EOF to <<-'EOF' without manual escaping in write_script
  calls for the test editors check-for-diff and check-for-no-diff. The
  proposed <<-\EOF syntax did not properly prevent the parameter
  substitution on my machine, as both Eric and the documentation
  suggested it should, but the quoted version seems to work fine.
  (Thanks to Eric)
* Fix extra test_set_editor invocations and broken %%-chains created as a result
  of the test_set_editor changes. (Thanks again to Eric)

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

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: support commit.verbose and --no-verbose

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

[PATCH v4 1/4] commit test: Use test_config instead of git-config

[caleb]

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 <caleb@calebthompson.io>
---
 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

[PATCH v4 2/4] commit test: Use write_script

[caleb]

Use write_script from t/test-lib-functions.sh instead of cat, shebang,
and chmod. This protects us from potential shell meta-characters in the
name of our trash directory, which would be interpreted if we set
$EDITOR directly.

Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
---
 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

[PATCH v4 3/4] commit test: test_set_editor in each test

[caleb]

t/t7507-commit-verbose.sh was using a global test_set_editor call to
build its environment.

Improve robustness against global state changes by having only tests
which intend to use the $EDITOR to check for presence of a diff in the
editor set up the test-editor to use check-for-diff rather than relying
upon the editor set once at script start.

Besides being in line with current practices, it also allows the tests
which set GIT_EDITOR=cat manually to avoid using a subshell and simplify
their logic.

Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
---
 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

[PATCH v4 4/4] commit: support commit.verbose and --no-verbose

[caleb]

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

Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
---
 Documentation/config.txt               |  5 +++++
 Documentation/git-commit.txt           |  8 +++++++-
 builtin/commit.c                       |  6 +++++-
 contrib/completion/git-completion.bash |  1 +
 t/t7507-commit-verbose.sh              | 36 ++++++++++++++++++++++++++++++++++
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cd2d651..ec51e1c 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 99c2044..c5b20c6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1489,6 +1489,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)
@@ -1556,7 +1560,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
	static struct wt_status s;
	static struct option builtin_commit_options[] = {
		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
-		OPT__VERBOSE(&verbose, N_("show diff in commit message template")),
+		OPT_BOOL('v', "verbose", &verbose, N_("show diff in commit message template")),

		OPT_GROUP(N_("Commit message options")),
		OPT_FILENAME('F', "file", &logfile, N_("read message from file")),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2c59a76..b8f4b94 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1976,6 +1976,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..512eef3 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
+	exec grep -v '^diff --git' "\$1"
+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

Re: [PATCH v4 4/4] commit: support commit.verbose and --no-verbose

[René Scharfe]

Am 11.06.2014 20:24, schrieb caleb@calebthompson.io:
> Add a new configuration variable commit.verbose to implicitly pass
> `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that
> setting.
>
> Signed-off-by: Caleb Thompson<caleb@calebthompson.io>
> ---
>   Documentation/config.txt               |  5 +++++
>   Documentation/git-commit.txt           |  8 +++++++-
>   builtin/commit.c                       |  6 +++++-
>   contrib/completion/git-completion.bash |  1 +
>   t/t7507-commit-verbose.sh              | 36 ++++++++++++++++++++++++++++++++++
>   5 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index cd2d651..ec51e1c 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 99c2044..c5b20c6 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1489,6 +1489,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)

> @@ -1556,7 +1560,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
> 	static struct wt_status s;
> 	static struct option builtin_commit_options[] = {
> 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
> -		OPT__VERBOSE(&verbose, N_("show diff in commit message template")),
> +		OPT_BOOL('v', "verbose", &verbose, N_("show diff in commit message template")),

Hmm, this hunk seems to make sure that there can only be two verbosity 
levels, 0 and 1, while before there could be higher levels (which 
produce the same output as verbose = 1).  This change is not mentioned 
in the commit message.  Why is it necessary?

NB: --no-verbose already works without this hunk.

> 		OPT_GROUP(N_("Commit message options")),
> 		OPT_FILENAME('F', "file", &logfile, N_("read message from file")),
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2c59a76..b8f4b94 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1976,6 +1976,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..512eef3 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
> +	exec grep -v '^diff --git' "\$1"
> +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
>

Re: [PATCH v4 4/4] commit: support commit.verbose and --no-verbose

[Caleb Thompson]

[-- Attachment #1: Type: text/plain, Size: 5638 bytes --]

On Wed, Jun 11, 2014, at 01:41 PM, René Scharfe wrote:
>> @@ -1556,7 +1560,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>  static struct wt_status s;
>>  static struct option builtin_commit_options[] = {
>>    OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
>> -    OPT__VERBOSE(&verbose, N_("show diff in commit message template")),
>> +    OPT_BOOL('v', "verbose", &verbose, N_("show diff in commit message template")),
>
>Hmm, this hunk seems to make sure that there can only be two verbosity
>levels, 0 and 1, while before there could be higher levels (which
>produce the same output as verbose = 1).  This change is not mentioned
>in the commit message.  Why is it necessary?
>
>NB: --no-verbose already works without this hunk.

I had no idea, but you're quite right. I suppose that option was just a no-op
before, or could cancel a previous --verbose flag?

Updated diff with that hunk removed follows. I rebuilt the branch and the tests
I added for --no-verbose work as expected, as does actually using the
flag/config with the local build.

Caleb Thompson

------------------------ >8 ------------------------

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

Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
---
 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 cd2d651..ec51e1c 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 99c2044..c782388 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1489,6 +1489,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 2c59a76..b8f4b94 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1976,6 +1976,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..512eef3 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
+ exec grep -v '^diff --git' "\$1"
+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

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Jeremiah Mahler]

On Wed, Jun 11, 2014 at 01:24:36PM -0500, caleb@calebthompson.io wrote:
> 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 <caleb@calebthompson.io>
> ---
>  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
> 

It is probably something dumb on my part but for some reason I cannot
apply this patch using 'git am'.  After I save the patch to a Maildir
from Mutt and run git am...

  jeri@hudson:~/git$ ./git-am ct1.patch
  Applying: commit test: Use test_config instead of git-config
  fatal: corrupt patch at line 15
  Patch failed at 0001 commit test: Use test_config instead of git-config
  The copy of the patch that failed is found in:
     /home/jeri/git/.git/rebase-apply/patch
  When you have resolved this problem, run "git am --continue".
  If you prefer to skip this patch, run "git am --skip" instead.
  To restore the original branch and stop patching, run "git am --abort".
  jeri@hudson:~/git$

The second patch in the series applies fine.  And I have applied other
patches this way without issue.  Can anyone confirm this problem?

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Caleb Thompson]

[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]

I assume that you're running it against master?

I rebased the branch yesterday, but I'll give it another shot. Obviously I
didn't use git-am, but I can try that.

Caleb Thompson

On Thu, Jun 12, 2014 at 01:41:52AM -0700, Jeremiah Mahler wrote:
> On Wed, Jun 11, 2014 at 01:24:36PM -0500, caleb@calebthompson.io wrote:
> > 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 <caleb@calebthompson.io>
> > ---
> >  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
> >
>
> It is probably something dumb on my part but for some reason I cannot
> apply this patch using 'git am'.  After I save the patch to a Maildir
> from Mutt and run git am...
>
>   jeri@hudson:~/git$ ./git-am ct1.patch
>   Applying: commit test: Use test_config instead of git-config
>   fatal: corrupt patch at line 15
>   Patch failed at 0001 commit test: Use test_config instead of git-config
>   The copy of the patch that failed is found in:
>      /home/jeri/git/.git/rebase-apply/patch
>   When you have resolved this problem, run "git am --continue".
>   If you prefer to skip this patch, run "git am --skip" instead.
>   To restore the original branch and stop patching, run "git am --abort".
>   jeri@hudson:~/git$
>
> The second patch in the series applies fine.  And I have applied other
> patches this way without issue.  Can anyone confirm this problem?
>
> --
> Jeremiah Mahler
> jmmahler@gmail.com
> http://github.com/jmahler

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 4/4] commit: support commit.verbose and --no-verbose

[Caleb Thompson]

[-- Attachment #1: Type: text/plain, Size: 4565 bytes --]

Another update to also use quotes around the check-for-no-diff script body.

If I don't get any more feedback in the next few hours I'll send another full
version of the patch.

Caleb Thompson

------------------------ >8 ------------------------

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

Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
---
 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 cd2d651..ec51e1c 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 99c2044..c782388 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1489,6 +1489,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 2c59a76..b8f4b94 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1976,6 +1976,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..402d6a1 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'
+ exec grep -v '^diff --git' "$1"
+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

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Jeremiah Mahler]

Caleb,

On Thu, Jun 12, 2014 at 08:51:31AM -0500, Caleb Thompson wrote:
> I assume that you're running it against master?
> 
> I rebased the branch yesterday, but I'll give it another shot. Obviously I
> didn't use git-am, but I can try that.
> 
> Caleb Thompson
> 
> On Thu, Jun 12, 2014 at 01:41:52AM -0700, Jeremiah Mahler wrote:
> > On Wed, Jun 11, 2014 at 01:24:36PM -0500, caleb@calebthompson.io wrote:
> > > 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 <caleb@calebthompson.io>
> > > ---
> > >  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
> > >
> >
> > It is probably something dumb on my part but for some reason I cannot
> > apply this patch using 'git am'.  After I save the patch to a Maildir
> > from Mutt and run git am...
> >
> >   jeri@hudson:~/git$ ./git-am ct1.patch
> >   Applying: commit test: Use test_config instead of git-config
> >   fatal: corrupt patch at line 15
> >   Patch failed at 0001 commit test: Use test_config instead of git-config
> >   The copy of the patch that failed is found in:
> >      /home/jeri/git/.git/rebase-apply/patch
> >   When you have resolved this problem, run "git am --continue".
> >   If you prefer to skip this patch, run "git am --skip" instead.
> >   To restore the original branch and stop patching, run "git am --abort".
> >   jeri@hudson:~/git$
> >
> > The second patch in the series applies fine.  And I have applied other
> > patches this way without issue.  Can anyone confirm this problem?
> >
> > --
> > Jeremiah Mahler
> > jmmahler@gmail.com
> > http://github.com/jmahler

Thanks for checking.  Unfortunately the problem persists for me.

It seems to be related to there not being a space at the left most part
of the patch.  It is just a tab with no space.  So 'git am' has a
problem because it is expecting to remove a space and there isn't one.
See the attached screenshots with tabs highlighted.

How are you preparing your patches.  'git format-patch' with a 'git
send-email'?

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Jeremiah Mahler]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On Thu, Jun 12, 2014 at 11:04:59AM -0700, Jeremiah Mahler wrote:
> Caleb,
> 
...
>
> Thanks for checking.  Unfortunately the problem persists for me.
> 
> It seems to be related to there not being a space at the left most part
> of the patch.  It is just a tab with no space.  So 'git am' has a
> problem because it is expecting to remove a space and there isn't one.
> See the attached screenshots with tabs highlighted.
> 
> How are you preparing your patches.  'git format-patch' with a 'git
> send-email'?
> 
> -- 
> Jeremiah Mahler
> jmmahler@gmail.com
> http://github.com/jmahler

Forgot the screenshots.  Here they are attached.

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler

[-- Attachment #2: patch-bad.png --]
[-- Type: image/png, Size: 20337 bytes --]

[-- Attachment #3: patch-good.png --]
[-- Type: image/png, Size: 21435 bytes --]

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Caleb Thompson]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

Jeremiah,

On Thu, Jun 12, 2014 at 11:04:59AM -0700, Jeremiah Mahler wrote:
>
> Thanks for checking.  Unfortunately the problem persists for me.
>
> It seems to be related to there not being a space at the left most part
> of the patch.  It is just a tab with no space.  So 'git am' has a
> problem because it is expecting to remove a space and there isn't one.
> See the attached screenshots with tabs highlighted.
>
> How are you preparing your patches.  'git format-patch' with a 'git
> send-email'?

I'm so glad you asked, because I've just been muddling through this.

I've been generating the cover page variously with request-pull or diff
--stat, then running a command like this, with --cc arguments added from
a list I've been keeping of people who respond to the threads:

    git send-email --compose --to=git@vger.kernel.org --thread \
        [--cc ...] --no-chain-reply origin/master...

Then I manually edit the subjects to add the version to the [PATCH N/M]
portions. I haven't been using format-patch.

It's interesting that only the first patch isn't applying. I'd love to
hear a better way.

Caleb

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Jeremiah Mahler]

Caleb,

On Thu, Jun 12, 2014 at 01:23:41PM -0500, Caleb Thompson wrote:
> Jeremiah,
> 
> On Thu, Jun 12, 2014 at 11:04:59AM -0700, Jeremiah Mahler wrote:
> >
...
> >
> > How are you preparing your patches.  'git format-patch' with a 'git
> > send-email'?
> 
> I'm so glad you asked, because I've just been muddling through this.
> 
> I've been generating the cover page variously with request-pull or diff
> --stat, then running a command like this, with --cc arguments added from
> a list I've been keeping of people who respond to the threads:
> 
>     git send-email --compose --to=git@vger.kernel.org --thread \
>         [--cc ...] --no-chain-reply origin/master...
> 
> Then I manually edit the subjects to add the version to the [PATCH N/M]
> portions. I haven't been using format-patch.
> 
> It's interesting that only the first patch isn't applying. I'd love to
> hear a better way.
> 
> Caleb

It sounds like you are doing too much work.

After I make a series of commits I run format-patch.  This example has 2
patches (-2).  I like --thread, although it seems to work fine without
it.  And --reroll-count will automatically do your N/M numbering for
you.  It also generates a cover letter which you can then edit by hand.
All the patches will be named v3-* in this case.

  git format-patch --reroll-count=3 --cover --thread -2

Then I can send the whole patch series using send-email.

  git send-email --to=caleb --cc=git --cc=junio v3-*

Felipe Contreras has a good writeup [1] on how to setup aliases with Mutt so
you don't have to type the full email every time.

[1]: http://felipec.wordpress.com/2009/10/25/git-send-email-tricks/

I recently setup Mutt with Offlineimap [2] and Msmtp.  This is an
awesome setup too.  Not patch related, but it makes the email part
easier.

[2]: https://wiki.archlinux.org/index.php/OfflineIMAP

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler

Re: [PATCH v4 1/4] commit test: Use test_config instead of git-config

[Caleb Thompson]

[-- Attachment #1: Type: text/plain, Size: 2266 bytes --]

Jeremiah,

That looks great. Since I'm not seeing any more feedback on the code for this
set of patches, I'll go ahead and send up another set.

Thank you for those pointers.

Caleb Thompson

On Thu, Jun 12, 2014 at 11:46:37AM -0700, Jeremiah Mahler wrote:
> Caleb,
>
> On Thu, Jun 12, 2014 at 01:23:41PM -0500, Caleb Thompson wrote:
> > Jeremiah,
> >
> > On Thu, Jun 12, 2014 at 11:04:59AM -0700, Jeremiah Mahler wrote:
> > >
> ...
> > >
> > > How are you preparing your patches.  'git format-patch' with a 'git
> > > send-email'?
> >
> > I'm so glad you asked, because I've just been muddling through this.
> >
> > I've been generating the cover page variously with request-pull or diff
> > --stat, then running a command like this, with --cc arguments added from
> > a list I've been keeping of people who respond to the threads:
> >
> >     git send-email --compose --to=git@vger.kernel.org --thread \
> >         [--cc ...] --no-chain-reply origin/master...
> >
> > Then I manually edit the subjects to add the version to the [PATCH N/M]
> > portions. I haven't been using format-patch.
> >
> > It's interesting that only the first patch isn't applying. I'd love to
> > hear a better way.
> >
> > Caleb
>
> It sounds like you are doing too much work.
>
> After I make a series of commits I run format-patch.  This example has 2
> patches (-2).  I like --thread, although it seems to work fine without
> it.  And --reroll-count will automatically do your N/M numbering for
> you.  It also generates a cover letter which you can then edit by hand.
> All the patches will be named v3-* in this case.
>
>   git format-patch --reroll-count=3 --cover --thread -2
>
> Then I can send the whole patch series using send-email.
>
>   git send-email --to=caleb --cc=git --cc=junio v3-*
>
> Felipe Contreras has a good writeup [1] on how to setup aliases with Mutt so
> you don't have to type the full email every time.
>
> [1]: http://felipec.wordpress.com/2009/10/25/git-send-email-tricks/
>
> I recently setup Mutt with Offlineimap [2] and Msmtp.  This is an
> awesome setup too.  Not patch related, but it makes the email part
> easier.
>
> [2]: https://wiki.archlinux.org/index.php/OfflineIMAP
>
> --
> Jeremiah Mahler
> jmmahler@gmail.com
> http://github.com/jmahler

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]