git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Caleb Thompson <caleb@calebthompson.io>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Jeremiah Mahler <jmmahler@gmail.com>,
	Duy Nguyen <pclouds@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Johannes Sixt <j.sixt@viscovery.net>, David Kastrup <dak@gnu.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v4 4/4] commit: support commit.verbose and --no-verbose
Date: Wed, 11 Jun 2014 16:26:30 -0500	[thread overview]
Message-ID: <20140611212247.GB30119@sirius.local> (raw)
In-Reply-To: <5398BEDE.1040305@web.de>

[-- 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 --]

  reply	other threads:[~2014-06-11 21:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-11 18:24 [PATCH v4 0/4] commit: support commit.verbose and --no-verbose caleb
2014-06-11 18:24 ` [PATCH v4 1/4] commit test: Use test_config instead of git-config caleb
2014-06-12  8:41   ` Jeremiah Mahler
2014-06-12 13:51     ` Caleb Thompson
2014-06-12 18:04       ` Jeremiah Mahler
2014-06-12 18:08         ` Jeremiah Mahler
2014-06-12 18:23           ` Caleb Thompson
2014-06-12 18:46             ` Jeremiah Mahler
2014-06-12 19:23               ` Caleb Thompson
2014-06-11 18:24 ` [PATCH v4 2/4] commit test: Use write_script caleb
2014-06-11 18:24 ` [PATCH v4 3/4] commit test: test_set_editor in each test caleb
2014-06-11 18:24 ` [PATCH v4 4/4] commit: support commit.verbose and --no-verbose caleb
2014-06-11 20:41   ` René Scharfe
2014-06-11 21:26     ` Caleb Thompson [this message]
2014-06-12 14:36       ` Caleb Thompson

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=20140611212247.GB30119@sirius.local \
    --to=caleb@calebthompson.io \
    --cc=dak@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=jmmahler@gmail.com \
    --cc=l.s.r@web.de \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).