{"thread":{"id":"36750","subject":"[PATCH v2] commit: support commit.verbose and --no-verbose","startedAt":"2014-05-25T06:24:27Z","lastAt":"2014-05-27T22:59:45Z","messageCount":25,"participants":["Caleb Thompson","Jeremiah Mahler","Duy Nguyen","Eric Sunshine","Johannes Sixt","David Kastrup","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"242657","messageId":"20140525062427.GA94219@sirius.att.net","threadId":"36750","inReplyTo":null,"subject":"[PATCH v2] commit: support commit.verbose and --no-verbose","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-25T06:24:27Z","receivedAt":"2014-05-25T06:24:27Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Incorporated changes from Duy Nguyen and Jeremiah Mahler.\n\nJeremiah, I didn't make the changes about `<<-EOF` or `test_expect_success`\nbecause I'm guessing that keeping the local style of the code intact is more\nimportant than using those. Do you think it makes sense to refactor the rest of\nthe test file (t/t7507-commit-verbose.sh) to use those? I could also change the\nother `git config` calls to use `test_config`.\n\nDuy, you were right about `-V`. Do you know of a simple way to add that\nshortened flag? `OPT_BOOL('v', \"verbose\", ...)` gives me `-v`, `--verbose`, and\n`--no-verbose`, but no `-V` as a shortened form of `--no-verbose`.\n\ncommit 1a49356b87c9028e68e731f34790c11a3075f736\nAuthor: Caleb Thompson <caleb@calebthompson.io>\nDate:   Fri May 23 11:47:44 2014 -0500\n\n    commit: support commit.verbose and --no-verbose\n\n    Add a new configuration variable commit.verbose to implicitly pass\n    `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that\n    setting.\n\n    Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n    Reviewed-by: Duy Nguyen <pclouds@gmail.com>\n    Reviewed-by: Jeremiah Mahler <jmmahler@gmail.com>\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1932e9b..a245928 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1009,6 +1009,11 @@ 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 enable/disable inclusion of diff information in the\n+\tcommit message template when using an editor to prepare the commit\n+\tmessage.  Defaults to false.\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 0bbc8f5..d7b50e2 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -282,7 +282,13 @@ configuration variable documented in linkgit:git-config[1].\n \tShow unified diff between the HEAD commit and what\n \twould be committed at the bottom of the commit message\n \ttemplate.  Note that this diff output doesn't have its\n-\tlines prefixed with '#'.\n+\tlines prefixed with '#'.  The `commit.verbose` configuration\n+\tvariable can be set to true to implicitly send this option.\n+\n+--no-verbose::\n+\tDo not show the unified diff  at the bottom of the commit message\n+\ttemplate.  This is the default behavior, but can be used to override\n+\tthe`commit.verbose` configuration variable.\n\n -q::\n --quiet::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9cfef6c..7978d7f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1417,6 +1417,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)\n@@ -1484,7 +1488,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstatic struct wt_status s;\n \tstatic struct option builtin_commit_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n-\t\tOPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n+\t\tOPT_BOOL('v', \"verbose\", &verbose, N_(\"show diff in commit message template\")),\n \n \t\tOPT_GROUP(N_(\"Commit message options\")),\n \t\tOPT_FILENAME('F', \"file\", &logfile, N_(\"read message from file\")),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2c59a76..b8f4b94 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1976,6 +1976,7 @@ _git_config ()\n \t\tcolor.ui\n \t\tcommit.status\n \t\tcommit.template\n+\t\tcommit.verbose\n \t\tcore.abbrev\n \t\tcore.askpass\n \t\tcore.attributesfile\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 2ddf28c..bea5d88 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -10,6 +10,12 @@ EOF\n chmod +x check-for-diff\n test_set_editor \"$PWD/check-for-diff\"\n \n+cat >check-for-no-diff <<EOF\n+#!$SHELL_PATH\n+exec grep -v '^diff --git' \"\\$1\"\n+EOF\n+chmod +x check-for-no-diff\n+\n cat >message <<'EOF'\n subject\n\n@@ -48,6 +54,21 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n \tcheck_message message\n '\n\n+test_expect_success 'commit shows verbose diff with set commit.verbose' '\n+\techo morecontent >file &&\n+\tgit add file &&\n+\ttest_config commit.verbose true &&\n+\tcheck_message message\n+'\n+\n+test_expect_success 'commit does not show verbose diff with --no-verbose' '\n+\techo morecontent >file &&\n+\tgit add file &&\n+\ttest_config commit.verbose true &&\n+\ttest_set_editor \"$PWD/check-for-no-diff\" &&\n+\tgit commit --amend --no-verbose\n+'\n+\n cat >diff <<'EOF'\n This is an example commit message that contains a diff.\n"},{"id":"242658","messageId":"20140525070210.GA18539@hudson.localdomain","threadId":"36750","inReplyTo":"20140525062427.GA94219@sirius.att.net","subject":"Re: [PATCH v2] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-25T07:02:10Z","receivedAt":"2014-05-25T07:02:10Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Sun, May 25, 2014 at 01:24:27AM -0500, Caleb Thompson wrote:\n...\n>  \twould be committed at the bottom of the commit message\n>  \ttemplate.  Note that this diff output doesn't have its\n> -\tlines prefixed with '#'.\n> +\tlines prefixed with '#'.  The `commit.verbose` configuration\n> +\tvariable can be set to true to implicitly send this option.\n> +\n> +--no-verbose::\n> +\tDo not show the unified diff  at the bottom of the commit message\n> +\ttemplate.  This is the default behavior, but can be used to override\n> +\tthe`commit.verbose` configuration variable.\n> \nWhy is there two spaces between \"diff  at\"?\nNeeds a space between \"the`comm\" -> \"the `comm\".\n\n> +cat >check-for-no-diff <<EOF\n> +#!$SHELL_PATH\n> +exec grep -v '^diff --git' \"\\$1\"\n> +EOF\n> +chmod +x check-for-no-diff\n> +\nMe personally, I would leave it like that for now, since that is\nthe style being used nearby.  We'll see what others have to say.\n\nI certainly wouldn't convert all the other cases to use\ntest_expect_success.  Leave that for another patch.\n\n> \n> @@ -48,6 +54,21 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n>  \tcheck_message message\n>  '\n> \n> +test_expect_success 'commit shows verbose diff with set commit.verbose' '\n> +\techo morecontent >file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose true &&\n> +\tcheck_message message\n> +'\n> +\n> +test_expect_success 'commit does not show verbose diff with --no-verbose' '\n> +\techo morecontent >file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose true &&\n> +\ttest_set_editor \"$PWD/check-for-no-diff\" &&\n> +\tgit commit --amend --no-verbose\n> +'\n> +\nI like those better with 'test_config' instead of 'git config', good.\n\nKeep working on it, it is looking better :-)\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242659","messageId":"20140525074413.GA26369@hudson.localdomain","threadId":"36750","inReplyTo":"20140525062427.GA94219@sirius.att.net","subject":"Re: [PATCH v2] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-25T07:44:13Z","receivedAt":"2014-05-25T07:44:13Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Sun, May 25, 2014 at 01:24:27AM -0500, Caleb Thompson wrote:\n> Incorporated changes from Duy Nguyen and Jeremiah Mahler.\n> \n...\n> \n> +test_expect_success 'commit shows verbose diff with set commit.verbose' '\n> +\techo morecontent >file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose true &&\n> +\tcheck_message message\n> +'\n\nThis test case doesn't appear to be checking for the verbose output.\nNo commit is made so it can't check for the presence of a diff.\n\"check_message message\" passes as it did in the test above this (not shown).\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242663","messageId":"CACsJy8DHqFQU5qZCBUXh3VibpL7Rrq_V9i5ZeddKv+VfyrP86g@mail.gmail.com","threadId":"36750","inReplyTo":"20140525062427.GA94219@sirius.att.net","subject":"Re: [PATCH v2] commit: support commit.verbose and --no-verbose","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-25T08:44:16Z","receivedAt":"2014-05-25T08:44:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 25, 2014 at 1:24 PM, Caleb Thompson <cjaysson@gmail.com> wrote:\n> Duy, you were right about `-V`. Do you know of a simple way to add that\n> shortened flag? `OPT_BOOL('v', \"verbose\", ...)` gives me `-v`, `--verbose`, and\n> `--no-verbose`, but no `-V` as a shortened form of `--no-verbose`.\n\nNo, I don't think parse_options() allows something like that. And we\nprobably don't want -V for --no-verbose unless it's very often used.\n-- \nDuy\n"},{"id":"242664","messageId":"CAPig+cSS9_u1zF5Zv2gZ=xbUjuLgk3FgyYzd1yEyQmyeO3gpcg@mail.gmail.com","threadId":"36750","inReplyTo":"20140525062427.GA94219@sirius.att.net","subject":"Re: [PATCH v2] commit: support commit.verbose and --no-verbose","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-05-25T10:23:18Z","receivedAt":"2014-05-25T10:23:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, May 25, 2014 at 2:24 AM, Caleb Thompson <cjaysson@gmail.com> wrote:\n> Incorporated changes from Duy Nguyen and Jeremiah Mahler.\n\nAs a courtesy to reviewers, it is helpful to provide a pointer to the\nprevious submission to give context for the new submission. For\ninstance, like this [1].\n\n[1]: http://git.661346.n2.nabble.com/commit-support-commit-verbose-and-no-verbose-td7611617.html\n\n> Jeremiah, I didn't make the changes about `<<-EOF` or `test_expect_success`\n> because I'm guessing that keeping the local style of the code intact is more\n> important than using those. Do you think it makes sense to refactor the rest of\n> the test file (t/t7507-commit-verbose.sh) to use those? I could also change the\n> other `git config` calls to use `test_config`.\n\nGenerally speaking, it is important to respect local style, however,\nit is also appropriate to include one or more cleanup patches before\nyour primary changes in order to bring the code in line with current\npractices. Conversion to test_config could be such a cleanup patch.\n\n> Duy, you were right about `-V`. Do you know of a simple way to add that\n> shortened flag? `OPT_BOOL('v', \"verbose\", ...)` gives me `-v`, `--verbose`, and\n> `--no-verbose`, but no `-V` as a shortened form of `--no-verbose`.\n\nAt this point, after your email commentary but before the actual\npatch, you should have a scissor line -->8-- so that \"git am\" can\nextract your patch automatically from the email.\n\n> commit 1a49356b87c9028e68e731f34790c11a3075f736\n\nDrop this line. It has no meaning outside of your local repository.\n\n> Author: Caleb Thompson <caleb@calebthompson.io>\n> Date:   Fri May 23 11:47:44 2014 -0500\n\nDitto for the date.\n\n>     commit: support commit.verbose and --no-verbose\n>\n>     Add a new configuration variable commit.verbose to implicitly pass\n>     `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that\n>     setting.\n\nThe commit message would read just as well or better without the backquotes.\n\n>     Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n>     Reviewed-by: Duy Nguyen <pclouds@gmail.com>\n>     Reviewed-by: Jeremiah Mahler <jmmahler@gmail.com>\n\nConsidering that the code in this patch has changed since v1, it's\nprobably not appropriate to add these Reviewed-by: lines.\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1932e9b..a245928 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1009,6 +1009,11 @@ commit.template::\n>         \"`~/`\" is expanded to the value of `$HOME` and \"`~user/`\" to the\n>         specified user's home directory.\n>\n> +commit.verbose::\n> +       A boolean to enable/disable inclusion of diff information in the\n> +       commit message template when using an editor to prepare the commit\n> +       message.  Defaults to false.\n> +\n>  credential.helper::\n>         Specify an external helper to be called when a username or\n>         password credential is needed; the helper may consult external\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index 0bbc8f5..d7b50e2 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -282,7 +282,13 @@ configuration variable documented in linkgit:git-config[1].\n>         Show unified diff between the HEAD commit and what\n>         would be committed at the bottom of the commit message\n>         template.  Note that this diff output doesn't have its\n> -       lines prefixed with '#'.\n> +       lines prefixed with '#'.  The `commit.verbose` configuration\n> +       variable can be set to true to implicitly send this option.\n> +\n> +--no-verbose::\n> +       Do not show the unified diff  at the bottom of the commit message\n\nAlready mentioned by Jeremiah: s/diff\\s+/diff /\n\n> +       template.  This is the default behavior, but can be used to override\n> +       the`commit.verbose` configuration variable.\n\nAlso already mentioned: s/the/the /\n\n>  -q::\n>  --quiet::\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 9cfef6c..7978d7f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1417,6 +1417,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n>                 sign_commit = git_config_bool(k, v) ? \"\" : NULL;\n>                 return 0;\n>         }\n> +       if (!strcmp(k, \"commit.verbose\")) {\n> +               verbose = git_config_bool(k, v);\n> +               return 0;\n> +       }\n>\n>         status = git_gpg_config(k, v, NULL);\n>         if (status)\n> @@ -1484,7 +1488,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>         static struct wt_status s;\n>         static struct option builtin_commit_options[] = {\n>                 OPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n> -               OPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n> +               OPT_BOOL('v', \"verbose\", &verbose, N_(\"show diff in commit message template\")),\n>\n>                 OPT_GROUP(N_(\"Commit message options\")),\n>                 OPT_FILENAME('F', \"file\", &logfile, N_(\"read message from file\")),\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 2c59a76..b8f4b94 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1976,6 +1976,7 @@ _git_config ()\n>                 color.ui\n>                 commit.status\n>                 commit.template\n> +               commit.verbose\n>                 core.abbrev\n>                 core.askpass\n>                 core.attributesfile\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index 2ddf28c..bea5d88 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -10,6 +10,12 @@ EOF\n>  chmod +x check-for-diff\n>  test_set_editor \"$PWD/check-for-diff\"\n\nThis is not a new problem, but since you copied and modified the\ntest_set_editor invocation for your own test (below), it can be\nmentioned that $(pwd) should be used rather than $PWD. See discussion\nof $(pwd) in t/README. A preparatory patch which fixes this would not\nbe unwelcome.\n\n> +cat >check-for-no-diff <<EOF\n> +#!$SHELL_PATH\n> +exec grep -v '^diff --git' \"\\$1\"\n> +EOF\n> +chmod +x check-for-no-diff\n\nwrite_script (from test-lib-functions.sh) would be a more appropriate\nand modern way to compose this script. If you're concerned about style\nconsistency, a cleanup patch before this one could employ write_script\nfor the check-for-diff script, as well.\n\nAlso, since this script is used by only the one test, current practice\nsuggests that script creation should be done within the test itself.\n\n>  cat >message <<'EOF'\n>  subject\n>\n> @@ -48,6 +54,21 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n>         check_message message\n>  '\n>\n> +test_expect_success 'commit shows verbose diff with set commit.verbose' '\n> +       echo morecontent >file &&\n\nIs your intention to add more content to 'file'? If so, use '>>'.\n\n> +       git add file &&\n> +       test_config commit.verbose true &&\n> +       check_message message\n\nAs Jeremiah pointed out, this test is not actually testing if\ncommit.verbose=true worked since it's not invoking git-commit. In\nfact, check_message is testing something unrelated. You probably meant\n\"git commit --amend\" rather than \"check_message message\"\n\n> +'\n> +\n> +test_expect_success 'commit does not show verbose diff with --no-verbose' '\n\nAs this is the only test which needs check-for-no-diff, it would be\nappropriate to move script creation (via write_script) here into the\ntest itself (unless you plan on adding more tests which invoke the\nscript).\n\n> +       echo morecontent >file &&\n> +       git add file &&\n\nAgain, since you're using '>' rather than '>>', you haven't actually\nchanged the content of the file since the last test, so this code\nserves no purpose.\n\n> +       test_config commit.verbose true &&\n> +       test_set_editor \"$PWD/check-for-no-diff\" &&\n\nAs noted above, use $(pwd) rather than $PWD.\n\nThis invocation of test_set_editor potentially breaks tests following\nthis one (including tests which may be added in the future) since it\nchanges the global state established by test_set_editor near the top\nof the script. To avoid such a problem, you could invoke\ntest_set_editor and git-commit in a subshell.\n\nAlternately, current practice would suggest that each test which\nrequires a particular editor should be responsible for setting it. As\nsuch, a preparatory patch could drop the global test_set_editor and\ninvoke it instead in each test which requires it. (In fact, there are\na couple tests which are still setting EDITOR manually, and these\ncould be converted to test_set_editor.)\n\n> +       git commit --amend --no-verbose\n> +'\n\nYou're missing some potential tests, such as:\n\ncommit.verbose = <unset> (optional)\ncommit.verbose = false\n--verbose overrides commit.verbose=false\n\n>  cat >diff <<'EOF'\n>  This is an example commit message that contains a diff.\n"},{"id":"242693","messageId":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"20140525062427.GA94219@sirius.att.net","subject":"[PATCH v3 0/5] commit: support commit.verbose and --no-verbose","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:21Z","receivedAt":"2014-05-26T18:56:21Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"This patch allows people to set commit.verbose to implicitly send\n--verbose to git-commit. It also introduces --no-verbose to\noverride the configuration setting.\n\nThis version incorporates changes suggested by Eric Sunshine, Duy\nNguyen, and Jeremiah Mahler.\n\nIt introduces several cleanup patches to t/t7505-commit-verbose.sh to\nbring it closer to the current state of the tests as Eric has explained\nthem to me, then adds the verbose config and --no-verbose flag.\n\nCaleb Thompson (5):\n      commit test: Use test_config instead of git-config\n      commit test: Change $PWD to $(pwd)\n      commit test: Use write_script\n      commit test: test_set_editor in each test\n      commit: support commit.verbose and --no-verbose\n\n Documentation/config.txt               |  5 ++++\n Documentation/git-commit.txt           |  8 +++++-\n builtin/commit.c                       |  6 ++++-\n contrib/completion/git-completion.bash |  1 +\n t/t7507-commit-verbose.sh              | 68 ++++++++++++++++++++++++++++++++++++-------------\n 5 files changed, 69 insertions(+), 19 deletions(-)\n"},{"id":"242689","messageId":"1401130586-93105-2-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"[PATCH v3 1/5] commit test: Use test_config instead of git-config","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:22Z","receivedAt":"2014-05-26T18:56:22Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Some of the tests in t/t7507-commit-verbose.sh were still using\ngit-config to set configuration. Change them to use the test_config\nhelper.\n\nSigned-off-by: Caleb Thompson <caleb@calebthompson.io>\n---\n t/t7507-commit-verbose.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 2ddf28c..6d778ed 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -43,7 +43,7 @@ test_expect_success 'verbose diff is stripped out' '\n '\n \n test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n-\tgit config diff.mnemonicprefix true &&\n+\ttest_config diff.mnemonicprefix true &&\n \tgit commit --amend -v &&\n \tcheck_message message\n '\n@@ -71,7 +71,7 @@ test_expect_success 'diff in message is retained with -v' '\n '\n \n test_expect_success 'submodule log is stripped out too with -v' '\n-\tgit config diff.submodule log &&\n+\ttest_config diff.submodule log &&\n \tgit submodule add ./. sub &&\n \tgit commit -m \"sub added\" &&\n \t(\n-- \n1.9.3\n"},{"id":"242690","messageId":"1401130586-93105-3-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"[PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:23Z","receivedAt":"2014-05-26T18:56:23Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n---\n t/t7507-commit-verbose.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 6d778ed..3b06d73 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -8,7 +8,7 @@ cat >check-for-diff <<EOF\n exec grep '^diff --git' \"\\$1\"\n EOF\n chmod +x check-for-diff\n-test_set_editor \"$PWD/check-for-diff\"\n+test_set_editor \"$(pwd)/check-for-diff\"\n \n cat >message <<'EOF'\n subject\n-- \n1.9.3\n"},{"id":"242692","messageId":"1401130586-93105-4-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"[PATCH v3 3/5] commit test: Use write_script","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:24Z","receivedAt":"2014-05-26T18:56:24Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Use write_script from t/test-lib-functions instead of cat, shebang, and\nchmod.\n\nSigned-off-by: Caleb Thompson <caleb@calebthompson.io>\n---\n t/t7507-commit-verbose.sh | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 3b06d73..e62d921 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -3,11 +3,9 @@\n test_description='verbose commit template'\n . ./test-lib.sh\n \n-cat >check-for-diff <<EOF\n-#!$SHELL_PATH\n-exec grep '^diff --git' \"\\$1\"\n+write_script check-for-diff <<-EOF\n+\texec grep '^diff --git' \"\\$1\"\n EOF\n-chmod +x check-for-diff\n test_set_editor \"$(pwd)/check-for-diff\"\n \n cat >message <<'EOF'\n-- \n1.9.3\n"},{"id":"242688","messageId":"1401130586-93105-5-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"[PATCH v3 4/5] commit test: test_set_editor in each test","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:25Z","receivedAt":"2014-05-26T18:56:25Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"t/t7507-commit-verbose.sh was using a global test_set_editor call to\nbuild its environment.\n\nRather than building global state with test_set_editor at the beginning\nof the file, move test_set_editor calls into each test.\n\nBesides being inline with current practices, it also allows the tests\nwhich required GIT_EDITOR=cat to avoid using a subshell and simplify\ntheir logic.\n\nSigned-off-by: Caleb Thompson <caleb@calebthompson.io>\n---\n t/t7507-commit-verbose.sh | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex e62d921..310b68b 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -6,7 +6,6 @@ test_description='verbose commit template'\n write_script check-for-diff <<-EOF\n \texec grep '^diff --git' \"\\$1\"\n EOF\n-test_set_editor \"$(pwd)/check-for-diff\"\n \n cat >message <<'EOF'\n subject\n@@ -21,10 +20,12 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'initial commit shows verbose diff' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \tgit commit --amend -v\n '\n \n test_expect_success 'second commit' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \techo content modified >file &&\n \tgit add file &&\n \tgit commit -F message\n@@ -36,11 +37,13 @@ check_message() {\n }\n \n test_expect_success 'verbose diff is stripped out' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \tgit commit --amend -v &&\n \tcheck_message message\n '\n \n test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \ttest_config diff.mnemonicprefix true &&\n \tgit commit --amend -v &&\n \tcheck_message message\n@@ -59,16 +62,19 @@ index 0000000..f95c11d\n EOF\n \n test_expect_success 'diff in message is retained without -v' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \tgit commit --amend -F diff &&\n \tcheck_message diff\n '\n \n test_expect_success 'diff in message is retained with -v' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \tgit commit --amend -F diff -v &&\n \tcheck_message diff\n '\n \n test_expect_success 'submodule log is stripped out too with -v' '\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n \ttest_config diff.submodule log &&\n \tgit submodule add ./. sub &&\n \tgit commit -m \"sub added\" &&\n@@ -77,20 +83,14 @@ test_expect_success 'submodule log is stripped out too with -v' '\n \t\techo \"more\" >>file &&\n \t\tgit commit -a -m \"submodule commit\"\n \t) &&\n-\t(\n-\t\tGIT_EDITOR=cat &&\n-\t\texport GIT_EDITOR &&\n-\t\ttest_must_fail git commit -a -v 2>err\n-\t) &&\n+\ttest_set_editor cat &&\n+\ttest_must_fail git commit -a -v 2>err\n \ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n '\n \n test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n-\t(\n-\t\tGIT_EDITOR=cat &&\n-\t\texport GIT_EDITOR &&\n-\t\ttest_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n-\t) &&\n+\ttest_set_editor cat &&\n+\ttest_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n \ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n '\n \n-- \n1.9.3\n"},{"id":"242691","messageId":"1401130586-93105-6-git-send-email-caleb@calebthompson.io","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"[PATCH v3 5/5] commit: support commit.verbose and --no-verbose","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T18:56:26Z","receivedAt":"2014-05-26T18:56:26Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Add a new configuration variable commit.verbose to implicitly pass\n`--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that\nsetting.\n\nSigned-off-by: Caleb Thompson <caleb@calebthompson.io>\n---\n Documentation/config.txt               |  5 +++++\n Documentation/git-commit.txt           |  8 +++++++-\n builtin/commit.c                       |  6 +++++-\n contrib/completion/git-completion.bash |  1 +\n t/t7507-commit-verbose.sh              | 36 ++++++++++++++++++++++++++++++++++\n 5 files changed, 54 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1932e9b..a245928 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1009,6 +1009,11 @@ 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 enable/disable inclusion of diff information in the\n+\tcommit message template when using an editor to prepare the commit\n+\tmessage.  Defaults to false.\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 0bbc8f5..8cb3439 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -282,7 +282,13 @@ configuration variable documented in linkgit:git-config[1].\n \tShow unified diff between the HEAD commit and what\n \twould be committed at the bottom of the commit message\n \ttemplate.  Note that this diff output doesn't have its\n-\tlines prefixed with '#'.\n+\tlines prefixed with '#'.  The `commit.verbose` configuration\n+\tvariable can be set to true to implicitly send this option.\n+\n+--no-verbose::\n+\tDo not show the unified diff at the bottom of the commit message\n+\ttemplate.  This is the default behavior, but can be used to override\n+\tthe `commit.verbose` configuration variable.\n \n -q::\n --quiet::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9cfef6c..7978d7f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1417,6 +1417,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)\n@@ -1484,7 +1488,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstatic struct wt_status s;\n \tstatic struct option builtin_commit_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress summary after successful commit\")),\n-\t\tOPT__VERBOSE(&verbose, N_(\"show diff in commit message template\")),\n+\t\tOPT_BOOL('v', \"verbose\", &verbose, N_(\"show diff in commit message template\")),\n \n \t\tOPT_GROUP(N_(\"Commit message options\")),\n \t\tOPT_FILENAME('F', \"file\", &logfile, N_(\"read message from file\")),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2c59a76..b8f4b94 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1976,6 +1976,7 @@ _git_config ()\n \t\tcolor.ui\n \t\tcommit.status\n \t\tcommit.template\n+\t\tcommit.verbose\n \t\tcore.abbrev\n \t\tcore.askpass\n \t\tcore.attributesfile\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 310b68b..b9eb317 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -7,6 +7,10 @@ write_script check-for-diff <<-EOF\n \texec grep '^diff --git' \"\\$1\"\n EOF\n \n+write_script check-for-no-diff <<-EOF\n+\texec grep -v '^diff --git' \"\\$1\"\n+EOF\n+\n cat >message <<'EOF'\n subject\n \n@@ -49,6 +53,38 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n \tcheck_message message\n '\n \n+test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n+\techo morecontent >>file &&\n+\tgit add file &&\n+\ttest_config commit.verbose true &&\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n+\tgit commit --amend\n+'\n+\n+test_expect_success 'commit --verbose overrides verbose=false' '\n+\techo evenmorecontent >>file &&\n+\tgit add file &&\n+\ttest_config commit.verbose false  &&\n+\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n+\tgit commit --amend --verbose\n+'\n+\n+test_expect_success 'commit does not show verbose diff with commit.verbose=false' '\n+\techo evenmorecontent >>file &&\n+\tgit add file &&\n+\ttest_config commit.verbose false &&\n+\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n+\tgit commit --amend\n+'\n+\n+test_expect_success 'commit --no-verbose overrides commit.verbose=true' '\n+\techo evenmorecontent >>file &&\n+\tgit add file &&\n+\ttest_config commit.verbose true &&\n+\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n+\tgit commit --amend --no-verbose\n+'\n+\n cat >diff <<'EOF'\n This is an example commit message that contains a diff.\n \n-- \n1.9.3\n"},{"id":"242698","messageId":"20140526203304.GA11888@hudson.localdomain","threadId":"36750","inReplyTo":"1401130586-93105-6-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 5/5] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-26T20:33:04Z","receivedAt":"2014-05-26T20:33:04Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"j\nOn Mon, May 26, 2014 at 01:56:26PM -0500, Caleb Thompson wrote:\n> Add a new configuration variable commit.verbose to implicitly pass\n>  \n...\n> +test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n> +\techo morecontent >>file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose true &&\n> +\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n> +\tgit commit --amend\n> +'\n> +\n> +test_expect_success 'commit --verbose overrides verbose=false' '\n> +\techo evenmorecontent >>file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose false  &&\n> +\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n> +\tgit commit --amend --verbose\n> +'\n> +\n> +test_expect_success 'commit does not show verbose diff with commit.verbose=false' '\n> +\techo evenmorecontent >>file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose false &&\n> +\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n> +\tgit commit --amend\n> +'\n> +\n> +test_expect_success 'commit --no-verbose overrides commit.verbose=true' '\n> +\techo evenmorecontent >>file &&\n> +\tgit add file &&\n> +\ttest_config commit.verbose true &&\n> +\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n> +\tgit commit --amend --no-verbose\n> +'\n> +\n...\n> \n\nIt appears that these tests still aren't checking to see if the\n\"verbose\" output appears in the commit message.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242699","messageId":"20140526204714.GA96869@sirius.att.net","threadId":"36750","inReplyTo":"20140526203304.GA11888@hudson.localdomain","subject":"Re: [PATCH v3 5/5] commit: support commit.verbose and --no-verbose","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T20:47:14Z","receivedAt":"2014-05-26T20:47:14Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"The editors, `check-for-diff` and `check-for-no-diffs`, are grepping for the\noutput and lack thereof, respectively.\n\nOn Mon, May 26, 2014 at 01:33:04PM -0700, Jeremiah Mahler wrote:\n> j\n> On Mon, May 26, 2014 at 01:56:26PM -0500, Caleb Thompson wrote:\n> > Add a new configuration variable commit.verbose to implicitly pass\n> >  \n> ...\n> > +test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n> > +\techo morecontent >>file &&\n> > +\tgit add file &&\n> > +\ttest_config commit.verbose true &&\n> > +\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n> > +\tgit commit --amend\n> > +'\n> > +\n> > +test_expect_success 'commit --verbose overrides verbose=false' '\n> > +\techo evenmorecontent >>file &&\n> > +\tgit add file &&\n> > +\ttest_config commit.verbose false  &&\n> > +\ttest_set_editor \"$(pwd)/check-for-diff\" &&\n> > +\tgit commit --amend --verbose\n> > +'\n> > +\n> > +test_expect_success 'commit does not show verbose diff with commit.verbose=false' '\n> > +\techo evenmorecontent >>file &&\n> > +\tgit add file &&\n> > +\ttest_config commit.verbose false &&\n> > +\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n> > +\tgit commit --amend\n> > +'\n> > +\n> > +test_expect_success 'commit --no-verbose overrides commit.verbose=true' '\n> > +\techo evenmorecontent >>file &&\n> > +\tgit add file &&\n> > +\ttest_config commit.verbose true &&\n> > +\ttest_set_editor \"$(pwd)/check-for-no-diff\" &&\n> > +\tgit commit --amend --no-verbose\n> > +'\n> > +\n> ...\n> > \n> \n> It appears that these tests still aren't checking to see if the\n> \"verbose\" output appears in the commit message.\n> \n> -- \n> Jeremiah Mahler\n> jmmahler@gmail.com\n> http://github.com/jmahler\n"},{"id":"242700","messageId":"20140526210035.GB11888@hudson.localdomain","threadId":"36750","inReplyTo":"CA+g4mq8iGNVm-2Uj8j2bJLDazaTS_U76BO9-jeS9Aw4RZnki5A@mail.gmail.com","subject":"Re: [PATCH v3 5/5] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-26T21:00:35Z","receivedAt":"2014-05-26T21:00:35Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Mon, May 26, 2014 at 03:39:55PM -0500, Caleb Thompson wrote:\n> The editors, `check-for-diff` and `check-for-no-diffs`, are grepping for\n> the output and lack thereof, respectively.\n...\n> >\n> > It appears that these tests still aren't checking to see if the\n> > \"verbose\" output appears in the commit message.\n> >\n> >\n\nOK, got it.  The editor, set by test_set_editor, is run as part of\nthe commit.  Thanks for explaining that.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242706","messageId":"20140526221425.GA20637@hudson.localdomain","threadId":"36750","inReplyTo":"1401130586-93105-6-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 5/5] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-26T22:14:25Z","receivedAt":"2014-05-26T22:14:25Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Caleb,\n\nOn Mon, May 26, 2014 at 01:56:26PM -0500, Caleb Thompson wrote:\n> Add a new configuration variable commit.verbose to implicitly pass\n> `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that\n> setting.\n> \n> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n> ---\n>  Documentation/config.txt               |  5 +++++\n>  '\n...\n>  \n> +test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n> +\techo morecontent >>file &&\n...\n> +'\n> +\n> +test_expect_success 'commit --verbose overrides verbose=false' '\n> +\techo evenmorecontent >>file &&\n...\n> +\n> +test_expect_success 'commit does not show verbose diff with commit.verbose=false' '\n> +\techo evenmorecontent >>file &&\n...\n> +'\n> +\n> +test_expect_success 'commit --no-verbose overrides commit.verbose=true' '\n> +\techo evenmorecontent >>file &&\n...\n> +'\n> +\n>  \n\nSome minor style nits...\n\nUse a consistent naming convention for your tests.  verbose=false looks\ndifferent than commit.verbose=false at first glance.  Also, since\n\"commit.verbose=false\" is an invalid syntax for a config option, I would\nremove the '=' and just make it \"commit.verbose false\".\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242708","messageId":"20140526223420.GB20637@hudson.localdomain","threadId":"36750","inReplyTo":"1401130586-93105-1-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 0/5] commit: support commit.verbose and --no-verbose","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-26T22:34:20Z","receivedAt":"2014-05-26T22:34:20Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Caleb,\n\nOn Mon, May 26, 2014 at 01:56:21PM -0500, Caleb Thompson wrote:\n> This patch allows people to set commit.verbose to implicitly send\n> --verbose to git-commit. It also introduces --no-verbose to\n> override the configuration setting.\n> \n> This version incorporates changes suggested by Eric Sunshine, Duy\n> Nguyen, and Jeremiah Mahler.\n> \n...\n> \n\nOther than the minor style issue I pointed out in another email, it looks\ngood, and the patch set works properly on my machine.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242709","messageId":"20140526224015.GA99174@sirius.att.net","threadId":"36750","inReplyTo":"20140526223420.GB20637@hudson.localdomain","subject":"Re: [PATCH v3 0/5] commit: support commit.verbose and --no-verbose","fromName":"Caleb Thompson","fromEmail":"cjaysson@gmail.com","sentAt":"2014-05-26T22:40:15Z","receivedAt":"2014-05-26T22:40:15Z","isPatch":true,"sender":{"key":"cjaysson@gmail.com","avatar":"https://gravatar.com/avatar/d0b3e333979bf3b7932399a11e5c3e523c501a64e6f419a4fe51f6cae0572259?d=mp&s=160"},"body":"Great, thanks Jeremiah!\n\nI made that change, and will send up another patch version in the next day or so\nwhile I wait on others who may have input.\n\nI'm really appreciative of everyone's feedback!\n\nCaleb\n\n------------------------------------>8----------------------------------\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex b9eb317..88de624 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -53,7 +53,7 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n        check_message message\n '\n \n-test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n+test_expect_success 'commit shows verbose diff with commit.verbose true' '\n        echo morecontent >>file &&\n        git add file &&\n        test_config commit.verbose true &&\n@@ -61,7 +61,7 @@ test_expect_success 'commit shows verbose diff with set commit.verbose=true' '\n        git commit --amend\n '\n \n-test_expect_success 'commit --verbose overrides verbose=false' '\n+test_expect_success 'commit --verbose overrides commit.verbose false' '\n        echo evenmorecontent >>file &&\n        git add file &&\n        test_config commit.verbose false  &&\n@@ -69,7 +69,7 @@ test_expect_success 'commit --verbose overrides verbose=false' '\n        git commit --amend --verbose\n '\n \n-test_expect_success 'commit does not show verbose diff with commit.verbose=false' '\n+test_expect_success 'commit does not show verbose diff with commit.verbose false' '\n        echo evenmorecontent >>file &&\n        git add file &&\n        test_config commit.verbose false &&\n@@ -77,7 +77,7 @@ test_expect_success 'commit does not show verbose diff with commit.verbose=false\n        git commit --amend\n '\n \n-test_expect_success 'commit --no-verbose overrides commit.verbose=true' '\n+test_expect_success 'commit --no-verbose overrides commit.verbose true' '\n        echo evenmorecontent >>file &&\n        git add file &&\n        test_config commit.verbose true &&\n\n\nOn Mon, May 26, 2014 at 03:34:20PM -0700, Jeremiah Mahler wrote:\n> Caleb,\n> \n> On Mon, May 26, 2014 at 01:56:21PM -0500, Caleb Thompson wrote:\n> > This patch allows people to set commit.verbose to implicitly send\n> > --verbose to git-commit. It also introduces --no-verbose to\n> > override the configuration setting.\n> > \n> > This version incorporates changes suggested by Eric Sunshine, Duy\n> > Nguyen, and Jeremiah Mahler.\n> > \n> ...\n> > \n> \n> Other than the minor style issue I pointed out in another email, it looks\n> good, and the patch set works properly on my machine.\n> \n> -- \n> Jeremiah Mahler\n> jmmahler@gmail.com\n> http://github.com/jmahler\n"},{"id":"242718","messageId":"538426D3.8090107@viscovery.net","threadId":"36750","inReplyTo":"1401130586-93105-3-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-05-27T05:46:59Z","receivedAt":"2014-05-27T05:46:59Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/26/2014 20:56, schrieb Caleb Thompson:\n> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n> ---\n>  t/t7507-commit-verbose.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index 6d778ed..3b06d73 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -8,7 +8,7 @@ cat >check-for-diff <<EOF\n>  exec grep '^diff --git' \"\\$1\"\n>  EOF\n>  chmod +x check-for-diff\n> -test_set_editor \"$PWD/check-for-diff\"\n> +test_set_editor \"$(pwd)/check-for-diff\"\n>  \n>  cat >message <<'EOF'\n>  subject\n\nWhy? I see no benefit. Both $PWD and $(pwd) work fine everywhere,\nincluding Windows, and the former is faster, particularly on Windows.\n\n-- Hannes\n"},{"id":"242719","messageId":"CAPig+cTRF5NFUagF6mLbBduJe8TgACTHdc7CbK_QRehZ-2DXqw@mail.gmail.com","threadId":"36750","inReplyTo":"538426D3.8090107@viscovery.net","subject":"Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-05-27T06:10:48Z","receivedAt":"2014-05-27T06:10:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 27, 2014 at 1:46 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Am 5/26/2014 20:56, schrieb Caleb Thompson:\n>> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n>> ---\n>>  t/t7507-commit-verbose.sh | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n>> index 6d778ed..3b06d73 100755\n>> --- a/t/t7507-commit-verbose.sh\n>> +++ b/t/t7507-commit-verbose.sh\n>> @@ -8,7 +8,7 @@ cat >check-for-diff <<EOF\n>>  exec grep '^diff --git' \"\\$1\"\n>>  EOF\n>>  chmod +x check-for-diff\n>> -test_set_editor \"$PWD/check-for-diff\"\n>> +test_set_editor \"$(pwd)/check-for-diff\"\n>>\n>>  cat >message <<'EOF'\n>>  subject\n>\n> Why? I see no benefit. Both $PWD and $(pwd) work fine everywhere,\n> including Windows, and the former is faster, particularly on Windows.\n\nPoor advice on my part when reviewing the previous round. When I had\nread in git/t/README (in the distant past):\n\n    When a test checks for an absolute path that a git command\n    generated, construct the expected value using $(pwd) rather than\n    $PWD, $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference\n    on Windows, where the shell (MSYS bash) mangles absolute path\n    names.  For details, see the commit message of 4114156ae9.\n\nI must have missed the word \"check\" in the first sentence.\n"},{"id":"242720","messageId":"20140527061448.GA25927@hudson.localdomain","threadId":"36750","inReplyTo":"538426D3.8090107@viscovery.net","subject":"Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-27T06:14:48Z","receivedAt":"2014-05-27T06:14:48Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Tue, May 27, 2014 at 07:46:59AM +0200, Johannes Sixt wrote:\n> Am 5/26/2014 20:56, schrieb Caleb Thompson:\n> > Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n> > ---\n> >  t/t7507-commit-verbose.sh | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> > index 6d778ed..3b06d73 100755\n> > --- a/t/t7507-commit-verbose.sh\n> > +++ b/t/t7507-commit-verbose.sh\n> > @@ -8,7 +8,7 @@ cat >check-for-diff <<EOF\n> >  exec grep '^diff --git' \"\\$1\"\n> >  EOF\n> >  chmod +x check-for-diff\n> > -test_set_editor \"$PWD/check-for-diff\"\n> > +test_set_editor \"$(pwd)/check-for-diff\"\n> >  \n> >  cat >message <<'EOF'\n> >  subject\n> \n> Why? I see no benefit. Both $PWD and $(pwd) work fine everywhere,\n> including Windows, and the former is faster, particularly on Windows.\n> \n> -- Hannes\n\nI don't know the technical details of why this change is needed.\nBut someone felt it was important enough to put in t/README.\n\n  - When a test checks for an absolute path that a git command generated,\n    construct the expected value using $(pwd) rather than $PWD,\n    $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on\n    Windows, where the shell (MSYS bash) mangles absolute path names.\n    For details, see the commit message of 4114156ae9.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242721","messageId":"53843206.3040902@viscovery.net","threadId":"36750","inReplyTo":"20140527061448.GA25927@hudson.localdomain","subject":"Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-05-27T06:34:46Z","receivedAt":"2014-05-27T06:34:46Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Please do not cull the Cc list.\n\nAm 5/27/2014 8:14, schrieb Jeremiah Mahler:\n> On Tue, May 27, 2014 at 07:46:59AM +0200, Johannes Sixt wrote:\n>> Am 5/26/2014 20:56, schrieb Caleb Thompson:\n>>> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n>>> ---\n>>>  t/t7507-commit-verbose.sh | 2 +-\n>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n>>> index 6d778ed..3b06d73 100755\n>>> --- a/t/t7507-commit-verbose.sh\n>>> +++ b/t/t7507-commit-verbose.sh\n>>> @@ -8,7 +8,7 @@ cat >check-for-diff <<EOF\n>>>  exec grep '^diff --git' \"\\$1\"\n>>>  EOF\n>>>  chmod +x check-for-diff\n>>> -test_set_editor \"$PWD/check-for-diff\"\n>>> +test_set_editor \"$(pwd)/check-for-diff\"\n>>>  \n>>>  cat >message <<'EOF'\n>>>  subject\n>>\n>> Why? I see no benefit. Both $PWD and $(pwd) work fine everywhere,\n>> including Windows, and the former is faster, particularly on Windows.\n> \n> I don't know the technical details of why this change is needed.\n> But someone felt it was important enough to put in t/README.\n> \n>   - When a test checks for an absolute path that a git command generated,\n>     construct the expected value using $(pwd) rather than $PWD,\n>     $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on\n>     Windows, where the shell (MSYS bash) mangles absolute path names.\n>     For details, see the commit message of 4114156ae9.\n\nThat someone was I. I appreciate that people study t/README and do not\nignore the sentence.\n\nHowever, it does not apply to the situation because the path to the editor\nis not \"generated by a git command and checked for by a test\".\n\nThat said, it is not wrong to use $(pwd) with test_set_editor, it's just\nunnecessarily slow.\n\n-- Hannes\n"},{"id":"242722","messageId":"87sinv3c8t.fsf@fencepost.gnu.org","threadId":"36750","inReplyTo":"53843206.3040902@viscovery.net","subject":"Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-27T07:35:30Z","receivedAt":"2014-05-27T07:35:30Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> That said, it is not wrong to use $(pwd) with test_set_editor, it's just\n> unnecessarily slow.\n\nAny shell that knows $(...) is pretty sure to have pwd as a built-in.\nI don't think Git will run on those kind of ancient shells reverting to\n/bin/pwd here.\n\nThe autoconf manual (info \"(autoconf) Limitations of Builtins\") states\n\n'pwd'\n     With modern shells, plain 'pwd' outputs a \"logical\" directory name,\n     some of whose components may be symbolic links.  These directory\n     names are in contrast to \"physical\" directory names, whose\n     components are all directories.\n\n     Posix 1003.1-2001 requires that 'pwd' must support the '-L'\n     (\"logical\") and '-P' (\"physical\") options, with '-L' being the\n     default.  However, traditional shells do not support these options,\n     and their 'pwd' command has the '-P' behavior.\n\n     Portable scripts should assume neither option is supported, and\n     should assume neither behavior is the default.  Also, on many hosts\n     '/bin/pwd' is equivalent to 'pwd -P', but Posix does not require\n     this behavior and portable scripts should not rely on it.\n\n     Typically it's best to use plain 'pwd'.  On modern hosts this\n     outputs logical directory names, which have the following\n     advantages:\n\n        * Logical names are what the user specified.\n        * Physical names may not be portable from one installation host\n          to another due to network file system gymnastics.\n        * On modern hosts 'pwd -P' may fail due to lack of permissions\n          to some parent directory, but plain 'pwd' cannot fail for this\n          reason.\n\n     Also please see the discussion of the 'cd' command.\n\nSo $PWD is pretty much guaranteed to be the same as $(pwd) and pretty\nmuch guaranteed to _not_ be \"unnecessarily slow\" when not run in an\ninner loop.\n\nHowever, looking at (info \"(autoconf) Special Shell Variables\") I see\n\n'PWD'\n     Posix 1003.1-2001 requires that 'cd' and 'pwd' must update the\n     'PWD' environment variable to point to the logical name of the\n     current directory, but traditional shells do not support this.\n     This can cause confusion if one shell instance maintains 'PWD' but\n     a subsidiary and different shell does not know about 'PWD' and\n     executes 'cd'; in this case 'PWD' points to the wrong directory.\n     Use '`pwd`' rather than '$PWD'.\n\nOk, probably Git relies on Posix 1003.1-2001 in other respects so it's\nlikely not much of an actual issue.\n\n-- \nDavid Kastrup\n"},{"id":"242820","messageId":"CAPig+cQt0mfBTChw8y=2Jg3rNsSr+neDCresptBafPDQixseXA@mail.gmail.com","threadId":"36750","inReplyTo":"1401130586-93105-4-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 3/5] commit test: Use write_script","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-05-27T22:30:42Z","receivedAt":"2014-05-27T22:30:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 26, 2014 at 2:56 PM, Caleb Thompson <cjaysson@gmail.com> wrote:\n> Use write_script from t/test-lib-functions instead of cat, shebang, and\n> chmod.\n>\n> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n> ---\n>  t/t7507-commit-verbose.sh | 6 ++----\n>  1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index 3b06d73..e62d921 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -3,11 +3,9 @@\n>  test_description='verbose commit template'\n>  . ./test-lib.sh\n>\n> -cat >check-for-diff <<EOF\n> -#!$SHELL_PATH\n> -exec grep '^diff --git' \"\\$1\"\n> +write_script check-for-diff <<-EOF\n> +       exec grep '^diff --git' \"\\$1\"\n\nFood for thought:\n\nThe original code used <<EOF since it needed $SHELL_PATH to be\nevaluated at script creation time, and took special care to escape $1\nin the 'grep' invocation since $1 should be evaluated only at script\nexecution time.\n\nWith the change to write_script(), nothing within the here-doc\nrequires evaluation, yet you are still using the evaluating <<-EOF\nform (and manually escaping $1). The intent might be clearer if you\nswitch to <<-\\EOF which suppresses evaluation (and drop the manual\nescaping of $1).\n\nThe same observation applies to the new write_script() invocation to\ncreate check-for-no-diff in patch 5.\n\n>  EOF\n> -chmod +x check-for-diff\n>  test_set_editor \"$(pwd)/check-for-diff\"\n>\n>  cat >message <<'EOF'\n> --\n> 1.9.3\n"},{"id":"242822","messageId":"xmqqoayietdc.fsf@gitster.dls.corp.google.com","threadId":"36750","inReplyTo":"CAPig+cQt0mfBTChw8y=2Jg3rNsSr+neDCresptBafPDQixseXA@mail.gmail.com","subject":"Re: [PATCH v3 3/5] commit test: Use write_script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-27T22:42:23Z","receivedAt":"2014-05-27T22:42:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> -cat >check-for-diff <<EOF\n>> -#!$SHELL_PATH\n>> -exec grep '^diff --git' \"\\$1\"\n>> +write_script check-for-diff <<-EOF\n>> +       exec grep '^diff --git' \"\\$1\"\n>\n> Food for thought:\n>\n> The original code used <<EOF since it needed $SHELL_PATH to be\n> evaluated at script creation time, and took special care to escape $1\n> in the 'grep' invocation since $1 should be evaluated only at script\n> execution time.\n>\n> With the change to write_script(), nothing within the here-doc\n> requires evaluation, yet you are still using the evaluating <<-EOF\n> form (and manually escaping $1). The intent might be clearer if you\n> switch to <<-\\EOF which suppresses evaluation (and drop the manual\n> escaping of $1).\n>\n> The same observation applies to the new write_script() invocation to\n> create check-for-no-diff in patch 5.\n\nVery good comments.  Thanks.\n"},{"id":"242824","messageId":"CAPig+cTc_sSxLm0QGxY40awnr9MD5NoKgc8pH+K67wwV+2p5-Q@mail.gmail.com","threadId":"36750","inReplyTo":"1401130586-93105-5-git-send-email-caleb@calebthompson.io","subject":"Re: [PATCH v3 4/5] commit test: test_set_editor in each test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-05-27T22:59:45Z","receivedAt":"2014-05-27T22:59:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 26, 2014 at 2:56 PM, Caleb Thompson <cjaysson@gmail.com> wrote:\n> t/t7507-commit-verbose.sh was using a global test_set_editor call to\n> build its environment.\n>\n> Rather than building global state with test_set_editor at the beginning\n> of the file, move test_set_editor calls into each test.\n\nRather than repeating in prose what the patch itself says more\nconcisely and precisely, explain the reason for this change. For\ninstance, you might replace the above two sentences with something\nlike this (or better):\n\n    Improve robustness against global state changes by having each\n    test set up the test-editor it requires rather than relying upon\n    the editor set once at script start.\n\n> Besides being inline with current practices, it also allows the tests\n\ns/inline/in line/\n\n> which required GIT_EDITOR=cat to avoid using a subshell and simplify\n> their logic.\n\n\"required\" sounds odd here. Perhaps:\n\n    ...which set GIT_EDITOR=cat manually...\n\nMore below.\n\n> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>\n> ---\n>  t/t7507-commit-verbose.sh | 22 +++++++++++-----------\n>  1 file changed, 11 insertions(+), 11 deletions(-)\n>\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index e62d921..310b68b 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -6,7 +6,6 @@ test_description='verbose commit template'\n>  write_script check-for-diff <<-EOF\n>         exec grep '^diff --git' \"\\$1\"\n>  EOF\n> -test_set_editor \"$(pwd)/check-for-diff\"\n>\n>  cat >message <<'EOF'\n>  subject\n> @@ -21,10 +20,12 @@ test_expect_success 'setup' '\n>  '\n>\n>  test_expect_success 'initial commit shows verbose diff' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         git commit --amend -v\n>  '\n>\n>  test_expect_success 'second commit' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         echo content modified >file &&\n>         git add file &&\n>         git commit -F message\n\nThis test does not invoke the test-editor at all, so it's misleading\nto insert test_set_editor here.\n\n> @@ -36,11 +37,13 @@ check_message() {\n>  }\n>\n>  test_expect_success 'verbose diff is stripped out' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         git commit --amend -v &&\n>         check_message message\n>  '\n>\n>  test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         test_config diff.mnemonicprefix true &&\n>         git commit --amend -v &&\n>         check_message message\n> @@ -59,16 +62,19 @@ index 0000000..f95c11d\n>  EOF\n>\n>  test_expect_success 'diff in message is retained without -v' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         git commit --amend -F diff &&\n>         check_message diff\n>  '\n\nAlso misleading, unnecessary test_set_editor.\n\n>  test_expect_success 'diff in message is retained with -v' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n>         git commit --amend -F diff -v &&\n>         check_message diff\n>  '\n\nDitto.\n\n>  test_expect_success 'submodule log is stripped out too with -v' '\n> +       test_set_editor \"$(pwd)/check-for-diff\" &&\n\nUnnecessary. The editor isn't invoked until the test_must_fail line,\nand by then you've already overridden it with 'test_set_editor cat'.\n\n>         test_config diff.submodule log &&\n>         git submodule add ./. sub &&\n>         git commit -m \"sub added\" &&\n> @@ -77,20 +83,14 @@ test_expect_success 'submodule log is stripped out too with -v' '\n>                 echo \"more\" >>file &&\n>                 git commit -a -m \"submodule commit\"\n>         ) &&\n> -       (\n> -               GIT_EDITOR=cat &&\n> -               export GIT_EDITOR &&\n> -               test_must_fail git commit -a -v 2>err\n> -       ) &&\n> +       test_set_editor cat &&\n> +       test_must_fail git commit -a -v 2>err\n\nBroken &&-chain.\n\n>         test_i18ngrep \"Aborting commit due to empty commit message.\" err\n>  '\n>\n>  test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n> -       (\n> -               GIT_EDITOR=cat &&\n> -               export GIT_EDITOR &&\n> -               test_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n> -       ) &&\n> +       test_set_editor cat &&\n> +       test_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n\nBroken &&-chain.\n\n>         test_i18ngrep \"Aborting commit due to empty commit message.\" err\n>  '\n>\n> --\n> 1.9.3\n>\n"}]}