{"thread":{"id":"41677","subject":"[PATCH v5] commit: add a commit.verbose config variable","startedAt":"2016-03-12T06:41:23Z","lastAt":"2016-03-14T21:31:35Z","messageCount":5,"participants":["Pranit Bauva","Eric Sunshine"],"isPatch":true,"patchVersion":5,"patchTotal":null},"messages":[{"id":"280659","messageId":"01020153698edabd-73daa5db-3c3e-417b-8d83-6cbc04439d95-000000@eu-west-1.amazonses.com","threadId":"41677","inReplyTo":null,"subject":"[PATCH v5] commit: add a commit.verbose config variable","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-12T06:41:23Z","receivedAt":"2016-03-12T06:41:23Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Add commit.verbose configuration variable as a convenience for those\nwho always prefer --verbose.\n\nSigned-off-by: Pranit Bauva \nMentored-by: Eric Sunshine \n\n---\nThe previous versions of this patch are:\n - [v4] $gmane/288652\n - [v3] $gmane/288634\n - [v2] $gmane/288569\n - [v1] $gmane/287540\n\nThe changes with respect to the last version are :\n - Use the in built check-for-diff \"editor\" instead of cat and breaking\n the code unnecessarily.\n - Add test to check whether the verbose option did not break status.\n---\n Documentation/config.txt | 4 ++++\n Documentation/git-commit.txt | 3 ++-\n builtin/commit.c | 4 ++++\n t/t7507-commit-verbose.sh | 18 ++++++++++++++++++\n 4 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 01cca0a..9b93f6c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1110,6 +1110,10 @@ commit.template::\n \t\"`~/`\" is expanded to the value of `$HOME` and \"`~user/`\" to the\n \tspecified user's home directory.\n \n+commit.verbose::\n+\tA boolean to specify whether to always include the verbose option\n+\twith `git commit`. See linkgit:git-commit[1].\n+\n credential.helper::\n \tSpecify an external helper to be called when a username or\n \tpassword credential is needed; the helper may consult external\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 9ec6b3c..d474226 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -290,7 +290,8 @@ configuration variable documented in linkgit:git-config[1].\n \twhat changes the commit has.\n \tNote that this diff output doesn't have its\n \tlines prefixed with '#'. This diff will not be a part\n-\tof the commit message.\n+\tof the commit message. See the `commit.verbose` configuration\n+\tvariable in linkgit:git-config[1].\n +\n If specified twice, show in addition the unified diff between\n what would be committed and the worktree files, i.e. the unstaged\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex b3bd2d4..e0b96231 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"commit.verbose\")) {\n+\t\tverbose = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 2ddf28c..bb7ce7c 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -96,4 +96,22 @@ test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n \ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n '\n \n+test_expect_success 'commit.verbose true and --verbose omitted' '\n+\ttest_config commit.verbose true &&\n+\t! git status | grep \"*diff --git\" &&\n+\tgit commit --amend\n+'\n+\n+test_expect_success 'commit.verbose true and --no-verbose' '\n+\ttest_must_fail git -c commit.verbose=true commit --amend --no-verbose\n+'\n+\n+test_expect_success 'commit.verbose false and --verbose' '\n+\tgit -c commit.verbose=false commit --amend --verbose\n+'\n+\n+test_expect_success 'commit.verbose false and --verbose omitted' '\n+\ttest_must_fail git -c commit.verbose=false commit --amend\n+'\n+\n test_done\n\n--\nhttps://github.com/git/git/pull/205\n"},{"id":"280704","messageId":"CAPig+cQ+8+saxsGzV=-C2w-eSaaOf0MYC=zTQbH3CW2G5UE5uw@mail.gmail.com","threadId":"41677","inReplyTo":"01020153698edabd-73daa5db-3c3e-417b-8d83-6cbc04439d95-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v5] commit: add a commit.verbose config variable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-14T08:24:22Z","receivedAt":"2016-03-14T08:24:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Mar 12, 2016 at 1:41 AM, Pranit Bauva wrote:\n> Add commit.verbose configuration variable as a convenience for those\n> who always prefer --verbose.\n>\n> Signed-off-by: Pranit Bauva \n> Mentored-by: Eric Sunshine \n\nMore typical would be to say Helped-by: rather than Mentored-by:.\n\nAlso, place your sign-off last.\n\n> ---\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index 2ddf28c..bb7ce7c 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -96,4 +96,22 @@ test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n> +test_expect_success 'commit.verbose true and --verbose omitted' '\n> + test_config commit.verbose true &&\n> + ! git status | grep \"*diff --git\" &&\n\nThis is a bogus regular expression; some greps may barf on it\noutright, while others will treat \"*\" as a literal. Either way, it\ncould never match \"diff --git\" so the test would succeed by accident\neven if git-status did erroneously respect \"commit.verbose\".\n\n> + git commit --amend\n> +'\n\nI realize that it's possible to (mis)read Junio's recommendation[1]\nabout also testing git-status as a hint to combine the git-commit and\ngit-status checks into a single test, but that's not what was\nintended. These are conceptually distinct checks, thus they should\neach have a separate test. I'd suggest restoring the\n\"commit.verbose=true and --verbose omitted\" test as it was shown in\n[2], and then adding a new test in this script for git-status which\nlooks something like this:\n\n test_expect_success 'status ignores commit.verbose=true' '\n git status >actual &&\n ! grep \"^diff --git\" actual\n '\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/288648\n[2]: http://article.gmane.org/gmane.comp.version-control.git/288674\n\n> +test_expect_success 'commit.verbose true and --no-verbose' '\n> + test_must_fail git -c commit.verbose=true commit --amend --no-verbose\n> +'\n> +\n> +test_expect_success 'commit.verbose false and --verbose' '\n> + git -c commit.verbose=false commit --amend --verbose\n> +'\n> +\n> +test_expect_success 'commit.verbose false and --verbose omitted' '\n> + test_must_fail git -c commit.verbose=false commit --amend\n> +'\n\nNot at all mandatory, but it wouldn't hurt to add a couple additional tests:\n\n* commit.verbose=true and --verbose\n* commit.verbose=false and --no-verbose\n\n> test_done\n"},{"id":"280731","messageId":"CAFZEwPP_SZsk+B+1L93snQji=wg728P3Oq94oVJ+ZJkhmdC9Yw@mail.gmail.com","threadId":"41677","inReplyTo":"CAPig+cQ+8+saxsGzV=-C2w-eSaaOf0MYC=zTQbH3CW2G5UE5uw@mail.gmail.com","subject":"Re: [PATCH v5] commit: add a commit.verbose config variable","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-14T19:09:40Z","receivedAt":"2016-03-14T19:09:40Z","isPatch":true,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Mon, Mar 14, 2016 at 1:54 PM, Eric Sunshine wrote:\n> On Sat, Mar 12, 2016 at 1:41 AM, Pranit Bauva wrote:\n>> Add commit.verbose configuration variable as a convenience for those\n>> who always prefer --verbose.\n>>\n>> Signed-off-by: Pranit Bauva \n>> Mentored-by: Eric Sunshine \n\n> More typical would be to say Helped-by: rather than Mentored-by:.\n>\n> Also, place your sign-off last.\n\nSure!\n\n> This is a bogus regular expression; some greps may barf on it\n> outright, while others will treat \"*\" as a literal. Either way, it\n> could never match \"diff --git\" so the test would succeed by accident\n> even if git-status did erroneously respect \"commit.verbose\".\n\nI was unaware about this. Thanks for pointing it out\n\n> I realize that it's possible to (mis)read Junio's recommendation[1]\n> about also testing git-status as a hint to combine the git-commit and\n> git-status checks into a single test, but that's not what was\n> intended. These are conceptually distinct checks, thus they should\n> each have a separate test. I'd suggest restoring the\n> \"commit.verbose=true and --verbose omitted\" test as it was shown in\n> [2], and then adding a new test in this script for git-status which\n> looks something like this:\n>\n> test_expect_success 'status ignores commit.verbose=true' '\n> git status >actual &&\n> ! grep \"^diff --git\" actual\n> '\n\nI misread Junio's recommendation. Will change this.\n\n> Not at all mandatory, but it wouldn't hurt to add a couple additional tests:\n>\n> * commit.verbose=true and --verbose\n> * commit.verbose=false and --no-verbose\n\nI was thinking of putting these tests as when I was debugging (simply\nby printing verbose variable), I found that when commit.verbose=true\nand --verbose the value of the variable `verbose` is 2. But then I\nthought it wouldn't be that useful. But since you have pointed it out\nnow, I will definitely include them.\n\nRegards,\nPranit Bauva\n"},{"id":"280745","messageId":"CAPig+cTt+Mi=gOREG2zU7vbtB6qk7dVS3mWeMBY37ZwFp_oZng@mail.gmail.com","threadId":"41677","inReplyTo":"CAFZEwPP_SZsk+B+1L93snQji=wg728P3Oq94oVJ+ZJkhmdC9Yw@mail.gmail.com","subject":"Re: [PATCH v5] commit: add a commit.verbose config variable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-14T21:29:28Z","receivedAt":"2016-03-14T21:29:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 14, 2016 at 3:09 PM, Pranit Bauva wrote:\n> On Mon, Mar 14, 2016 at 1:54 PM, Eric Sunshine wrote:\n>> Not at all mandatory, but it wouldn't hurt to add a couple additional tests:\n>>\n>> * commit.verbose=true and --verbose\n>> * commit.verbose=false and --no-verbose\n>\n> I was thinking of putting these tests as when I was debugging (simply\n> by printing verbose variable), I found that when commit.verbose=true\n> and --verbose the value of the variable `verbose` is 2. But then I\n> thought it wouldn't be that useful. But since you have pointed it out\n> now, I will definitely include them.\n\nThe '2' is a result of OPT_VERBOSE() supporting multiple levels of\nverbosity (that is, \"--verbose --verbose\" could give more noisy output\nthan \"--verbose\") for commands for which verbosity levels make sense.\n\nIn this case, I wasn't suggesting these news tests for that reason,\nbut merely to prove that the combinations behave in the expected way.\nBy the way, the just-submitted v6 seems to be lacking these new tests\n(though, as I said, they are not by any means mandatory).\n"},{"id":"280746","messageId":"CAPig+cSEPBKR+Hd-8hqppNMrBOFa1GKs70cK7hzkbaNApgg8Lw@mail.gmail.com","threadId":"41677","inReplyTo":"CAPig+cTt+Mi=gOREG2zU7vbtB6qk7dVS3mWeMBY37ZwFp_oZng@mail.gmail.com","subject":"Re: [PATCH v5] commit: add a commit.verbose config variable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-14T21:31:35Z","receivedAt":"2016-03-14T21:31:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 14, 2016 at 5:29 PM, Eric Sunshine wrote:\n> By the way, the just-submitted v6 seems to be lacking these new tests\n> (though, as I said, they are not by any means mandatory).\n\nIgnore me. They are present in v6, yet I somehow overlooked them.\n"}]}