Skip to content

reset: make --no-refresh the only way to skip index refresh

Victoria Dye via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest3 patches 9 repliesIn review
  2. v14 patches 7 repliesSuperseded

Changes in v2 from the cover letter

================

* Dropped patch that removed '--refresh', again allowing both '--no-refresh' and '--refresh' as valid options.
* Updated documentation of "--refresh" option to remove unnecessary "proactively".
* Reworded commit titles to change "deprecate" to the more accurate "remove".

[1]
https://lore.kernel.org/git/[email protected]/
[2]
https://lore.kernel.org/git/[email protected]/

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3reset: do not make '--quiet' disable index refreshDocumentation/git-reset.txt builtin/reset.c t/t7102-reset.sh+7 -390 replies
2/3reset: remove 'reset.quiet' config optionDocumentation/config.txt Documentation/config/reset.txt Documentation/git-reset.txt +3 more+2 -120 replies
3/3reset: remove 'reset.refresh' config optionDocumentation/git-reset.txt builtin/reset.c t/t7102-reset.sh+4 -180 replies

Reviews trailers per patch

None of the 3 patches has a Reviewed-by or Acked-by.

PatchPhillip Wood
SeriesHelped-by
1/3Helped-by
2/3
3/3

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [PATCH v2 0/3] reset: make --no-refresh the only way to skip index refresh

Maintainer's note: this is based on vd/stash-silence-reset (specifically, 4b8b0f6fa2 (stash: make internal resets quiet and refresh index, 2022-03-15)).

----------------------------------------------------------------------------

This is a follow-up to the changes in vd/stash-silence-reset [1], in which index refreshing behavior was decoupled from log silencing in the '--quiet' option to 'git reset --mixed' by introducing a '--[no-]refresh' option and 'reset.refresh' config setting.

After some discussion [2] on the mailing list, both the backward-compatibility and use of global options in that series came into question:

* '--quiet' still skipped refresh if neither '--[no-]refresh' nor 'reset.refresh' were specified, meaning that users could still be left with an incorrect index state after reset.
* Having 'reset.quiet' and/or 'reset.refresh' potentially disable index refresh by default meant that developers would need to defensively add '--refresh' to all internal uses of 'git reset --mixed'. Without that option, different config setups could cause variability in index correctness from user to user.

In response, this series removes all methods of skipping index refresh in 'git reset --mixed' except for '--no-refresh' itself:

* Patch [1/3] removes the "fallback" behavior of 'reset.quiet' and '--quiet' implying '--no-refresh' if neither '--[no-]refresh' nor 'config.refresh' were specified. In other words, '--quiet' no longer does anything other than log silencing.
* Patch [2/3] removes 'reset.quiet', since its main use was to disable index refresh until that behavior was removed in [1/3].
* Patch [3/3] removes 'reset.refresh' to avoid users accidentally ending up with an incorrect index state after all resets as a result of a global setting's passive effects.

Changes since V1
================

* Dropped patch that removed '--refresh', again allowing both '--no-refresh' and '--refresh' as valid options.
* Updated documentation of "--refresh" option to remove unnecessary "proactively".
* Reworded commit titles to change "deprecate" to the more accurate "remove".

[1]
https://lore.kernel.org/git/[email protected]/
[2]
https://lore.kernel.org/git/[email protected]/

Thanks! -Victoria

Victoria Dye (3):
reset: do not make '--quiet' disable index refresh
reset: remove 'reset.quiet' config option
reset: remove 'reset.refresh' config option

 Documentation/config.txt       |  2 --
 Documentation/config/reset.txt |  2 --
 Documentation/git-reset.txt    | 12 ++-------
 builtin/reset.c                | 14 ++---------
 contrib/scalar/scalar.c        |  1 -
 t/t7102-reset.sh               | 45 +++++-----------------------------
 6 files changed, 10 insertions(+), 66 deletions(-)
 delete mode 100644 Documentation/config/reset.txt

base-commit: 877d90220e42b40cf5b899dc36a13c348220b54c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1184%2Fvdye%2Freset%2Fclean-up-refresh-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1184/vdye/reset/clean-up-refresh-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1184

Range-diff vs v1:

1: f89e9b4ae24 ! 1: 1b607e0610b reset: do not make '--quiet' disable index refresh
@@ Commit message
behavior from '--quiet' because it is completely unrelated to the stated purpose of the option: "Be quiet, only report errors."

+ Helped-by: Phillip Wood <[email protected]>

## Documentation/git-reset.txt ##
@@ Documentation/git-reset.txt: OPTIONS
Pathspec is passed in <file> instead of commandline args. If

## builtin/reset.c ##
+@@ builtin/reset.c: static int git_reset_config(const char *var, const char *value, void *cb)
+ int cmd_reset(int argc, const char **argv, const char *prefix)
+ {
+ int reset_type = NONE, update_ref_status = 0, quiet = 0;
+- int refresh = -1;
++ int refresh = 1;
+ int patch_mode = 0, pathspec_file_nul = 0, unborn;
+ const char *rev, *pathspec_from_file = NULL;
+ struct object_id oid;
@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH);
parse_args(&pathspec, argv, prefix, patch_mode, &rev);
2: d9bebd4b4e0 ! 2: a25aff3ac7c reset: deprecate 'reset.quiet' config option
@@ Metadata
Author: Victoria Dye <[email protected]>

## Commit message ##
- reset: deprecate 'reset.quiet' config option
+ reset: remove 'reset.quiet' config option

Remove the 'reset.quiet' config option, remove '--no-quiet' documentation in 'Documentation/git-reset.txt'. In 4c3abd0551 (reset: add new reset.quiet
3: ecd3296fd25 ! 3: 597aa82851c reset: deprecate 'reset.refresh' config option
@@ Metadata
Author: Victoria Dye <[email protected]>

## Commit message ##
- reset: deprecate 'reset.refresh' config option
+ reset: remove 'reset.refresh' config option

Remove the 'reset.refresh' option, requiring that users explicitly specify '--no-refresh' if they want to skip refreshing the index.
@@ Documentation/git-reset.txt: OPTIONS
- Proactively refresh the index after a mixed reset. If unspecified, the
- behavior falls back on the reset.refresh config option. If neither
- --[no-]refresh nor reset.refresh are set, refresh is enabled.
-+ Proactively refresh the index after a mixed reset. Enabled by default.
++ Refresh the index after a mixed reset. Enabled by default.

--pathspec-from-file=<file>::
Pathspec is passed in <file> instead of commandline args. If
4: dbb63c4caa8 < -: ----------- reset: deprecate '--refresh', leaving only '--no-refresh'

Signature · 1 line
gitgitgadget