{"thread":{"id":"49749","subject":"[PATCH] commit: add a commit.allowEmpty config variable","startedAt":"2018-11-03T11:25:51Z","lastAt":"2018-11-15T16:16:44Z","messageCount":14,"participants":["tanushree27","Duy Nguyen","Ævar Arnfjörð Bjarmason","Junio C Hamano","Tanushree Tumane","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"362356","messageId":"20181103112535.5730-1-tanushreetumane@gmail.com","threadId":"49749","inReplyTo":null,"subject":"[PATCH] commit: add a commit.allowEmpty config variable","fromName":"tanushree27","fromEmail":"tanushreetumane@gmail.com","sentAt":"2018-11-03T11:25:35Z","receivedAt":"2018-11-03T11:25:51Z","isPatch":true,"sender":{"key":"tanushreetumane@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22275398?v=4"},"body":"Add commit.allowEmpty configuration variable as a convenience for those\nwho always prefer --allow-empty.\n\nAdd tests to check the behavior introduced by this commit.\n\nThis closes https://github.com/git-for-windows/git/issues/1854\n\nSigned-off-by: tanushree27 <tanushreetumane@gmail.com>\n---\n Documentation/config.txt     |  5 +++++\n Documentation/git-commit.txt |  3 ++-\n builtin/commit.c             |  8 ++++++++\n t/t7500-commit.sh            | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 47 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c0727b7866..ac63b12ab3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1467,6 +1467,11 @@ commit.verbose::\n \tA boolean or int to specify the level of verbose with `git commit`.\n \tSee linkgit:git-commit[1].\n \n+commit.allowempty::\n+\tA boolean to specify whether empty commits are allowed with `git\n+\tcommit`. See linkgit:git-commit[1]. \n+\tDefaults 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 f970a43422..07a5b60ab9 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \tUsually recording a commit that has the exact same tree as its\n \tsole parent commit is a mistake, and the command prevents you\n \tfrom making such a commit.  This option bypasses the safety, and\n-\tis primarily for use by foreign SCM interface scripts.\n+\tis primarily for use by foreign SCM interface scripts. See\n+\t`commit.allowempty` in linkgit:git-config[1].\n \n --allow-empty-message::\n        Like --allow-empty this command is primarily for use by foreign\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 67fa949204..4516309ac2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n+static int config_commit_allow_empty = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit;\n@@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"commit.allowempty\")) {\n+\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n@@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n+\t\tallow_empty = config_commit_allow_empty;\n+\t\n \tif (dry_run)\n \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 170b4810e0..fb9bfbfb03 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -359,4 +359,36 @@ test_expect_success 'new line found before status message in commit template' '\n \ttest_i18ncmp expected-template editor-input\n '\n \n+# Tests for commit.allowempty config\n+\n+test_expect_success \"no commit.allowempty and no --allow-empty\" \"\n+\ttest_must_fail git commit -m 'test'\n+\"\n+\n+test_expect_success \"no commit.allowempty and --allow-empty\" \"\n+\tgit commit --allow-empty -m 'test'\n+\"\n+\n+for i in true 1\n+do\n+\ttest_expect_success \"commit.allowempty=$i and no --allow-empty\" \"\n+\t\tgit -c commit.allowempty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowempty=$i and --allow-empty\" \"\n+\t\tgit -c commit.allowempty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n+for i in false 0\n+do\n+\ttest_expect_success \"commit.allowempty=$i and no --allow-empty\" \"\n+\t\ttest_must_fail git -c commit.allowempty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowempty=$i and --allow-empty\" \"\n+\t\ttest_must_fail git -c commit.allowempty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n test_done\n-- \n2.19.1.windows.1.495.gd17cbd8b09\n\n"},{"id":"362358","messageId":"20181103115300.6518-1-tanushreetumane@gmail.com","threadId":"49749","inReplyTo":"20181103112535.5730-1-tanushreetumane@gmail.com","subject":"[[PATCH v2]] commit: add a commit.allowempty config variable","fromName":"tanushree27","fromEmail":"tanushreetumane@gmail.com","sentAt":"2018-11-03T11:53:00Z","receivedAt":"2018-11-03T11:53:14Z","isPatch":true,"sender":{"key":"tanushreetumane@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22275398?v=4"},"body":"Add commit.allowempty configuration variable as a convenience for those\nwho always prefer --allow-empty.\n\nAdd tests to check the behavior introduced by this commit.\n\nThis closes https://github.com/git-for-windows/git/issues/1854\n\nSigned-off-by: tanushree27 <tanushreetumane@gmail.com>\n---\n Documentation/config.txt     |  5 +++++\n Documentation/git-commit.txt |  3 ++-\n builtin/commit.c             |  8 ++++++++\n t/t7500-commit.sh            | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 47 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c0727b7866..ac63b12ab3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1467,6 +1467,11 @@ commit.verbose::\n \tA boolean or int to specify the level of verbose with `git commit`.\n \tSee linkgit:git-commit[1].\n \n+commit.allowempty::\n+\tA boolean to specify whether empty commits are allowed with `git\n+\tcommit`. See linkgit:git-commit[1]. \n+\tDefaults 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 f970a43422..07a5b60ab9 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \tUsually recording a commit that has the exact same tree as its\n \tsole parent commit is a mistake, and the command prevents you\n \tfrom making such a commit.  This option bypasses the safety, and\n-\tis primarily for use by foreign SCM interface scripts.\n+\tis primarily for use by foreign SCM interface scripts. See\n+\t`commit.allowempty` in linkgit:git-config[1].\n \n --allow-empty-message::\n        Like --allow-empty this command is primarily for use by foreign\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 67fa949204..4516309ac2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n+static int config_commit_allow_empty = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit;\n@@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"commit.allowempty\")) {\n+\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n@@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n+\t\tallow_empty = config_commit_allow_empty;\n+\t\n \tif (dry_run)\n \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 170b4810e0..fb9bfbfb03 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -359,4 +359,36 @@ test_expect_success 'new line found before status message in commit template' '\n \ttest_i18ncmp expected-template editor-input\n '\n \n+# Tests for commit.allowempty config\n+\n+test_expect_success \"no commit.allowempty and no --allow-empty\" \"\n+\ttest_must_fail git commit -m 'test'\n+\"\n+\n+test_expect_success \"no commit.allowempty and --allow-empty\" \"\n+\tgit commit --allow-empty -m 'test'\n+\"\n+\n+for i in true 1\n+do\n+\ttest_expect_success \"commit.allowempty=$i and no --allow-empty\" \"\n+\t\tgit -c commit.allowempty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowempty=$i and --allow-empty\" \"\n+\t\tgit -c commit.allowempty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n+for i in false 0\n+do\n+\ttest_expect_success \"commit.allowempty=$i and no --allow-empty\" \"\n+\t\ttest_must_fail git -c commit.allowempty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowempty=$i and --allow-empty\" \"\n+\t\ttest_must_fail git -c commit.allowempty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n test_done\n-- \n2.19.1.windows.1.495.gd17cbd8b09\n\n"},{"id":"362362","messageId":"CACsJy8DttJ2EBcN8Kq-yECY0Pvp3vd0Vx45=szWD0cBW0Mcixw@mail.gmail.com","threadId":"49749","inReplyTo":"20181103115300.6518-1-tanushreetumane@gmail.com","subject":"Re: [[PATCH v2]] commit: add a commit.allowempty config variable","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-03T14:43:06Z","receivedAt":"2018-11-03T14:43:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Nov 3, 2018 at 12:55 PM tanushree27 <tanushreetumane@gmail.com> wrote:\n>\n> Add commit.allowempty configuration variable as a convenience for those\n> who always prefer --allow-empty.\n>\n> Add tests to check the behavior introduced by this commit.\n>\n> This closes https://github.com/git-for-windows/git/issues/1854\n>\n> Signed-off-by: tanushree27 <tanushreetumane@gmail.com>\n> ---\n>  Documentation/config.txt     |  5 +++++\n>  Documentation/git-commit.txt |  3 ++-\n>  builtin/commit.c             |  8 ++++++++\n>  t/t7500-commit.sh            | 32 ++++++++++++++++++++++++++++++++\n>  4 files changed, 47 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index c0727b7866..ac63b12ab3 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1467,6 +1467,11 @@ commit.verbose::\n>         A boolean or int to specify the level of verbose with `git commit`.\n>         See linkgit:git-commit[1].\n>\n> +commit.allowempty::\n\nThe current naming convention is camelCase. So this should be commit.allowEmpty.\n\n> +       A boolean to specify whether empty commits are allowed with `git\n> +       commit`. See linkgit:git-commit[1].\n> +       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 f970a43422..07a5b60ab9 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n>         Usually recording a commit that has the exact same tree as its\n>         sole parent commit is a mistake, and the command prevents you\n>         from making such a commit.  This option bypasses the safety, and\n> -       is primarily for use by foreign SCM interface scripts.\n> +       is primarily for use by foreign SCM interface scripts. See\n> +       `commit.allowempty` in linkgit:git-config[1].\n\nSame.\n\n>\n>  --allow-empty-message::\n>         Like --allow-empty this command is primarily for use by foreign\n\n-- \nDuy\n"},{"id":"362363","messageId":"20181103151205.29122-1-tanushreetumane@gmail.com","threadId":"49749","inReplyTo":"CACsJy8DttJ2EBcN8Kq-yECY0Pvp3vd0Vx45=szWD0cBW0Mcixw@mail.gmail.com","subject":"[PATCH v3] commit: add a commit.allowEmpty config variable","fromName":"tanushree27","fromEmail":"tanushreetumane@gmail.com","sentAt":"2018-11-03T15:12:06Z","receivedAt":"2018-11-03T15:13:11Z","isPatch":true,"sender":{"key":"tanushreetumane@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22275398?v=4"},"body":"Add commit.allowEmpty configuration variable as a convenience for those\nwho always prefer --allow-empty.\n\nAdd tests to check the behavior introduced by this commit.\n\nThis closes https://github.com/git-for-windows/git/issues/1854\n\nSigned-off-by: tanushree27 <tanushreetumane@gmail.com>\n---\n Documentation/config.txt     |  5 +++++\n Documentation/git-commit.txt |  3 ++-\n builtin/commit.c             |  8 ++++++++\n t/t7500-commit.sh            | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 47 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c0727b7866..f3828518a5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1467,6 +1467,11 @@ commit.verbose::\n \tA boolean or int to specify the level of verbose with `git commit`.\n \tSee linkgit:git-commit[1].\n \n+commit.allowEmpty::\n+\tA boolean to specify whether empty commits are allowed with `git\n+\tcommit`. See linkgit:git-commit[1]. \n+\tDefaults 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 f970a43422..5d3bbf017a 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \tUsually recording a commit that has the exact same tree as its\n \tsole parent commit is a mistake, and the command prevents you\n \tfrom making such a commit.  This option bypasses the safety, and\n-\tis primarily for use by foreign SCM interface scripts.\n+\tis primarily for use by foreign SCM interface scripts. See\n+\t`commit.allowEmpty` in linkgit:git-config[1].\n \n --allow-empty-message::\n        Like --allow-empty this command is primarily for use by foreign\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 67fa949204..4516309ac2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n+static int config_commit_allow_empty = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit;\n@@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"commit.allowempty\")) {\n+\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n@@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n+\t\tallow_empty = config_commit_allow_empty;\n+\t\n \tif (dry_run)\n \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 170b4810e0..25a7facd53 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -359,4 +359,36 @@ test_expect_success 'new line found before status message in commit template' '\n \ttest_i18ncmp expected-template editor-input\n '\n \n+# Tests for commit.allowEmpty config\n+\n+test_expect_success \"no commit.allowEmpty and no --allow-empty\" \"\n+\ttest_must_fail git commit -m 'test'\n+\"\n+\n+test_expect_success \"no commit.allowEmpty and --allow-empty\" \"\n+\tgit commit --allow-empty -m 'test'\n+\"\n+\n+for i in true 1\n+do\n+\ttest_expect_success \"commit.allowEmpty=$i and no --allow-empty\" \"\n+\t\tgit -c commit.allowEmpty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowEmpty=$i and --allow-empty\" \"\n+\t\tgit -c commit.allowEmpty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n+for i in false 0\n+do\n+\ttest_expect_success \"commit.allowEmpty=$i and no --allow-empty\" \"\n+\t\ttest_must_fail git -C commit.allowEmpty=$i commit -m 'test'\n+\t\"\n+\n+\ttest_expect_success \"commit.allowEmpty=$i and --allow-empty\" \"\n+\t\ttest_must_fail git -c commit.allowEmpty=$i commit --allow-empty -m 'test'\n+\t\"\n+done\n+\n test_done\n-- \n2.19.1.windows.1.495.g9597888df3.dirty\n\n"},{"id":"362375","messageId":"87d0rm7zeo.fsf@evledraar.gmail.com","threadId":"49749","inReplyTo":"20181103151205.29122-1-tanushreetumane@gmail.com","subject":"Re: [PATCH v3] commit: add a commit.allowEmpty config variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-03T19:07:43Z","receivedAt":"2018-11-03T19:07:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Nov 03 2018, tanushree27 wrote:\n\n> +commit.allowEmpty::\n> +\tA boolean to specify whether empty commits are allowed with `git\n> +\tcommit`. See linkgit:git-commit[1].\n> +\tDefaults to false.\n> +\n\nGood.\n\n> +\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n> +\t\tallow_empty = config_commit_allow_empty;\n> +\n\nThis works, but != -1 is our usual idiom for this as you initialize it\nto -1. I think that comment can also go then, since it's clear what's\ngoing on.\n\n> +# Tests for commit.allowEmpty config\n> +\n> +test_expect_success \"no commit.allowEmpty and no --allow-empty\" \"\n> +\ttest_must_fail git commit -m 'test'\n> +\"\n> +\n> +test_expect_success \"no commit.allowEmpty and --allow-empty\" \"\n> +\tgit commit --allow-empty -m 'test'\n> +\"\n> +\n> +for i in true 1\n> +do\n> +\ttest_expect_success \"commit.allowEmpty=$i and no --allow-empty\" \"\n> +\t\tgit -c commit.allowEmpty=$i commit -m 'test'\n> +\t\"\n> +\n> +\ttest_expect_success \"commit.allowEmpty=$i and --allow-empty\" \"\n> +\t\tgit -c commit.allowEmpty=$i commit --allow-empty -m 'test'\n> +\t\"\n> +done\n> +\n> +for i in false 0\n> +do\n> +\ttest_expect_success \"commit.allowEmpty=$i and no --allow-empty\" \"\n> +\t\ttest_must_fail git -C commit.allowEmpty=$i commit -m 'test'\n> +\t\"\n> +\n> +\ttest_expect_success \"commit.allowEmpty=$i and --allow-empty\" \"\n> +\t\ttest_must_fail git -c commit.allowEmpty=$i commit --allow-empty -m 'test'\n> +\t\"\n> +done\n\nTesting both 1 and \"true\" can be dropped here. Things that use\ngit_config_bool() can just assume it works, we test it more exhaustively\nelsewhere.\n\nYour patch has whitespace errors. Try with \"git show --check\" or apply\nit with git-am, it also doesn't apply cleanly on the latest master.\n\nBut on this patch in general: I don't mind making this configurable, but\nneither your commit message nor these tests make it clear what the\nactual motivation is, which can be seen on the upstream GitHub bug\nreport.\n\nI.e. you seemingly have no interest in using \"git commit\" to produce\nempty commits, but are just trying to cherry-pick something and it's\nfailing because it (presumably, or am I missing something) cherry picks\nan existing commit content ends up not changing anything.\n\nI.e. you'd like to make the logic 37f7a85793 (\"Teach commit about\nCHERRY_PICK_HEAD\", 2011-02-19) added a message for the default.\n\nSo let's talk about that use case, and for those of us less familiar\nwith this explain why it is that this needs to still be optional at\nall. I.e. aren't we just exposing an implementation detail here where\ncherry-pick uses the commit machinery? Should we maybe just always pass\n--allow-empty on cherry-pick, if not why not?\n\nI can think of some reasons, but the above is a hint that both this\npatch + the current documentation which talks about \"foreign SCM\nscripts\" have drifted very far from what this is actually being used\nfor, so let's update that.\n"},{"id":"362432","messageId":"xmqq5zxcmkm8.fsf@gitster-ct.c.googlers.com","threadId":"49749","inReplyTo":"87d0rm7zeo.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3] commit: add a commit.allowEmpty config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-05T00:30:23Z","receivedAt":"2018-11-05T00:30:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I.e. you seemingly have no interest in using \"git commit\" to produce\n> empty commits, but are just trying to cherry-pick something and it's\n> failing because it (presumably, or am I missing something) cherry picks\n> an existing commit content ends up not changing anything.\n>\n> I.e. you'd like to make the logic 37f7a85793 (\"Teach commit about\n> CHERRY_PICK_HEAD\", 2011-02-19) added a message for the default.\n>\n> So let's talk about that use case, and for those of us less familiar\n> with this explain why it is that this needs to still be optional at\n> all. I.e. aren't we just exposing an implementation detail here where\n> cherry-pick uses the commit machinery? Should we maybe just always pass\n> --allow-empty on cherry-pick, if not why not?\n>\n> I can think of some reasons, but the above is a hint that both this\n> patch + the current documentation which talks about \"foreign SCM\n> scripts\" have drifted very far from what this is actually being used\n> for, so let's update that.\n\nThe command line \"--allowAnything\" in general is meant to be an\nescape hatch for unusual situations, and if a workflow requires\nconstant use of that escape hatch, there is something wrong either\nin the workflow or in the tool used in the workflow, and it is what\nwe should first see if we can fix, I would think, before making it\neasy to constantly use the escape hatch.\n\nI didn't look at the external reference you looked at but it sounds\nlike your review comment is taking the topic in the right direction.\n\nThanks for digging for the backstory.  \n"},{"id":"363173","messageId":"20181113155656.22975-1-tanushreetumane@gmail.com","threadId":"49749","inReplyTo":"87d0rm7zeo.fsf@evledraar.gmail.com","subject":"[PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Tanushree Tumane","fromEmail":"tanushreetumane@gmail.com","sentAt":"2018-11-13T15:56:56Z","receivedAt":"2018-11-13T15:57:13Z","isPatch":true,"sender":{"key":"tanushreetumane@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22275398?v=4"},"body":"From: tanushree27 <tanushreetumane@gmail.com>\n\nwhen we cherrypick an existing commit it doesn't change anything and\ntherefore it fails prompting to reset (skip commit) or commit using\n--allow-empty attribute and then continue.\n\nAdd commit.allowEmpty configuration variable as a convenience to skip\nthis process.\n\nAdd tests to check the behavior introduced by this commit.\n\nThis closes https://github.com/git-for-windows/git/issues/1854\n\nSigned-off-by: tanushree27 <tanushreetumane@gmail.com>\nSigned-off-by: Tanushree Tumane <tanushreetumane@gmail.com>\n---\n Documentation/config.txt     |  5 +++++\n Documentation/git-commit.txt |  3 ++-\n builtin/commit.c             |  8 ++++++++\n t/t3500-cherry.sh            | 10 ++++++++++\n 4 files changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c0727b7866..f3828518a5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1467,6 +1467,11 @@ commit.verbose::\n \tA boolean or int to specify the level of verbose with `git commit`.\n \tSee linkgit:git-commit[1].\n \n+commit.allowEmpty::\n+\tA boolean to specify whether empty commits are allowed with `git\n+\tcommit`. See linkgit:git-commit[1]. \n+\tDefaults 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 f970a43422..5d3bbf017a 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \tUsually recording a commit that has the exact same tree as its\n \tsole parent commit is a mistake, and the command prevents you\n \tfrom making such a commit.  This option bypasses the safety, and\n-\tis primarily for use by foreign SCM interface scripts.\n+\tis primarily for use by foreign SCM interface scripts. See\n+\t`commit.allowEmpty` in linkgit:git-config[1].\n \n --allow-empty-message::\n        Like --allow-empty this command is primarily for use by foreign\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 67fa949204..4516309ac2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n+static int config_commit_allow_empty = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit;\n@@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"commit.allowempty\")) {\n+\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n@@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (verbose == -1)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n+\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n+\t\tallow_empty = config_commit_allow_empty;\n+\t\n \tif (dry_run)\n \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\ndiff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\nindex f038f34b7c..11504e2d9f 100755\n--- a/t/t3500-cherry.sh\n+++ b/t/t3500-cherry.sh\n@@ -55,4 +55,14 @@ test_expect_success \\\n      expr \"$(echo $(git cherry master my-topic-branch) )\" : \"+ [^ ]* - .*\"\n '\n \n+\n+# Tests for commit.allowEmpty config\n+\n+test_expect_success 'cherry-pick existing commit with commit.allowEmpty' '\n+    test_tick &&\n+\ttest_commit \"first\" &&\n+\ttest_commit \"second\" &&\n+\tgit -c commit.allowEmpty=true cherry-pick HEAD~1\n+'\n+\n test_done\n-- \n2.19.1.windows.1.495.g7e9d1c442b.dirty\n\n"},{"id":"363202","messageId":"nycvar.QRO.7.76.6.1811132021390.39@tvgsbejvaqbjf.bet","threadId":"49749","inReplyTo":"20181113155656.22975-1-tanushreetumane@gmail.com","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-13T19:24:02Z","receivedAt":"2018-11-13T19:24:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Nov 2018, Tanushree Tumane wrote:\n\n> From: tanushree27 <tanushreetumane@gmail.com>\n> \n> when we cherrypick an existing commit it doesn't change anything and\n> therefore it fails prompting to reset (skip commit) or commit using\n> --allow-empty attribute and then continue.\n\nThis is a nice paragraph, but it might make sense to connect it to the\ncommit's oneline somehow. I, for one, was surprised to see the oneline\ntalk about `git commit` and the commit message about `git cherry-pick`.\n\nI could imagine that an introductory paragraph, talking about why one\nmight want to commit empty commits, might be the best lead into the\nsubject, and the paragraph about `cherry-pick` could follow (and be\nintroduced by saying something along the lines that this config setting\nhas more reach than just `git commit`; it also affects `git cherry-pick`)?\n\nCiao,\nJohannes\n\n> \n> Add commit.allowEmpty configuration variable as a convenience to skip\n> this process.\n> \n> Add tests to check the behavior introduced by this commit.\n> \n> This closes https://github.com/git-for-windows/git/issues/1854\n> \n> Signed-off-by: tanushree27 <tanushreetumane@gmail.com>\n> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>\n> ---\n>  Documentation/config.txt     |  5 +++++\n>  Documentation/git-commit.txt |  3 ++-\n>  builtin/commit.c             |  8 ++++++++\n>  t/t3500-cherry.sh            | 10 ++++++++++\n>  4 files changed, 25 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index c0727b7866..f3828518a5 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1467,6 +1467,11 @@ commit.verbose::\n>  \tA boolean or int to specify the level of verbose with `git commit`.\n>  \tSee linkgit:git-commit[1].\n>  \n> +commit.allowEmpty::\n> +\tA boolean to specify whether empty commits are allowed with `git\n> +\tcommit`. See linkgit:git-commit[1]. \n> +\tDefaults 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\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index f970a43422..5d3bbf017a 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n>  \tUsually recording a commit that has the exact same tree as its\n>  \tsole parent commit is a mistake, and the command prevents you\n>  \tfrom making such a commit.  This option bypasses the safety, and\n> -\tis primarily for use by foreign SCM interface scripts.\n> +\tis primarily for use by foreign SCM interface scripts. See\n> +\t`commit.allowEmpty` in linkgit:git-config[1].\n>  \n>  --allow-empty-message::\n>         Like --allow-empty this command is primarily for use by foreign\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 67fa949204..4516309ac2 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n>  static int edit_flag = -1; /* unspecified */\n>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n>  static int config_commit_verbose = -1; /* unspecified */\n> +static int config_commit_allow_empty = -1; /* unspecified */\n>  static int no_post_rewrite, allow_empty_message;\n>  static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n>  static char *sign_commit;\n> @@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n>  \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n>  \t\treturn 0;\n>  \t}\n> +\tif (!strcmp(k, \"commit.allowempty\")) {\n> +\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n> +\t\treturn 0;\n> +\t}\n>  \n>  \tstatus = git_gpg_config(k, v, NULL);\n>  \tif (status)\n> @@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \tif (verbose == -1)\n>  \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n>  \n> +\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n> +\t\tallow_empty = config_commit_allow_empty;\n> +\t\n>  \tif (dry_run)\n>  \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n>  \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\n> diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\n> index f038f34b7c..11504e2d9f 100755\n> --- a/t/t3500-cherry.sh\n> +++ b/t/t3500-cherry.sh\n> @@ -55,4 +55,14 @@ test_expect_success \\\n>       expr \"$(echo $(git cherry master my-topic-branch) )\" : \"+ [^ ]* - .*\"\n>  '\n>  \n> +\n> +# Tests for commit.allowEmpty config\n> +\n> +test_expect_success 'cherry-pick existing commit with commit.allowEmpty' '\n> +    test_tick &&\n> +\ttest_commit \"first\" &&\n> +\ttest_commit \"second\" &&\n> +\tgit -c commit.allowEmpty=true cherry-pick HEAD~1\n> +'\n> +\n>  test_done\n> -- \n> 2.19.1.windows.1.495.g7e9d1c442b.dirty\n> \n> \n"},{"id":"363219","messageId":"87zhuc1xcx.fsf@evledraar.gmail.com","threadId":"49749","inReplyTo":"nycvar.QRO.7.76.6.1811132021390.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T21:27:58Z","receivedAt":"2018-11-13T21:28:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 13 2018, Johannes Schindelin wrote:\n\n[Comments on the v4 patch also inline, found it easier to reply just to\nthis one]\n\n> On Tue, 13 Nov 2018, Tanushree Tumane wrote:\n>\n>> From: tanushree27 <tanushreetumane@gmail.com>\n>>\n>> when we cherrypick an existing commit it doesn't change anything and\n>> therefore it fails prompting to reset (skip commit) or commit using\n>> --allow-empty attribute and then continue.\n>\n> This is a nice paragraph, but it might make sense to connect it to the\n> commit's oneline somehow. I, for one, was surprised to see the oneline\n> talk about `git commit` and the commit message about `git cherry-pick`.\n>\n> I could imagine that an introductory paragraph, talking about why one\n> might want to commit empty commits, might be the best lead into the\n> subject, and the paragraph about `cherry-pick` could follow (and be\n> introduced by saying something along the lines that this config setting\n> has more reach than just `git commit`; it also affects `git cherry-pick`)?\n\nAgreed. I'm happy to see the test for-loop gone as I noted in\nhttps://public-inbox.org/git/87d0rm7zeo.fsf@evledraar.gmail.com/ but as\nnoted in that v3 feedback the whole \"why would anyone want this?\"\nexplanation is still missing, and this still smells like a workaround\nfor a bug we should be fixing elsewhere in the sequencing code.\n\n[The rest of this for Tanushree]\n\n>>\n>> Add commit.allowEmpty configuration variable as a convenience to skip\n>> this process.\n>>\n>> Add tests to check the behavior introduced by this commit.\n>>\n>> This closes https://github.com/git-for-windows/git/issues/1854\n>>\n>> Signed-off-by: tanushree27 <tanushreetumane@gmail.com>\n>> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>\n>> ---\n>>  Documentation/config.txt     |  5 +++++\n>>  Documentation/git-commit.txt |  3 ++-\n>>  builtin/commit.c             |  8 ++++++++\n>>  t/t3500-cherry.sh            | 10 ++++++++++\n>>  4 files changed, 25 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index c0727b7866..f3828518a5 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1467,6 +1467,11 @@ commit.verbose::\n>>  \tA boolean or int to specify the level of verbose with `git commit`.\n>>  \tSee linkgit:git-commit[1].\n>>\n>> +commit.allowEmpty::\n>> +\tA boolean to specify whether empty commits are allowed with `git\n>> +\tcommit`. See linkgit:git-commit[1].\n>> +\tDefaults 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\n>> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n>> index f970a43422..5d3bbf017a 100644\n>> --- a/Documentation/git-commit.txt\n>> +++ b/Documentation/git-commit.txt\n>> @@ -176,7 +176,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n>>  \tUsually recording a commit that has the exact same tree as its\n>>  \tsole parent commit is a mistake, and the command prevents you\n>>  \tfrom making such a commit.  This option bypasses the safety, and\n>> -\tis primarily for use by foreign SCM interface scripts.\n>> +\tis primarily for use by foreign SCM interface scripts. See\n>> +\t`commit.allowEmpty` in linkgit:git-config[1].\n>>\n>>  --allow-empty-message::\n>>         Like --allow-empty this command is primarily for use by foreign\n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> index 67fa949204..4516309ac2 100644\n>> --- a/builtin/commit.c\n>> +++ b/builtin/commit.c\n>> @@ -101,6 +101,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n>>  static int edit_flag = -1; /* unspecified */\n>>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n>>  static int config_commit_verbose = -1; /* unspecified */\n>> +static int config_commit_allow_empty = -1; /* unspecified */\n>>  static int no_post_rewrite, allow_empty_message;\n>>  static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n>>  static char *sign_commit;\n>> @@ -1435,6 +1436,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n>>  \t\tconfig_commit_verbose = git_config_bool_or_int(k, v, &is_bool);\n>>  \t\treturn 0;\n>>  \t}\n>> +\tif (!strcmp(k, \"commit.allowempty\")) {\n>> +\t\tconfig_commit_allow_empty = git_config_bool(k, v);\n>> +\t\treturn 0;\n>> +\t}\n>>\n>>  \tstatus = git_gpg_config(k, v, NULL);\n>>  \tif (status)\n>> @@ -1556,6 +1561,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>>  \tif (verbose == -1)\n>>  \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n>>\n>> +\tif (config_commit_allow_empty >= 0)  /* if allowEmpty is allowed in config*/\n>> +\t\tallow_empty = config_commit_allow_empty;\n>> +\n\nI had two comments on this hunk in my v3 feedback. It's fine to go for\nsomething different (although I still think you should change it), but\nfor others following along you should at least say \"I had such-and-such\nfeedback suggesting X, but decided to go for Y anyway because...\".\n\n>>  \tif (dry_run)\n>>  \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n>>  \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\n>> diff --git a/t/t3500-cherry.sh b/t/t3500-cherry.sh\n>> index f038f34b7c..11504e2d9f 100755\n>> --- a/t/t3500-cherry.sh\n>> +++ b/t/t3500-cherry.sh\n>> @@ -55,4 +55,14 @@ test_expect_success \\\n>>       expr \"$(echo $(git cherry master my-topic-branch) )\" : \"+ [^ ]* - .*\"\n>>  '\n>>\n>> +\n>> +# Tests for commit.allowEmpty config\n>> +\n\nLet's drop this comment. It's redundant to the test description.\n\n>> +test_expect_success 'cherry-pick existing commit with commit.allowEmpty' '\n>> +    test_tick &&\n>> +\ttest_commit \"first\" &&\n>> +\ttest_commit \"second\" &&\n>> +\tgit -c commit.allowEmpty=true cherry-pick HEAD~1\n>> +'\n\nSo now you've dropped any tests of \"git commit\" (even though you're\nchanging commit.c, and just testing revert.c. So again, if that's all we\nwant isn't this whole thing just a simple bugfix of:\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 96d336ec3d..1a12cc559e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1546,6 +1546,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n        if (verbose == -1)\n                verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n\n+       if (whence == FROM_CHERRY_PICK)\n+               allow_empty = allow_empty_message = 1;\n+\n        if (dry_run)\n                return dry_run_commit(argc, argv, prefix, current_head, &s);\n        index_file = prepare_index(argc, argv, prefix, current_head, 0);\n\nPossibly dropping the allow_empty_message part, but it seems reasonable\nthat whether you're re-picking an empty commit or one with an empty\nmessage cherry-pick should always work.\n\nI see that fails various existing tests, and I'm going to stop digging\nnow, but that brings me back to the \"let's explain this better\" part of\nthe feedback. I.e. if we can't just fix it let's explain why it can't be\nmade a default because it breaks such-and-such, so we need the config\noption.\n"},{"id":"363307","messageId":"xmqqzhucpa37.fsf@gitster-ct.c.googlers.com","threadId":"49749","inReplyTo":"87zhuc1xcx.fsf@evledraar.gmail.com","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:16:44Z","receivedAt":"2018-11-14T04:16:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Agreed. I'm happy to see the test for-loop gone as I noted in\n> https://public-inbox.org/git/87d0rm7zeo.fsf@evledraar.gmail.com/ but as\n> noted in that v3 feedback the whole \"why would anyone want this?\"\n> explanation is still missing, and this still smells like a workaround\n> for a bug we should be fixing elsewhere in the sequencing code.\n\nThanks.  I share the same impression that this is sweeping a bug\nunder a wrong rug.\n"},{"id":"363364","messageId":"nycvar.QRO.7.76.6.1811141456590.39@tvgsbejvaqbjf.bet","threadId":"49749","inReplyTo":"xmqqzhucpa37.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-14T14:04:03Z","receivedAt":"2018-11-14T14:04:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Nov 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> \n> > Agreed. I'm happy to see the test for-loop gone as I noted in\n> > https://public-inbox.org/git/87d0rm7zeo.fsf@evledraar.gmail.com/ but as\n> > noted in that v3 feedback the whole \"why would anyone want this?\"\n> > explanation is still missing, and this still smells like a workaround\n> > for a bug we should be fixing elsewhere in the sequencing code.\n> \n> Thanks.  I share the same impression that this is sweeping a bug\n> under a wrong rug.\n\nI agree that the scenario is under-explained. Of course, I have to say\nthat this is not Tanushree's problem; They only copied what is in\nhttps://github.com/git-for-windows/git/issues/1854 and @chucklu did not\ngrace us with an explanation, either.\n\nBased on historical context, I would wager a bet that the scenario is\nthat some commits that may or may not have been in a different SCM\noriginally and that may or may not have been empty and/or squashed in\n`master` need to be cherry-picked.\n\nBut I agree that this should be clarified. I prodded the original\nwish-haver.\n\nCiao,\nDscho"},{"id":"363415","messageId":"nycvar.QRO.7.76.6.1811150938070.41@tvgsbejvaqbjf.bet","threadId":"49749","inReplyTo":"xmqqzhucpa37.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-15T08:40:38Z","receivedAt":"2018-11-15T08:40:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Nov 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> \n> > Agreed. I'm happy to see the test for-loop gone as I noted in\n> > https://public-inbox.org/git/87d0rm7zeo.fsf@evledraar.gmail.com/ but as\n> > noted in that v3 feedback the whole \"why would anyone want this?\"\n> > explanation is still missing, and this still smells like a workaround\n> > for a bug we should be fixing elsewhere in the sequencing code.\n> \n> Thanks.  I share the same impression that this is sweeping a bug\n> under a wrong rug.\n\nI asked for clarification at\nhttps://github.com/git-for-windows/git/issues/1854 and in my best\nimitation of Lt Tawney Madison, I report back:\n\nFrom @chucklu:\n\n> my user case is like this :\n>\n> When I want to cherr-pick commits from A to G (ABCDEFG), image C and E\n> are merge commits.  Then I will get lots of popup like:\n>\n>    The previous cherry-pick is now empty, possibly due to conflict\n>    resolution.\n>    If you wish to commit it anyway, use:\n>\n>        git commit --allow-empty\n>\n>    If you wish to skip this commit, use:\n>\n>        git reset\n>\n>    Then \"git cherry-pick --continue\" will resume cherry-picking\n>    the remaining commits.\n\nMy quick interpretation of this is that the user actually needs a way to\nskip silently commits which are now empty.\n\nCiao,\nDscho"},{"id":"363417","messageId":"20181115094101.GA15279@sigill.intra.peff.net","threadId":"49749","inReplyTo":"nycvar.QRO.7.76.6.1811150938070.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T09:41:01Z","receivedAt":"2018-11-15T09:41:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 15, 2018 at 09:40:38AM +0100, Johannes Schindelin wrote:\n\n> From @chucklu:\n> \n> > my user case is like this :\n> >\n> > When I want to cherr-pick commits from A to G (ABCDEFG), image C and E\n> > are merge commits.  Then I will get lots of popup like:\n> >\n> >    The previous cherry-pick is now empty, possibly due to conflict\n> >    resolution.\n> >    If you wish to commit it anyway, use:\n> >\n> >        git commit --allow-empty\n> >\n> >    If you wish to skip this commit, use:\n> >\n> >        git reset\n> >\n> >    Then \"git cherry-pick --continue\" will resume cherry-picking\n> >    the remaining commits.\n> \n> My quick interpretation of this is that the user actually needs a way to\n> skip silently commits which are now empty.\n\nIf it's always intended to be used with cherry-pick, shouldn't\ncherry-pick learn a --keep-empty (like rebase has)? That would avoid\neven stopping for this case in the first place.\n\n-Peff\n"},{"id":"363439","messageId":"nycvar.QRO.7.76.6.1811151713530.41@tvgsbejvaqbjf.bet","threadId":"49749","inReplyTo":"20181115094101.GA15279@sigill.intra.peff.net","subject":"Re: [PATCH v4] commit: add a commit.allowEmpty config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-15T16:16:13Z","receivedAt":"2018-11-15T16:16:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 15 Nov 2018, Jeff King wrote:\n\n> On Thu, Nov 15, 2018 at 09:40:38AM +0100, Johannes Schindelin wrote:\n> \n> > From @chucklu:\n> > \n> > > my user case is like this :\n> > >\n> > > When I want to cherr-pick commits from A to G (ABCDEFG), image C and E\n> > > are merge commits.  Then I will get lots of popup like:\n> > >\n> > >    The previous cherry-pick is now empty, possibly due to conflict\n> > >    resolution.\n> > >    If you wish to commit it anyway, use:\n> > >\n> > >        git commit --allow-empty\n> > >\n> > >    If you wish to skip this commit, use:\n> > >\n> > >        git reset\n> > >\n> > >    Then \"git cherry-pick --continue\" will resume cherry-picking\n> > >    the remaining commits.\n> > \n> > My quick interpretation of this is that the user actually needs a way to\n> > skip silently commits which are now empty.\n> \n> If it's always intended to be used with cherry-pick, shouldn't\n> cherry-pick learn a --keep-empty (like rebase has)? That would avoid\n> even stopping for this case in the first place.\n\nI'd go for the other way round: --skip-empty.\n\nHowever, given the very unhappy turn in that Git for Windows ticket\n(somebody asks for a feature, then just sits back, and does not even\nconfirm that the analysis covers their use case, let alone participates in\nthis discussion), I am personally not really interested in driving this\none any further.\n\nTanushree proved that they know how to contribute to the Git mailing list,\nas a pre-requisite for the Outreachy project, and that is the positive\noutcome of this thread as far as I am concerned. I am pretty happy about\nthat, too.\n\nCiao,\nDscho\n"}]}