{"thread":{"id":"57595","subject":"[PATCH 0/4] reset: make --no-refresh the only way to skip index refresh","startedAt":"2022-03-21T20:35:01Z","lastAt":"2022-03-25T16:35:23Z","messageCount":25,"participants":["Victoria Dye via GitGitGadget","Phillip Wood","Victoria Dye","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"451761","messageId":"pull.1184.git.1647894889.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":null,"subject":"[PATCH 0/4] reset: make --no-refresh the only way to skip index refresh","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-21T20:34:45Z","receivedAt":"2022-03-21T20:35:01Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Maintainer's note: this is based on vd/stash-silence-reset\n\n----------------------------------------------------------------------------\n\nThis is a follow-up to the changes in vd/stash-silence-reset [1], in which\nindex refreshing behavior was decoupled from log silencing in the '--quiet'\noption to 'git reset --mixed' by introducing a '--[no-]refresh' option and\n'reset.refresh' config setting.\n\nAfter some discussion [2] on the mailing list, both the\nbackward-compatibility and use of global options in that series came into\nquestion:\n\n * '--quiet' still skipped refresh if neither '--[no-]refresh' nor\n 'reset.refresh' were specified, meaning that users could still be left\n with an incorrect index state after reset.\n * Having 'reset.quiet' and/or 'reset.refresh' potentially disable index\n refresh by default meant that developers would need to defensively add\n '--refresh' to all internal uses of 'git reset --mixed'. Without that\n option, different config setups could cause variability in index\n correctness from user to user.\n\nIn response, this series deprecates all instances of skipping index refresh\nin 'git reset --mixed' except for '--no-refresh' itself:\n\n * Patch [1/4] removes the \"fallback\" behavior of 'reset.quiet' and\n '--quiet' implying '--no-refresh' if neither '--[no-]refresh' nor\n 'config.refresh' were specified. In other words, '--quiet' no longer does\n anything other than log silencing.\n * Patch [2/4] deprecates 'reset.quiet', since its main use was to disable\n index refresh until it was deprecated in [1/4].\n * Patch [3/4] deprecates 'reset.refresh' to avoid users accidentally ending\n up with an incorrect index state after all resets as a result of a global\n setting's passive effects.\n * Patch [4/4] removes the '--refresh' option, leaving only '--no-refresh'.\n Because nothing but '--no-refresh' can skip the 'reset' index refresh\n anymore, '--refresh' would never be needed.\n\n[1]\nhttps://lore.kernel.org/git/pull.1170.v3.git.1647308982.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/80a2a5a2-256f-6c3b-2430-10bef99ce1e9@github.com/\n\nThanks! -Victoria\n\nVictoria Dye (4):\n reset: do not make '--quiet' disable index refresh\n reset: deprecate 'reset.quiet' config option\n reset: deprecate 'reset.refresh' config option\n reset: deprecate '--refresh', leaving only '--no-refresh'\n\n Documentation/config.txt | 2 --\n Documentation/config/reset.txt | 2 --\n Documentation/git-reset.txt | 13 ++-------\n builtin/reset.c | 18 +++----------\n builtin/stash.c | 4 +--\n contrib/scalar/scalar.c | 1 -\n t/t7102-reset.sh | 48 +++++-----------------------------\n 7 files changed, 15 insertions(+), 73 deletions(-)\n delete mode 100644 Documentation/config/reset.txt\n\n\nbase-commit: 4b8b0f6fa2778c1f9c373620e3f07787543914c6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1184%2Fvdye%2Freset%2Fclean-up-refresh-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1184/vdye/reset/clean-up-refresh-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1184\n-- \ngitgitgadget\n"},{"id":"451762","messageId":"d9bebd4b4e0000850e076ee642b20b09f71db700.1647894889.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.git.1647894889.gitgitgadget@gmail.com","subject":"[PATCH 2/4] reset: deprecate 'reset.quiet' config option","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-21T20:34:47Z","receivedAt":"2022-03-21T20:35:03Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nRemove the 'reset.quiet' config option, remove '--no-quiet' documentation in\n'Documentation/git-reset.txt'. In 4c3abd0551 (reset: add new reset.quiet\nconfig setting, 2018-10-23), 'reset.quiet' was introduced as a way to\nglobally change the default behavior of 'git reset --mixed' to skip index\nrefresh.\n\nHowever, now that '--quiet' does not affect index refresh, 'reset.quiet'\nwould only serve to globally silence logging. This was not the original\nintention of the config setting, and there's no precedent for such a setting\nin other commands with a '--quiet' option, so it appears to be obsolete.\n\nIn addition to the options & its documentation, remove 'reset.quiet' from\nthe recommended config for 'scalar'.\n\nSigned-off-by: Victoria Dye \n---\n Documentation/config.txt | 2 --\n Documentation/config/reset.txt | 2 --\n Documentation/git-reset.txt | 5 +----\n builtin/reset.c | 1 -\n contrib/scalar/scalar.c | 1 -\n t/t7102-reset.sh | 3 +--\n 6 files changed, 2 insertions(+), 12 deletions(-)\n delete mode 100644 Documentation/config/reset.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f0fb25a371c..43f5e6fd6d3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -495,8 +495,6 @@ include::config/repack.txt[]\n \n include::config/rerere.txt[]\n \n-include::config/reset.txt[]\n-\n include::config/sendemail.txt[]\n \n include::config/sequencer.txt[]\ndiff --git a/Documentation/config/reset.txt b/Documentation/config/reset.txt\ndeleted file mode 100644\nindex 63b7c45aac2..00000000000\n--- a/Documentation/config/reset.txt\n+++ /dev/null\n@@ -1,2 +0,0 @@\n-reset.quiet::\n-\tWhen set to true, 'git reset' will default to the '--quiet' option.\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex bc1646c3016..f4aca9dd35c 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -105,10 +105,7 @@ OPTIONS\n \n -q::\n --quiet::\n---no-quiet::\n-\tBe quiet, only report errors. The default behavior is set by the\n-\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n-\toverride the default behavior.\n+\tBe quiet, only report errors.\n \n --refresh::\n --no-refresh::\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 7c3828f6fc5..e824aad3604 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n-\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \tgit_config_get_bool(\"reset.refresh\", &refresh);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 7db2a97416e..58ca0e56f14 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -152,7 +152,6 @@ static int set_recommended_config(int reconfigure)\n \t\t{ \"pack.useBitmaps\", \"false\", 1 },\n \t\t{ \"pack.useSparse\", \"true\", 1 },\n \t\t{ \"receive.autoGC\", \"false\", 1 },\n-\t\t{ \"reset.quiet\", \"true\", 1 },\n \t\t{ \"feature.manyFiles\", \"false\", 1 },\n \t\t{ \"feature.experimental\", \"false\", 1 },\n \t\t{ \"fetch.unpackLimit\", \"1\", 1 },\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 8b62bb39b3d..9e4c4deee35 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -488,8 +488,7 @@ test_expect_success '--mixed refreshes the index' '\n \t# Verify default behavior (without --[no-]refresh or reset.refresh)\n \ttest_reset_refreshes_index &&\n \n-\t# With --quiet & reset.quiet\n-\ttest_reset_refreshes_index \"-c reset.quiet=true\" &&\n+\t# With --quiet\n \ttest_reset_refreshes_index \"\" --quiet\n '\n \n-- \ngitgitgadget\n\n"},{"id":"451763","messageId":"f89e9b4ae24718116d0275333f4ece1d4024ab6b.1647894889.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.git.1647894889.gitgitgadget@gmail.com","subject":"[PATCH 1/4] reset: do not make '--quiet' disable index refresh","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-21T20:34:46Z","receivedAt":"2022-03-21T20:35:04Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nUpdate '--quiet' to no longer implicitly skip refreshing the index in a\nmixed reset. Users now have the ability to explicitly disable refreshing the\nindex with the '--no-refresh' option, so they no longer need to use\n'--quiet' to do so. Moreover, we explicitly remove the refresh-skipping\nbehavior from '--quiet' because it is completely unrelated to the stated\npurpose of the option: \"Be quiet, only report errors.\"\n\nSigned-off-by: Victoria Dye \n---\n Documentation/git-reset.txt | 5 +----\n builtin/reset.c | 7 -------\n t/t7102-reset.sh | 32 +++++---------------------------\n 3 files changed, 6 insertions(+), 38 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 89ddc85c2e4..bc1646c3016 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -114,10 +114,7 @@ OPTIONS\n --no-refresh::\n \tProactively refresh the index after a mixed reset. If unspecified, the\n \tbehavior falls back on the `reset.refresh` config option. If neither\n-\t`--[no-]refresh` nor `reset.refresh` are set, the default behavior is\n-\tdecided by the `--[no-]quiet` option and/or `reset.quiet` config.\n-\tIf `--quiet` is specified or `reset.quiet` is set with no command-line\n-\t\"quiet\" setting, refresh is disabled. Otherwise, refresh is enabled.\n+\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c8a356ec5b0..7c3828f6fc5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -430,13 +430,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n-\t/*\n-\t * If refresh is completely unspecified (either by config or by command\n-\t * line option), decide based on 'quiet'.\n-\t */\n-\tif (refresh < 0)\n-\t\trefresh = !quiet;\n-\n \tif (pathspec_from_file) {\n \t\tif (patch_mode)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 1dc3911a060..8b62bb39b3d 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -485,25 +485,12 @@ test_reset_refreshes_index () {\n }\n \n test_expect_success '--mixed refreshes the index' '\n-\t# Verify default behavior (with no config settings or command line\n-\t# options)\n-\ttest_reset_refreshes_index\n-'\n-test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '\n-\t# Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)\n-\t# determine refresh behavior\n-\n-\t# Config setting\n-\t! test_reset_refreshes_index \"-c reset.quiet=true\" &&\n-\ttest_reset_refreshes_index \"-c reset.quiet=false\" &&\n-\n-\t# Command line option\n-\t! test_reset_refreshes_index \"\" --quiet &&\n-\ttest_reset_refreshes_index \"\" --no-quiet &&\n+\t# Verify default behavior (without --[no-]refresh or reset.refresh)\n+\ttest_reset_refreshes_index &&\n \n-\t# Command line option overrides config setting\n-\t! test_reset_refreshes_index \"-c reset.quiet=false\" --quiet &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" --no-quiet\n+\t# With --quiet & reset.quiet\n+\ttest_reset_refreshes_index \"-c reset.quiet=true\" &&\n+\ttest_reset_refreshes_index \"\" --quiet\n '\n \n test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n@@ -522,15 +509,6 @@ test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n \t! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n '\n \n-test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '\n-\t# Verify that *both* --refresh and `reset.refresh` override the\n-\t# default non-refresh behavior of --quiet\n-\ttest_reset_refreshes_index \"\" \"--quiet --refresh\" &&\n-\ttest_reset_refreshes_index \"-c reset.quiet=true\" --refresh &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" --quiet &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true -c reset.quiet=true\"\n-'\n-\n test_expect_success '--mixed preserves skip-worktree' '\n \techo 123 >>file2 &&\n \tgit add file2 &&\n-- \ngitgitgadget\n\n"},{"id":"451764","messageId":"ecd3296fd25cc936aeb2f8ae160352a2326441c5.1647894889.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.git.1647894889.gitgitgadget@gmail.com","subject":"[PATCH 3/4] reset: deprecate 'reset.refresh' config option","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-21T20:34:48Z","receivedAt":"2022-03-21T20:35:05Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nRemove the 'reset.refresh' option, requiring that users explicitly specify\n'--no-refresh' if they want to skip refreshing the index.\n\nThe 'reset.refresh' option was introduced in 101cee42dd (reset: introduce\n--[no-]refresh option to --mixed, 2022-03-11) as a replacement for the\nrefresh-skipping behavior originally controlled by 'reset.quiet'.\n\nAlthough 'reset.refresh=false' functionally served the same purpose as\n'reset.quiet=true', it exposed [1] the fact that the existence of a global\n\"skip refresh\" option could potentially cause problems for users. Allowing a\nglobal config option to avoid refreshing the index forces scripts using 'git\nreset --mixed' to defensively use '--refresh' if index refresh is expected;\nif that option is missing, behavior of a script could vary from user-to-user\nwithout explanation.\n\nFurthermore, globally disabling index refresh in 'reset --mixed' was\ninitially devised as a passive performance improvement; since the\nintroduction of the option, other changes have been made to Git (e.g., the\nsparse index) with a greater potential performance impact without\nsacrificing index correctness. Therefore, we can more aggressively err on\nthe side of correctness and limit the cases of skipping index refresh to\nonly when a user specifies the '--no-refresh' option.\n\n[1] https://lore.kernel.org/git/xmqqy2179o3c.fsf@gitster.g/\n\nSigned-off-by: Victoria Dye \n---\n Documentation/git-reset.txt | 4 +---\n builtin/reset.c | 4 +---\n t/t7102-reset.sh | 14 ++------------\n 3 files changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex f4aca9dd35c..df167eaa766 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -109,9 +109,7 @@ OPTIONS\n \n --refresh::\n --no-refresh::\n-\tProactively refresh the index after a mixed reset. If unspecified, the\n-\tbehavior falls back on the `reset.refresh` config option. If neither\n-\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n+\tProactively refresh the index after a mixed reset. Enabled by default.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex e824aad3604..54324217f93 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n-\tgit_config_get_bool(\"reset.refresh\", &refresh);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n@@ -529,8 +528,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n \t\t\t\tif (!quiet && advice_enabled(ADVICE_RESET_NO_REFRESH_WARNING) && t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n \t\t\t\t\tadvise(_(\"It took %.2f seconds to refresh the index after reset. You can use\\n\"\n-\t\t\t\t\t\t \"'--no-refresh' to avoid this. Set the config setting reset.refresh to false\\n\"\n-\t\t\t\t\t\t \"to make this the default.\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t\t\t \"'--no-refresh' to avoid this.\"), t_delta_in_ms / 1000.0);\n \t\t\t\t}\n \t\t\t}\n \t\t} else {\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 9e4c4deee35..22477f3a312 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -493,19 +493,9 @@ test_expect_success '--mixed refreshes the index' '\n '\n \n test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n-\t# Verify that --[no-]refresh and `reset.refresh` control index refresh\n-\n-\t# Config setting\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" &&\n-\t! test_reset_refreshes_index \"-c reset.refresh=false\" &&\n-\n-\t# Command line option\n+\t# Verify that --[no-]refresh controls index refresh\n \ttest_reset_refreshes_index \"\" --refresh &&\n-\t! test_reset_refreshes_index \"\" --no-refresh &&\n-\n-\t# Command line option overrides config setting\n-\ttest_reset_refreshes_index \"-c reset.refresh=false\" --refresh &&\n-\t! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n+\t! test_reset_refreshes_index \"\" --no-refresh\n '\n \n test_expect_success '--mixed preserves skip-worktree' '\n-- \ngitgitgadget\n\n"},{"id":"451765","messageId":"dbb63c4caa83cc764535a739d20736b706ee44a5.1647894889.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.git.1647894889.gitgitgadget@gmail.com","subject":"[PATCH 4/4] reset: deprecate '--refresh', leaving only '--no-refresh'","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-21T20:34:49Z","receivedAt":"2022-03-21T20:35:09Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nThe explicit '--refresh' option was needed in the past when '--quiet',\n'reset.quiet', and/or 'reset.refresh' disabled the index refresh in 'reset\n--mixed'. Those options have since either been deprecated or made to always\nrefresh the index by default, leaving only '--[no-]refresh' determining\nwhether the index is refreshed or not.\n\nBecause there is nothing other than '--no-refresh' to disable index refresh,\nwe do not need a '--refresh' option to counteract some other refresh\ndisabling.\n\nTo ensure users don't use what is effectively a no-op, remove '--refresh'\nfrom the set of 'reset' options, as well as its usage in 'git stash'.\n\nSigned-off-by: Victoria Dye \n---\n Documentation/git-reset.txt | 3 +--\n builtin/reset.c | 6 +++---\n builtin/stash.c | 4 ++--\n t/t7102-reset.sh | 5 ++---\n 4 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex df167eaa766..ba8dece0c03 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -107,9 +107,8 @@ OPTIONS\n --quiet::\n \tBe quiet, only report errors.\n \n---refresh::\n --no-refresh::\n-\tProactively refresh the index after a mixed reset. Enabled by default.\n+\tDisable refreshing the index after a mixed reset.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 54324217f93..d9427abc483 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -392,7 +392,7 @@ static int git_reset_config(const char *var, const char *value, void *cb)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint refresh = -1;\n+\tint refresh = 1;\n \tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n \tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n@@ -400,8 +400,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tint intent_to_add = 0;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n-\t\tOPT_BOOL(0, \"refresh\", &refresh,\n-\t\t\t\tN_(\"skip refreshing the index after reset\")),\n+\t\tOPT_SET_INT(0, \"no-refresh\", &refresh,\n+\t\t\t\tN_(\"skip refreshing the index after reset\"), 0),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n \t\t\t\t\t\tN_(\"reset HEAD and index\"), MIXED),\n \t\tOPT_SET_INT(0, \"soft\", &reset_type, N_(\"reset only HEAD\"), SOFT),\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 91407d9bbe0..73f2ba88823 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -310,7 +310,7 @@ static int reset_head(void)\n \t * API for resetting.\n \t */\n \tcp.git_cmd = 1;\n-\tstrvec_pushl(&cp.args, \"reset\", \"--quiet\", \"--refresh\", NULL);\n+\tstrvec_pushl(&cp.args, \"reset\", \"--quiet\", NULL);\n \n \treturn run_command(&cp);\n }\n@@ -1633,7 +1633,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \t\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n \n \t\t\tcp.git_cmd = 1;\n-\t\t\tstrvec_pushl(&cp.args, \"reset\", \"-q\", \"--refresh\", \"--\",\n+\t\t\tstrvec_pushl(&cp.args, \"reset\", \"-q\", \"--\",\n \t\t\t\t NULL);\n \t\t\tadd_pathspecs(&cp.args, ps);\n \t\t\tif (run_command(&cp)) {\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 22477f3a312..7a9b845df8c 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -492,9 +492,8 @@ test_expect_success '--mixed refreshes the index' '\n \ttest_reset_refreshes_index \"\" --quiet\n '\n \n-test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n-\t# Verify that --[no-]refresh controls index refresh\n-\ttest_reset_refreshes_index \"\" --refresh &&\n+test_expect_success '--mixed --no-refresh sets refresh behavior' '\n+\t# Verify that --no-refresh controls index refresh\n \t! test_reset_refreshes_index \"\" --no-refresh\n '\n \n-- \ngitgitgadget\n"},{"id":"451997","messageId":"355f36d5-7153-42a4-6db8-b9ba5fcac422@gmail.com","threadId":"57595","inReplyTo":"f89e9b4ae24718116d0275333f4ece1d4024ab6b.1647894889.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/4] reset: do not make '--quiet' disable index refresh","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-23T15:59:56Z","receivedAt":"2022-03-23T16:00:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Victoria\n\nOn 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n> From: Victoria Dye \n> \n> Update '--quiet' to no longer implicitly skip refreshing the index in a\n> mixed reset. Users now have the ability to explicitly disable refreshing the\n> index with the '--no-refresh' option, so they no longer need to use\n> '--quiet' to do so. Moreover, we explicitly remove the refresh-skipping\n> behavior from '--quiet' because it is completely unrelated to the stated\n> purpose of the option: \"Be quiet, only report errors.\"\n> \n> Signed-off-by: Victoria Dye \n> ---\n> Documentation/git-reset.txt | 5 +----\n> builtin/reset.c | 7 -------\n> t/t7102-reset.sh | 32 +++++---------------------------\n> 3 files changed, 6 insertions(+), 38 deletions(-)\n> \n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index 89ddc85c2e4..bc1646c3016 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -114,10 +114,7 @@ OPTIONS\n> --no-refresh::\n> \tProactively refresh the index after a mixed reset. If unspecified, the\n> \tbehavior falls back on the `reset.refresh` config option. If neither\n> -\t`--[no-]refresh` nor `reset.refresh` are set, the default behavior is\n> -\tdecided by the `--[no-]quiet` option and/or `reset.quiet` config.\n> -\tIf `--quiet` is specified or `reset.quiet` is set with no command-line\n> -\t\"quiet\" setting, refresh is disabled. Otherwise, refresh is enabled.\n> +\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n> \n> --pathspec-from-file=::\n> \tPathspec is passed in `` instead of commandline args. If\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index c8a356ec5b0..7c3828f6fc5 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -430,13 +430,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n> \n> -\t/*\n> -\t * If refresh is completely unspecified (either by config or by command\n> -\t * line option), decide based on 'quiet'.\n> -\t */\n> -\tif (refresh < 0)\n> -\t\trefresh = !quiet;\n\nDoes this mean we can stop initializing refresh to -1?\n\nBest Wishes\n\nPhillip\n"},{"id":"451998","messageId":"9462c3f0-db92-e1de-cef5-df811ddf9eb2@gmail.com","threadId":"57595","inReplyTo":"d9bebd4b4e0000850e076ee642b20b09f71db700.1647894889.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/4] reset: deprecate 'reset.quiet' config option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-23T16:00:46Z","receivedAt":"2022-03-23T16:00:52Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Victoria\n\nOn 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n> From: Victoria Dye \n\nSmall nit-pick about the title, I think \"deprecate\" is a bit misleading \nas we're actually removing the config option.\n\n> Remove the 'reset.quiet' config option, remove '--no-quiet' documentation in\n> 'Documentation/git-reset.txt'. In 4c3abd0551 (reset: add new reset.quiet\n> config setting, 2018-10-23), 'reset.quiet' was introduced as a way to\n> globally change the default behavior of 'git reset --mixed' to skip index\n> refresh.\n> \n> However, now that '--quiet' does not affect index refresh, 'reset.quiet'\n> would only serve to globally silence logging. This was not the original\n> intention of the config setting, and there's no precedent for such a setting\n> in other commands with a '--quiet' option, so it appears to be obsolete.\n> \n> In addition to the options & its documentation, remove 'reset.quiet' from\n> the recommended config for 'scalar'.\n\nNice attention to detail\n\n\nThe code changes look good\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Victoria Dye \n> ---\n> Documentation/config.txt | 2 --\n> Documentation/config/reset.txt | 2 --\n> Documentation/git-reset.txt | 5 +----\n> builtin/reset.c | 1 -\n> contrib/scalar/scalar.c | 1 -\n> t/t7102-reset.sh | 3 +--\n> 6 files changed, 2 insertions(+), 12 deletions(-)\n> delete mode 100644 Documentation/config/reset.txt\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index f0fb25a371c..43f5e6fd6d3 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -495,8 +495,6 @@ include::config/repack.txt[]\n> \n> include::config/rerere.txt[]\n> \n> -include::config/reset.txt[]\n> -\n> include::config/sendemail.txt[]\n> \n> include::config/sequencer.txt[]\n> diff --git a/Documentation/config/reset.txt b/Documentation/config/reset.txt\n> deleted file mode 100644\n> index 63b7c45aac2..00000000000\n> --- a/Documentation/config/reset.txt\n> +++ /dev/null\n> @@ -1,2 +0,0 @@\n> -reset.quiet::\n> -\tWhen set to true, 'git reset' will default to the '--quiet' option.\n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index bc1646c3016..f4aca9dd35c 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -105,10 +105,7 @@ OPTIONS\n> \n> -q::\n> --quiet::\n> ---no-quiet::\n> -\tBe quiet, only report errors. The default behavior is set by the\n> -\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n> -\toverride the default behavior.\n> +\tBe quiet, only report errors.\n> \n> --refresh::\n> --no-refresh::\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 7c3828f6fc5..e824aad3604 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t};\n> \n> \tgit_config(git_reset_config, NULL);\n> -\tgit_config_get_bool(\"reset.quiet\", &quiet);\n> \tgit_config_get_bool(\"reset.refresh\", &refresh);\n> \n> \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n> diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\n> index 7db2a97416e..58ca0e56f14 100644\n> --- a/contrib/scalar/scalar.c\n> +++ b/contrib/scalar/scalar.c\n> @@ -152,7 +152,6 @@ static int set_recommended_config(int reconfigure)\n> \t\t{ \"pack.useBitmaps\", \"false\", 1 },\n> \t\t{ \"pack.useSparse\", \"true\", 1 },\n> \t\t{ \"receive.autoGC\", \"false\", 1 },\n> -\t\t{ \"reset.quiet\", \"true\", 1 },\n> \t\t{ \"feature.manyFiles\", \"false\", 1 },\n> \t\t{ \"feature.experimental\", \"false\", 1 },\n> \t\t{ \"fetch.unpackLimit\", \"1\", 1 },\n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 8b62bb39b3d..9e4c4deee35 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -488,8 +488,7 @@ test_expect_success '--mixed refreshes the index' '\n> \t# Verify default behavior (without --[no-]refresh or reset.refresh)\n> \ttest_reset_refreshes_index &&\n> \n> -\t# With --quiet & reset.quiet\n> -\ttest_reset_refreshes_index \"-c reset.quiet=true\" &&\n> +\t# With --quiet\n> \ttest_reset_refreshes_index \"\" --quiet\n> '\n> \n\n"},{"id":"451999","messageId":"4f3cbfd5-65df-f44f-4867-45b1ab69c400@gmail.com","threadId":"57595","inReplyTo":"ecd3296fd25cc936aeb2f8ae160352a2326441c5.1647894889.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] reset: deprecate 'reset.refresh' config option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-23T16:02:06Z","receivedAt":"2022-03-23T16:02:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Victoria\n\nOn 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n> From: Victoria Dye \n\nSame concern about the title as the last patch\n\n> Remove the 'reset.refresh' option, requiring that users explicitly specify\n> '--no-refresh' if they want to skip refreshing the index.\n> \n> The 'reset.refresh' option was introduced in 101cee42dd (reset: introduce\n> --[no-]refresh option to --mixed, 2022-03-11) as a replacement for the\n> refresh-skipping behavior originally controlled by 'reset.quiet'.\n> \n> Although 'reset.refresh=false' functionally served the same purpose as\n> 'reset.quiet=true', it exposed [1] the fact that the existence of a global\n> \"skip refresh\" option could potentially cause problems for users. Allowing a\n> global config option to avoid refreshing the index forces scripts using 'git\n> reset --mixed' to defensively use '--refresh' if index refresh is expected;\n> if that option is missing, behavior of a script could vary from user-to-user\n> without explanation.\n> \n> Furthermore, globally disabling index refresh in 'reset --mixed' was\n> initially devised as a passive performance improvement; since the\n> introduction of the option, other changes have been made to Git (e.g., the\n> sparse index) with a greater potential performance impact without\n> sacrificing index correctness. Therefore, we can more aggressively err on\n> the side of correctness and limit the cases of skipping index refresh to\n> only when a user specifies the '--no-refresh' option.\n\nThanks for the well explained commit message\n> [1] https://lore.kernel.org/git/xmqqy2179o3c.fsf@gitster.g/\n> \n> Signed-off-by: Victoria Dye \n> ---\n> Documentation/git-reset.txt | 4 +---\n> builtin/reset.c | 4 +---\n> t/t7102-reset.sh | 14 ++------------\n> 3 files changed, 4 insertions(+), 18 deletions(-)\n> \n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index f4aca9dd35c..df167eaa766 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -109,9 +109,7 @@ OPTIONS\n> \n> --refresh::\n> --no-refresh::\n> -\tProactively refresh the index after a mixed reset. If unspecified, the\n> -\tbehavior falls back on the `reset.refresh` config option. If neither\n> -\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n> +\tProactively refresh the index after a mixed reset. Enabled by default.\n\n\"Proactively\" seems a but superfluous if I'm being picky. There was no \nentry in the config man page for reset.refresh so we don't need to do \nanything there. The code changes below look good\n\nBest Wishes\n\nPhillip\n\n\n> --pathspec-from-file=::\n> \tPathspec is passed in `` instead of commandline args. If\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index e824aad3604..54324217f93 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t};\n> \n> \tgit_config(git_reset_config, NULL);\n> -\tgit_config_get_bool(\"reset.refresh\", &refresh);\n> \n> \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> @@ -529,8 +528,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n> \t\t\t\tif (!quiet && advice_enabled(ADVICE_RESET_NO_REFRESH_WARNING) && t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n> \t\t\t\t\tadvise(_(\"It took %.2f seconds to refresh the index after reset. You can use\\n\"\n> -\t\t\t\t\t\t \"'--no-refresh' to avoid this. Set the config setting reset.refresh to false\\n\"\n> -\t\t\t\t\t\t \"to make this the default.\"), t_delta_in_ms / 1000.0);\n> +\t\t\t\t\t\t \"'--no-refresh' to avoid this.\"), t_delta_in_ms / 1000.0);\n> \t\t\t\t}\n> \t\t\t}\n> \t\t} else {\n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 9e4c4deee35..22477f3a312 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -493,19 +493,9 @@ test_expect_success '--mixed refreshes the index' '\n> '\n> \n> test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n> -\t# Verify that --[no-]refresh and `reset.refresh` control index refresh\n> -\n> -\t# Config setting\n> -\ttest_reset_refreshes_index \"-c reset.refresh=true\" &&\n> -\t! test_reset_refreshes_index \"-c reset.refresh=false\" &&\n> -\n> -\t# Command line option\n> +\t# Verify that --[no-]refresh controls index refresh\n> \ttest_reset_refreshes_index \"\" --refresh &&\n> -\t! test_reset_refreshes_index \"\" --no-refresh &&\n> -\n> -\t# Command line option overrides config setting\n> -\ttest_reset_refreshes_index \"-c reset.refresh=false\" --refresh &&\n> -\t! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n> +\t! test_reset_refreshes_index \"\" --no-refresh\n> '\n> \n> test_expect_success '--mixed preserves skip-worktree' '\n\n"},{"id":"452000","messageId":"3f8c28c6-5f99-e9ac-e356-48182e269b5c@gmail.com","threadId":"57595","inReplyTo":"dbb63c4caa83cc764535a739d20736b706ee44a5.1647894889.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/4] reset: deprecate '--refresh', leaving only '--no-refresh'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-23T16:02:27Z","receivedAt":"2022-03-23T16:02:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Victoria\n\nOn 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n> From: Victoria Dye \n> \n> The explicit '--refresh' option was needed in the past when '--quiet',\n> 'reset.quiet', and/or 'reset.refresh' disabled the index refresh in 'reset\n> --mixed'. Those options have since either been deprecated or made to always\n> refresh the index by default, leaving only '--[no-]refresh' determining\n> whether the index is refreshed or not.\n> \n> Because there is nothing other than '--no-refresh' to disable index refresh,\n> we do not need a '--refresh' option to counteract some other refresh\n> disabling.\n\n'--refresh' could be used to countermand a previous '--no-refresh' \n(typically when using an alias that includes '--no-refresh'). Usually we \nhave '--foo' even when it is enabled by default e.g. 'commit --verify'. \nI think the code below does actually support '--refresh' as \nparse_options() is smart enough to recognize option names beginning with \n'--no-' and creates an inverse option by removing the prefix rather than \nadding '--no-' as it normally does.\n\n> To ensure users don't use what is effectively a no-op, remove '--refresh'\n> from the set of 'reset' options, as well as its usage in 'git stash'.\n> \n> Signed-off-by: Victoria Dye \n> ---\n> Documentation/git-reset.txt | 3 +--\n> builtin/reset.c | 6 +++---\n> builtin/stash.c | 4 ++--\n> t/t7102-reset.sh | 5 ++---\n> 4 files changed, 8 insertions(+), 10 deletions(-)\n> \n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index df167eaa766..ba8dece0c03 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -107,9 +107,8 @@ OPTIONS\n> --quiet::\n> \tBe quiet, only report errors.\n> \n> ---refresh::\n> --no-refresh::\n> -\tProactively refresh the index after a mixed reset. Enabled by default.\n> +\tDisable refreshing the index after a mixed reset.\n> \n> --pathspec-from-file=::\n> \tPathspec is passed in `` instead of commandline args. If\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 54324217f93..d9427abc483 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -392,7 +392,7 @@ static int git_reset_config(const char *var, const char *value, void *cb)\n> int cmd_reset(int argc, const char **argv, const char *prefix)\n> {\n> \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n> -\tint refresh = -1;\n> +\tint refresh = 1;\n> \tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n> \tconst char *rev, *pathspec_from_file = NULL;\n> \tstruct object_id oid;\n> @@ -400,8 +400,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \tint intent_to_add = 0;\n> \tconst struct option options[] = {\n> \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n> -\t\tOPT_BOOL(0, \"refresh\", &refresh,\n> -\t\t\t\tN_(\"skip refreshing the index after reset\")),\n> +\t\tOPT_SET_INT(0, \"no-refresh\", &refresh,\n> +\t\t\t\tN_(\"skip refreshing the index after reset\"), 0),\n\nI'm confused why we need to change the option type here - am I missing \nsomething?\n\n\nBest Wishes\n\nPhillip\n"},{"id":"452004","messageId":"96e386f9-01b5-a231-f1f6-4d9ffefc8f8d@github.com","threadId":"57595","inReplyTo":"355f36d5-7153-42a4-6db8-b9ba5fcac422@gmail.com","subject":"Re: [PATCH 1/4] reset: do not make '--quiet' disable index refresh","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-03-23T16:52:30Z","receivedAt":"2022-03-23T16:52:35Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Phillip Wood wrote:\n> Hi Victoria\n> \n> On 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n>> From: Victoria Dye \n>>\n>> Update '--quiet' to no longer implicitly skip refreshing the index in a\n>> mixed reset. Users now have the ability to explicitly disable refreshing the\n>> index with the '--no-refresh' option, so they no longer need to use\n>> '--quiet' to do so. Moreover, we explicitly remove the refresh-skipping\n>> behavior from '--quiet' because it is completely unrelated to the stated\n>> purpose of the option: \"Be quiet, only report errors.\"\n>>\n>> Signed-off-by: Victoria Dye \n>> ---\n>>   Documentation/git-reset.txt |  5 +----\n>>   builtin/reset.c             |  7 -------\n>>   t/t7102-reset.sh            | 32 +++++---------------------------\n>>   3 files changed, 6 insertions(+), 38 deletions(-)\n>>\n>> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n>> index 89ddc85c2e4..bc1646c3016 100644\n>> --- a/Documentation/git-reset.txt\n>> +++ b/Documentation/git-reset.txt\n>> @@ -114,10 +114,7 @@ OPTIONS\n>>   --no-refresh::\n>>       Proactively refresh the index after a mixed reset. If unspecified, the\n>>       behavior falls back on the `reset.refresh` config option. If neither\n>> -    `--[no-]refresh` nor `reset.refresh` are set, the default behavior is\n>> -    decided by the `--[no-]quiet` option and/or `reset.quiet` config.\n>> -    If `--quiet` is specified or `reset.quiet` is set with no command-line\n>> -    \"quiet\" setting, refresh is disabled. Otherwise, refresh is enabled.\n>> +    `--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n>>     --pathspec-from-file=::\n>>       Pathspec is passed in `` instead of commandline args. If\n>> diff --git a/builtin/reset.c b/builtin/reset.c\n>> index c8a356ec5b0..7c3828f6fc5 100644\n>> --- a/builtin/reset.c\n>> +++ b/builtin/reset.c\n>> @@ -430,13 +430,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>>                           PARSE_OPT_KEEP_DASHDASH);\n>>       parse_args(&pathspec, argv, prefix, patch_mode, &rev);\n>>   -    /*\n>> -     * If refresh is completely unspecified (either by config or by command\n>> -     * line option), decide based on 'quiet'.\n>> -     */\n>> -    if (refresh < 0)\n>> -        refresh = !quiet;\n> \n> Does this mean we can stop initializing refresh to -1?\n> \n> Best Wishes\n> \n> Phillip\n\nYes, thanks for catching that! It should be initialized to 1 in this patch\n(rather than later in patch 4).\n"},{"id":"452005","messageId":"017dd12a-7b9e-4150-1327-8ca45435a097@github.com","threadId":"57595","inReplyTo":"3f8c28c6-5f99-e9ac-e356-48182e269b5c@gmail.com","subject":"Re: [PATCH 4/4] reset: deprecate '--refresh', leaving only '--no-refresh'","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-03-23T16:58:37Z","receivedAt":"2022-03-23T16:58:42Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Phillip Wood wrote:\n> Hi Victoria\n> \n> On 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n>> From: Victoria Dye \n>>\n>> The explicit '--refresh' option was needed in the past when '--quiet',\n>> 'reset.quiet', and/or 'reset.refresh' disabled the index refresh in 'reset\n>> --mixed'. Those options have since either been deprecated or made to always\n>> refresh the index by default, leaving only '--[no-]refresh' determining\n>> whether the index is refreshed or not.\n>>\n>> Because there is nothing other than '--no-refresh' to disable index refresh,\n>> we do not need a '--refresh' option to counteract some other refresh\n>> disabling.\n> \n> '--refresh' could be used to countermand a previous '--no-refresh' (typically when using an alias that includes '--no-refresh'). Usually we have '--foo' even when it is enabled by default e.g. 'commit --verify'. I think the code below does actually support '--refresh' as parse_options() is smart enough to recognize option names beginning with '--no-' and creates an inverse option by removing the prefix rather than adding '--no-' as it normally does.\n> \n\nMakes sense, I didn't think of the alias use-case/generally wanting to\ncounteract the negative version of the option. Thanks! \n\n>> To ensure users don't use what is effectively a no-op, remove '--refresh'\n>> from the set of 'reset' options, as well as its usage in 'git stash'.\n>>\n>> Signed-off-by: Victoria Dye \n>> ---\n>>   Documentation/git-reset.txt | 3 +--\n>>   builtin/reset.c             | 6 +++---\n>>   builtin/stash.c             | 4 ++--\n>>   t/t7102-reset.sh            | 5 ++---\n>>   4 files changed, 8 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n>> index df167eaa766..ba8dece0c03 100644\n>> --- a/Documentation/git-reset.txt\n>> +++ b/Documentation/git-reset.txt\n>> @@ -107,9 +107,8 @@ OPTIONS\n>>   --quiet::\n>>       Be quiet, only report errors.\n>>   ---refresh::\n>>   --no-refresh::\n>> -    Proactively refresh the index after a mixed reset. Enabled by default.\n>> +    Disable refreshing the index after a mixed reset.\n>>     --pathspec-from-file=::\n>>       Pathspec is passed in `` instead of commandline args. If\n>> diff --git a/builtin/reset.c b/builtin/reset.c\n>> index 54324217f93..d9427abc483 100644\n>> --- a/builtin/reset.c\n>> +++ b/builtin/reset.c\n>> @@ -392,7 +392,7 @@ static int git_reset_config(const char *var, const char *value, void *cb)\n>>   int cmd_reset(int argc, const char **argv, const char *prefix)\n>>   {\n>>       int reset_type = NONE, update_ref_status = 0, quiet = 0;\n>> -    int refresh = -1;\n>> +    int refresh = 1;\n>>       int patch_mode = 0, pathspec_file_nul = 0, unborn;\n>>       const char *rev, *pathspec_from_file = NULL;\n>>       struct object_id oid;\n>> @@ -400,8 +400,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>>       int intent_to_add = 0;\n>>       const struct option options[] = {\n>>           OPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n>> -        OPT_BOOL(0, \"refresh\", &refresh,\n>> -                N_(\"skip refreshing the index after reset\")),\n>> +        OPT_SET_INT(0, \"no-refresh\", &refresh,\n>> +                N_(\"skip refreshing the index after reset\"), 0),\n> \n> I'm confused why we need to change the option type here - am I missing something?\n> \n\nThis was to explicitly prevent a user from specifying \"--refresh\", only\nallowing \"--no-refresh\". But, based on your earlier comments, there's still\nsome value in having \"--refresh\", so I'll drop this patch entirely.\n\n> \n> Best Wishes\n> \n> Phillip\n\n"},{"id":"452006","messageId":"1d78ed3f-9644-e1c2-b95b-e1ae97df2c43@github.com","threadId":"57595","inReplyTo":"4f3cbfd5-65df-f44f-4867-45b1ab69c400@gmail.com","subject":"Re: [PATCH 3/4] reset: deprecate 'reset.refresh' config option","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-03-23T17:19:45Z","receivedAt":"2022-03-23T17:19:50Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Phillip Wood wrote:\n> Hi Victoria\n> \n> On 21/03/2022 20:34, Victoria Dye via GitGitGadget wrote:\n>> From: Victoria Dye \n> \n> Same concern about the title as the last patch\n> \n\nAgreed, I'll change it to \"remove\" in this and the previous patch.\n\n>> Remove the 'reset.refresh' option, requiring that users explicitly specify\n>> '--no-refresh' if they want to skip refreshing the index.\n>>\n>> The 'reset.refresh' option was introduced in 101cee42dd (reset: introduce\n>> --[no-]refresh option to --mixed, 2022-03-11) as a replacement for the\n>> refresh-skipping behavior originally controlled by 'reset.quiet'.\n>>\n>> Although 'reset.refresh=false' functionally served the same purpose as\n>> 'reset.quiet=true', it exposed [1] the fact that the existence of a global\n>> \"skip refresh\" option could potentially cause problems for users. Allowing a\n>> global config option to avoid refreshing the index forces scripts using 'git\n>> reset --mixed' to defensively use '--refresh' if index refresh is expected;\n>> if that option is missing, behavior of a script could vary from user-to-user\n>> without explanation.\n>>\n>> Furthermore, globally disabling index refresh in 'reset --mixed' was\n>> initially devised as a passive performance improvement; since the\n>> introduction of the option, other changes have been made to Git (e.g., the\n>> sparse index) with a greater potential performance impact without\n>> sacrificing index correctness. Therefore, we can more aggressively err on\n>> the side of correctness and limit the cases of skipping index refresh to\n>> only when a user specifies the '--no-refresh' option.\n> \n> Thanks for the well explained commit message\n>> [1] https://lore.kernel.org/git/xmqqy2179o3c.fsf@gitster.g/\n>>\n>> Signed-off-by: Victoria Dye \n>> ---\n>>   Documentation/git-reset.txt |  4 +---\n>>   builtin/reset.c             |  4 +---\n>>   t/t7102-reset.sh            | 14 ++------------\n>>   3 files changed, 4 insertions(+), 18 deletions(-)\n>>\n>> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n>> index f4aca9dd35c..df167eaa766 100644\n>> --- a/Documentation/git-reset.txt\n>> +++ b/Documentation/git-reset.txt\n>> @@ -109,9 +109,7 @@ OPTIONS\n>>     --refresh::\n>>   --no-refresh::\n>> -    Proactively refresh the index after a mixed reset. If unspecified, the\n>> -    behavior falls back on the `reset.refresh` config option. If neither\n>> -    `--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n>> +    Proactively refresh the index after a mixed reset. Enabled by default.\n> \n> \"Proactively\" seems a but superfluous if I'm being picky. There was no entry in the config man page for reset.refresh so we don't need to do anything there. The code changes below look good\n> \n\nWill update, thanks!\n\n> Best Wishes\n> \n> Phillip\n> \n> \n>>   --pathspec-from-file=::\n>>       Pathspec is passed in `` instead of commandline args. If\n>> diff --git a/builtin/reset.c b/builtin/reset.c\n>> index e824aad3604..54324217f93 100644\n>> --- a/builtin/reset.c\n>> +++ b/builtin/reset.c\n>> @@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>>       };\n>>         git_config(git_reset_config, NULL);\n>> -    git_config_get_bool(\"reset.refresh\", &refresh);\n>>         argc = parse_options(argc, argv, prefix, options, git_reset_usage,\n>>                           PARSE_OPT_KEEP_DASHDASH);\n>> @@ -529,8 +528,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>>                   t_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n>>                   if (!quiet && advice_enabled(ADVICE_RESET_NO_REFRESH_WARNING) && t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n>>                       advise(_(\"It took %.2f seconds to refresh the index after reset.  You can use\\n\"\n>> -                         \"'--no-refresh' to avoid this.  Set the config setting reset.refresh to false\\n\"\n>> -                         \"to make this the default.\"), t_delta_in_ms / 1000.0);\n>> +                         \"'--no-refresh' to avoid this.\"), t_delta_in_ms / 1000.0);\n>>                   }\n>>               }\n>>           } else {\n>> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n>> index 9e4c4deee35..22477f3a312 100755\n>> --- a/t/t7102-reset.sh\n>> +++ b/t/t7102-reset.sh\n>> @@ -493,19 +493,9 @@ test_expect_success '--mixed refreshes the index' '\n>>   '\n>>     test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n>> -    # Verify that --[no-]refresh and `reset.refresh` control index refresh\n>> -\n>> -    # Config setting\n>> -    test_reset_refreshes_index \"-c reset.refresh=true\" &&\n>> -    ! test_reset_refreshes_index \"-c reset.refresh=false\" &&\n>> -\n>> -    # Command line option\n>> +    # Verify that --[no-]refresh controls index refresh\n>>       test_reset_refreshes_index \"\" --refresh &&\n>> -    ! test_reset_refreshes_index \"\" --no-refresh &&\n>> -\n>> -    # Command line option overrides config setting\n>> -    test_reset_refreshes_index \"-c reset.refresh=false\" --refresh &&\n>> -    ! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n>> +    ! test_reset_refreshes_index \"\" --no-refresh\n>>   '\n>>     test_expect_success '--mixed preserves skip-worktree' '\n> \n\n"},{"id":"452010","messageId":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.git.1647894889.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-23T18:17:57Z","receivedAt":"2022-03-23T18:18:08Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Maintainer's note: this is based on vd/stash-silence-reset (specifically,\n4b8b0f6fa2 (stash: make internal resets quiet and refresh index,\n2022-03-15)).\n\n----------------------------------------------------------------------------\n\nThis is a follow-up to the changes in vd/stash-silence-reset [1], in which\nindex refreshing behavior was decoupled from log silencing in the '--quiet'\noption to 'git reset --mixed' by introducing a '--[no-]refresh' option and\n'reset.refresh' config setting.\n\nAfter some discussion [2] on the mailing list, both the\nbackward-compatibility and use of global options in that series came into\nquestion:\n\n * '--quiet' still skipped refresh if neither '--[no-]refresh' nor\n 'reset.refresh' were specified, meaning that users could still be left\n with an incorrect index state after reset.\n * Having 'reset.quiet' and/or 'reset.refresh' potentially disable index\n refresh by default meant that developers would need to defensively add\n '--refresh' to all internal uses of 'git reset --mixed'. Without that\n option, different config setups could cause variability in index\n correctness from user to user.\n\nIn response, this series removes all methods of skipping index refresh in\n'git reset --mixed' except for '--no-refresh' itself:\n\n * Patch [1/3] removes the \"fallback\" behavior of 'reset.quiet' and\n '--quiet' implying '--no-refresh' if neither '--[no-]refresh' nor\n 'config.refresh' were specified. In other words, '--quiet' no longer does\n anything other than log silencing.\n * Patch [2/3] removes 'reset.quiet', since its main use was to disable\n index refresh until that behavior was removed in [1/3].\n * Patch [3/3] removes 'reset.refresh' to avoid users accidentally ending up\n with an incorrect index state after all resets as a result of a global\n setting's passive effects.\n\n\nChanges since V1\n================\n\n * Dropped patch that removed '--refresh', again allowing both\n '--no-refresh' and '--refresh' as valid options.\n * Updated documentation of \"--refresh\" option to remove unnecessary\n \"proactively\".\n * Reworded commit titles to change \"deprecate\" to the more accurate\n \"remove\".\n\n[1]\nhttps://lore.kernel.org/git/pull.1170.v3.git.1647308982.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/80a2a5a2-256f-6c3b-2430-10bef99ce1e9@github.com/\n\nThanks! -Victoria\n\nVictoria Dye (3):\n reset: do not make '--quiet' disable index refresh\n reset: remove 'reset.quiet' config option\n reset: remove 'reset.refresh' config option\n\n Documentation/config.txt | 2 --\n Documentation/config/reset.txt | 2 --\n Documentation/git-reset.txt | 12 ++-------\n builtin/reset.c | 14 ++---------\n contrib/scalar/scalar.c | 1 -\n t/t7102-reset.sh | 45 +++++-----------------------------\n 6 files changed, 10 insertions(+), 66 deletions(-)\n delete mode 100644 Documentation/config/reset.txt\n\n\nbase-commit: 877d90220e42b40cf5b899dc36a13c348220b54c\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1184%2Fvdye%2Freset%2Fclean-up-refresh-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1184/vdye/reset/clean-up-refresh-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1184\n\nRange-diff vs v1:\n\n 1: f89e9b4ae24 ! 1: 1b607e0610b reset: do not make '--quiet' disable index refresh\n @@ Commit message\n behavior from '--quiet' because it is completely unrelated to the stated\n purpose of the option: \"Be quiet, only report errors.\"\n \n + Helped-by: Phillip Wood \n Signed-off-by: Victoria Dye \n \n ## Documentation/git-reset.txt ##\n @@ Documentation/git-reset.txt: OPTIONS\n \tPathspec is passed in `` instead of commandline args. If\n \n ## builtin/reset.c ##\n +@@ builtin/reset.c: static int git_reset_config(const char *var, const char *value, void *cb)\n + int cmd_reset(int argc, const char **argv, const char *prefix)\n + {\n + \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n +-\tint refresh = -1;\n ++\tint refresh = 1;\n + \tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n + \tconst char *rev, *pathspec_from_file = NULL;\n + \tstruct object_id oid;\n @@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n 2: d9bebd4b4e0 ! 2: a25aff3ac7c reset: deprecate 'reset.quiet' config option\n @@ Metadata\n Author: Victoria Dye \n \n ## Commit message ##\n - reset: deprecate 'reset.quiet' config option\n + reset: remove 'reset.quiet' config option\n \n Remove the 'reset.quiet' config option, remove '--no-quiet' documentation in\n 'Documentation/git-reset.txt'. In 4c3abd0551 (reset: add new reset.quiet\n 3: ecd3296fd25 ! 3: 597aa82851c reset: deprecate 'reset.refresh' config option\n @@ Metadata\n Author: Victoria Dye \n \n ## Commit message ##\n - reset: deprecate 'reset.refresh' config option\n + reset: remove 'reset.refresh' config option\n \n Remove the 'reset.refresh' option, requiring that users explicitly specify\n '--no-refresh' if they want to skip refreshing the index.\n @@ Documentation/git-reset.txt: OPTIONS\n -\tProactively refresh the index after a mixed reset. If unspecified, the\n -\tbehavior falls back on the `reset.refresh` config option. If neither\n -\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n -+\tProactively refresh the index after a mixed reset. Enabled by default.\n ++\tRefresh the index after a mixed reset. Enabled by default.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\n 4: dbb63c4caa8 < -: ----------- reset: deprecate '--refresh', leaving only '--no-refresh'\n\n-- \ngitgitgadget\n"},{"id":"452011","messageId":"1b607e0610b6535b2cc2bbda22eb57c4bf2d11e7.1648059480.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] reset: do not make '--quiet' disable index refresh","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-23T18:17:58Z","receivedAt":"2022-03-23T18:18:10Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nUpdate '--quiet' to no longer implicitly skip refreshing the index in a\nmixed reset. Users now have the ability to explicitly disable refreshing the\nindex with the '--no-refresh' option, so they no longer need to use\n'--quiet' to do so. Moreover, we explicitly remove the refresh-skipping\nbehavior from '--quiet' because it is completely unrelated to the stated\npurpose of the option: \"Be quiet, only report errors.\"\n\nHelped-by: Phillip Wood \nSigned-off-by: Victoria Dye \n---\n Documentation/git-reset.txt | 5 +----\n builtin/reset.c | 9 +--------\n t/t7102-reset.sh | 32 +++++---------------------------\n 3 files changed, 7 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 89ddc85c2e4..bc1646c3016 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -114,10 +114,7 @@ OPTIONS\n --no-refresh::\n \tProactively refresh the index after a mixed reset. If unspecified, the\n \tbehavior falls back on the `reset.refresh` config option. If neither\n-\t`--[no-]refresh` nor `reset.refresh` are set, the default behavior is\n-\tdecided by the `--[no-]quiet` option and/or `reset.quiet` config.\n-\tIf `--quiet` is specified or `reset.quiet` is set with no command-line\n-\t\"quiet\" setting, refresh is disabled. Otherwise, refresh is enabled.\n+\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c8a356ec5b0..1804d0eeb84 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -392,7 +392,7 @@ static int git_reset_config(const char *var, const char *value, void *cb)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint refresh = -1;\n+\tint refresh = 1;\n \tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n \tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n@@ -430,13 +430,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n-\t/*\n-\t * If refresh is completely unspecified (either by config or by command\n-\t * line option), decide based on 'quiet'.\n-\t */\n-\tif (refresh < 0)\n-\t\trefresh = !quiet;\n-\n \tif (pathspec_from_file) {\n \t\tif (patch_mode)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--pathspec-from-file\", \"--patch\");\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 1dc3911a060..8b62bb39b3d 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -485,25 +485,12 @@ test_reset_refreshes_index () {\n }\n \n test_expect_success '--mixed refreshes the index' '\n-\t# Verify default behavior (with no config settings or command line\n-\t# options)\n-\ttest_reset_refreshes_index\n-'\n-test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '\n-\t# Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)\n-\t# determine refresh behavior\n-\n-\t# Config setting\n-\t! test_reset_refreshes_index \"-c reset.quiet=true\" &&\n-\ttest_reset_refreshes_index \"-c reset.quiet=false\" &&\n-\n-\t# Command line option\n-\t! test_reset_refreshes_index \"\" --quiet &&\n-\ttest_reset_refreshes_index \"\" --no-quiet &&\n+\t# Verify default behavior (without --[no-]refresh or reset.refresh)\n+\ttest_reset_refreshes_index &&\n \n-\t# Command line option overrides config setting\n-\t! test_reset_refreshes_index \"-c reset.quiet=false\" --quiet &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" --no-quiet\n+\t# With --quiet & reset.quiet\n+\ttest_reset_refreshes_index \"-c reset.quiet=true\" &&\n+\ttest_reset_refreshes_index \"\" --quiet\n '\n \n test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n@@ -522,15 +509,6 @@ test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n \t! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n '\n \n-test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '\n-\t# Verify that *both* --refresh and `reset.refresh` override the\n-\t# default non-refresh behavior of --quiet\n-\ttest_reset_refreshes_index \"\" \"--quiet --refresh\" &&\n-\ttest_reset_refreshes_index \"-c reset.quiet=true\" --refresh &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" --quiet &&\n-\ttest_reset_refreshes_index \"-c reset.refresh=true -c reset.quiet=true\"\n-'\n-\n test_expect_success '--mixed preserves skip-worktree' '\n \techo 123 >>file2 &&\n \tgit add file2 &&\n-- \ngitgitgadget\n\n"},{"id":"452012","messageId":"a25aff3ac7c5835445b4387c3d4cce82c6c36758.1648059480.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] reset: remove 'reset.quiet' config option","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-23T18:17:59Z","receivedAt":"2022-03-23T18:18:13Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nRemove the 'reset.quiet' config option, remove '--no-quiet' documentation in\n'Documentation/git-reset.txt'. In 4c3abd0551 (reset: add new reset.quiet\nconfig setting, 2018-10-23), 'reset.quiet' was introduced as a way to\nglobally change the default behavior of 'git reset --mixed' to skip index\nrefresh.\n\nHowever, now that '--quiet' does not affect index refresh, 'reset.quiet'\nwould only serve to globally silence logging. This was not the original\nintention of the config setting, and there's no precedent for such a setting\nin other commands with a '--quiet' option, so it appears to be obsolete.\n\nIn addition to the options & its documentation, remove 'reset.quiet' from\nthe recommended config for 'scalar'.\n\nSigned-off-by: Victoria Dye \n---\n Documentation/config.txt | 2 --\n Documentation/config/reset.txt | 2 --\n Documentation/git-reset.txt | 5 +----\n builtin/reset.c | 1 -\n contrib/scalar/scalar.c | 1 -\n t/t7102-reset.sh | 3 +--\n 6 files changed, 2 insertions(+), 12 deletions(-)\n delete mode 100644 Documentation/config/reset.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f0fb25a371c..43f5e6fd6d3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -495,8 +495,6 @@ include::config/repack.txt[]\n \n include::config/rerere.txt[]\n \n-include::config/reset.txt[]\n-\n include::config/sendemail.txt[]\n \n include::config/sequencer.txt[]\ndiff --git a/Documentation/config/reset.txt b/Documentation/config/reset.txt\ndeleted file mode 100644\nindex 63b7c45aac2..00000000000\n--- a/Documentation/config/reset.txt\n+++ /dev/null\n@@ -1,2 +0,0 @@\n-reset.quiet::\n-\tWhen set to true, 'git reset' will default to the '--quiet' option.\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex bc1646c3016..f4aca9dd35c 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -105,10 +105,7 @@ OPTIONS\n \n -q::\n --quiet::\n---no-quiet::\n-\tBe quiet, only report errors. The default behavior is set by the\n-\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n-\toverride the default behavior.\n+\tBe quiet, only report errors.\n \n --refresh::\n --no-refresh::\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 1804d0eeb84..9ce55afd1be 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n-\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \tgit_config_get_bool(\"reset.refresh\", &refresh);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\ndiff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c\nindex 7db2a97416e..58ca0e56f14 100644\n--- a/contrib/scalar/scalar.c\n+++ b/contrib/scalar/scalar.c\n@@ -152,7 +152,6 @@ static int set_recommended_config(int reconfigure)\n \t\t{ \"pack.useBitmaps\", \"false\", 1 },\n \t\t{ \"pack.useSparse\", \"true\", 1 },\n \t\t{ \"receive.autoGC\", \"false\", 1 },\n-\t\t{ \"reset.quiet\", \"true\", 1 },\n \t\t{ \"feature.manyFiles\", \"false\", 1 },\n \t\t{ \"feature.experimental\", \"false\", 1 },\n \t\t{ \"fetch.unpackLimit\", \"1\", 1 },\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 8b62bb39b3d..9e4c4deee35 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -488,8 +488,7 @@ test_expect_success '--mixed refreshes the index' '\n \t# Verify default behavior (without --[no-]refresh or reset.refresh)\n \ttest_reset_refreshes_index &&\n \n-\t# With --quiet & reset.quiet\n-\ttest_reset_refreshes_index \"-c reset.quiet=true\" &&\n+\t# With --quiet\n \ttest_reset_refreshes_index \"\" --quiet\n '\n \n-- \ngitgitgadget\n\n"},{"id":"452013","messageId":"597aa82851c77b79bd296f9c3c9bab8907fe0e95.1648059480.git.gitgitgadget@gmail.com","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] reset: remove 'reset.refresh' config option","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-23T18:18:00Z","receivedAt":"2022-03-23T18:18:14Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye \n\nRemove the 'reset.refresh' option, requiring that users explicitly specify\n'--no-refresh' if they want to skip refreshing the index.\n\nThe 'reset.refresh' option was introduced in 101cee42dd (reset: introduce\n--[no-]refresh option to --mixed, 2022-03-11) as a replacement for the\nrefresh-skipping behavior originally controlled by 'reset.quiet'.\n\nAlthough 'reset.refresh=false' functionally served the same purpose as\n'reset.quiet=true', it exposed [1] the fact that the existence of a global\n\"skip refresh\" option could potentially cause problems for users. Allowing a\nglobal config option to avoid refreshing the index forces scripts using 'git\nreset --mixed' to defensively use '--refresh' if index refresh is expected;\nif that option is missing, behavior of a script could vary from user-to-user\nwithout explanation.\n\nFurthermore, globally disabling index refresh in 'reset --mixed' was\ninitially devised as a passive performance improvement; since the\nintroduction of the option, other changes have been made to Git (e.g., the\nsparse index) with a greater potential performance impact without\nsacrificing index correctness. Therefore, we can more aggressively err on\nthe side of correctness and limit the cases of skipping index refresh to\nonly when a user specifies the '--no-refresh' option.\n\n[1] https://lore.kernel.org/git/xmqqy2179o3c.fsf@gitster.g/\n\nSigned-off-by: Victoria Dye \n---\n Documentation/git-reset.txt | 4 +---\n builtin/reset.c | 4 +---\n t/t7102-reset.sh | 14 ++------------\n 3 files changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex f4aca9dd35c..01cb4c9b9c5 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -109,9 +109,7 @@ OPTIONS\n \n --refresh::\n --no-refresh::\n-\tProactively refresh the index after a mixed reset. If unspecified, the\n-\tbehavior falls back on the `reset.refresh` config option. If neither\n-\t`--[no-]refresh` nor `reset.refresh` are set, refresh is enabled.\n+\tRefresh the index after a mixed reset. Enabled by default.\n \n --pathspec-from-file=::\n \tPathspec is passed in `` instead of commandline args. If\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9ce55afd1be..1d89faef5ec 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -423,7 +423,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n-\tgit_config_get_bool(\"reset.refresh\", &refresh);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n@@ -529,8 +528,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n \t\t\t\tif (!quiet && advice_enabled(ADVICE_RESET_NO_REFRESH_WARNING) && t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n \t\t\t\t\tadvise(_(\"It took %.2f seconds to refresh the index after reset. You can use\\n\"\n-\t\t\t\t\t\t \"'--no-refresh' to avoid this. Set the config setting reset.refresh to false\\n\"\n-\t\t\t\t\t\t \"to make this the default.\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t\t\t \"'--no-refresh' to avoid this.\"), t_delta_in_ms / 1000.0);\n \t\t\t\t}\n \t\t\t}\n \t\t} else {\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 9e4c4deee35..22477f3a312 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -493,19 +493,9 @@ test_expect_success '--mixed refreshes the index' '\n '\n \n test_expect_success '--mixed --[no-]refresh sets refresh behavior' '\n-\t# Verify that --[no-]refresh and `reset.refresh` control index refresh\n-\n-\t# Config setting\n-\ttest_reset_refreshes_index \"-c reset.refresh=true\" &&\n-\t! test_reset_refreshes_index \"-c reset.refresh=false\" &&\n-\n-\t# Command line option\n+\t# Verify that --[no-]refresh controls index refresh\n \ttest_reset_refreshes_index \"\" --refresh &&\n-\t! test_reset_refreshes_index \"\" --no-refresh &&\n-\n-\t# Command line option overrides config setting\n-\ttest_reset_refreshes_index \"-c reset.refresh=false\" --refresh &&\n-\t! test_reset_refreshes_index \"-c reset.refresh=true\" --no-refresh\n+\t! test_reset_refreshes_index \"\" --no-refresh\n '\n \n test_expect_success '--mixed preserves skip-worktree' '\n-- \ngitgitgadget\n"},{"id":"452019","messageId":"d279c648-36e4-2fd9-90ee-b3cff7d9a1e2@github.com","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-23T19:26:53Z","receivedAt":"2022-03-23T19:27:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/23/2022 2:17 PM, Victoria Dye via GitGitGadget wrote:\n> This is a follow-up to the changes in vd/stash-silence-reset [1], in which\n> index refreshing behavior was decoupled from log silencing in the '--quiet'\n> option to 'git reset --mixed' by introducing a '--[no-]refresh' option and\n> 'reset.refresh' config setting.\n\n> Changes since V1\n> ================\n> \n> * Dropped patch that removed '--refresh', again allowing both\n> '--no-refresh' and '--refresh' as valid options.\n> * Updated documentation of \"--refresh\" option to remove unnecessary\n> \"proactively\".\n> * Reworded commit titles to change \"deprecate\" to the more accurate\n> \"remove\".\n\nI read through the patches in v2 along with the discussion from v1\nand the changes due to that discussion. This version looks good to me.\n\nThanks,\n-Stolee\n"},{"id":"452073","messageId":"xmqq5yo4qrcp.fsf@gitster.g","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-23T21:41:26Z","receivedAt":"2022-03-23T21:41:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Victoria Dye via GitGitGadget\" writes:\n\n> Changes since V1\n> ================\n>\n> * Dropped patch that removed '--refresh', again allowing both\n> '--no-refresh' and '--refresh' as valid options.\n\nMakes sense. Thanks for suggesting this, Phillip.\n\n> * Updated documentation of \"--refresh\" option to remove unnecessary\n> \"proactively\".\n\nOK.\n\n> * Reworded commit titles to change \"deprecate\" to the more accurate\n> \"remove\".\n\nOK.\n\nWill queue. Thanks.\n"},{"id":"452102","messageId":"6d424915-5c63-bebd-f3b5-f153a541d077@gmail.com","threadId":"57595","inReplyTo":"pull.1184.v2.git.1648059480.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-24T11:11:02Z","receivedAt":"2022-03-24T11:11:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Victoria\n\nOn 23/03/2022 18:17, Victoria Dye via GitGitGadget wrote:\n\n> Changes since V1\n> ================\n> \n> * Dropped patch that removed '--refresh', again allowing both\n> '--no-refresh' and '--refresh' as valid options.\n\nI should have been clearer in my comments that I think changing the \noption name no '--no-refresh' whilst retaining '--refresh' is \nworthwhile. '--no-refresh' is the form that users will want most of the \ntime and changing the option name means that the useful version will be \nshown by \"reset -h\".\n\nThe range-diff for the other changes looks good\n\nBest Wishes\n\nPhillip\n"},{"id":"452196","messageId":"xmqqsfr7jmtb.fsf@gitster.g","threadId":"57595","inReplyTo":"6d424915-5c63-bebd-f3b5-f153a541d077@gmail.com","subject":"Re: [PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-24T17:13:36Z","receivedAt":"2022-03-24T17:13:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I should have been clearer in my comments that I think changing the\n> option name no '--no-refresh' whilst retaining '--refresh' is \n> worthwhile. '--no-refresh' is the form that users will want most of\n> the time and changing the option name means that the useful version\n> will be shown by \"reset -h\".\n\nI am OK with it shown as \"--[no-]refresh\" or even \"--refresh\" alone,\nas long as the description describes the \"refresh\" behaviour and\nmakes it clear that it is the default, with the expectation that the\nusers know from other boolean options that \"--option\" listed in \"-h\"\nwould naturally take \"--no-option\".\n\nBut as posted, \n\n $ rungit seen reset -h 2>&1 | grep refresh\n --refresh skip refreshing the index after reset\n\nthe explanation given is for \"--no-refresh\" (which is wrong), so\nwe'd need some fix in the area. We could rephrase it to read\n\n --refresh refresh the index after reset (default)\n\nbut as you suggested, I think mimicking how existing commands with\n\"--no-