[PATCH v2 0/5] Allow 'reset --quiet' to refresh the index, use 'reset --quiet' in 'stash'

["Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>]

In the process of working on tests for 'git stash' sparse index integration,
I found that the '--quiet' option in 'git stash' does not suppress all
non-error output when used with '--index'. Specifically, this comes from an
invocation of 'git reset' without the '--quiet' flag in 'reset_head()'. Upon
enabling that flag, though, I discovered that 1) 'reset' does not refresh
the index if '--quiet' is specified (as of [1]) and 2) 'git stash' needs the
index to be refreshed after the reset.

This series aims to decouple the "suppress logging" and "skip index refresh"
behaviors in 'git reset --mixed', then allow 'stash' to internally use reset
with logs suppressed but index refresh enabled. This is accomplished by
introducing the '--[no-]refresh' option and 'reset.refresh' config setting
to 'git reset'. Additionally, in the spirit of backward-compatibility,
'--quiet' and/or 'reset.quiet=true' without any specified "refresh"
option/config will continue to skip 'refresh_index(...)'.

There are also some minor updates to the advice that suggests skipping the
index refresh:

 * replace recommendation to use "--quiet" with "--no-refresh"
 * use 'advise()' rather than 'printf()'
 * rename the advice config setting from 'advice.resetQuiet' to to
   'advice.resetNoRefresh'
 * suppress advice if '--quiet' is specified in 'reset'

Finally, tests are added to 't7102-reset.sh' verifying whether index refresh
happens when expected and to 't3903-stash.sh' verifying that 'apply --quiet'
no longer prints extraneous logs.


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

 * Update cover letter title
 * Squash 't7102' test fixes from patch 5 into patch 2
 * Refactor 't7102' tests to properly use inline config settings

[1] https://lore.kernel.org/git/20181023190423.5772-2-peartben@gmail.com/

Thanks! -Victoria

Victoria Dye (5):
  reset: revise index refresh advice
  reset: introduce --[no-]refresh option to --mixed
  reset: replace '--quiet' with '--no-refresh' in performance advice
  reset: suppress '--no-refresh' advice if logging is silenced
  stash: make internal resets quiet and refresh index

 Documentation/config/advice.txt |  8 ++--
 Documentation/git-reset.txt     |  9 ++++
 advice.c                        |  2 +-
 advice.h                        |  2 +-
 builtin/reset.c                 | 21 ++++++---
 builtin/stash.c                 |  5 ++-
 t/t3903-stash.sh                | 12 +++++
 t/t7102-reset.sh                | 77 ++++++++++++++++++++++++++++++---
 8 files changed, 116 insertions(+), 20 deletions(-)


base-commit: 1a4874565fa3b6668042216189551b98b4dc0b1b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1170%2Fvdye%2Fstash%2Fmake-reset-quiet-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1170/vdye/stash/make-reset-quiet-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1170

Range-diff vs v1:

 1:  7206ef8dd8a = 1:  7206ef8dd8a reset: revise index refresh advice
 2:  bda93703013 ! 2:  7f0226bc3e6 reset: introduce --[no-]refresh option to --mixed
     @@ Commit message
          Add a new --[no-]refresh option that is intended to explicitly determine
          whether a mixed reset should end in an index refresh.
      
     -    A few years ago, [1] introduced behavior to the '--quiet' option to skip the
     -    call to 'refresh_index(...)' at the end of a mixed reset with the goal of
     -    improving performance. However, by coupling behavior that modifies the index
     -    with the option that silences logs, there is no way for users to have one
     -    without the other (i.e., silenced logs with a refreshed index) without
     +    Starting at 9ac8125d1a (reset: don't compute unstaged changes after reset
     +    when --quiet, 2018-10-23), using the '--quiet' option results in skipping
     +    the call to 'refresh_index(...)' at the end of a mixed reset with the goal
     +    of improving performance. However, by coupling behavior that modifies the
     +    index with the option that silences logs, there is no way for users to have
     +    one without the other (i.e., silenced logs with a refreshed index) without
          incurring the overhead of a separate call to 'git update-index --refresh'.
          Furthermore, there is minimal user-facing documentation indicating that
          --quiet skips the index refresh, potentially leading to unexpected issues
     @@ Commit message
          either is set, '--quiet' and 'reset.quiet' revert to controlling only
          whether logs are silenced and do not affect index refresh.
      
     -    [1] 9ac8125d1a (reset: don't compute unstaged changes after reset when
     -        --quiet, 2018-10-23)
     -
     +    Helped-by: Derrick Stolee <derrickstolee@github.com>
          Signed-off-by: Victoria Dye <vdye@github.com>
      
       ## Documentation/git-reset.txt ##
     @@ t/t7102-reset.sh: test_expect_success 'resetting an unmodified path is a no-op'
      +	git rm --cached file2 &&
      +
      +	# Step 2
     -+	git reset $1 --mixed HEAD &&
     ++	git $1 reset $2 --mixed HEAD &&
      +
      +	# Step 3
      +	git read-tree -m HEAD~1
     @@ t/t7102-reset.sh: test_expect_success 'resetting an unmodified path is a no-op'
      -	test_cmp expect output
      +	# Verify default behavior (with no config settings or command line
      +	# options)
     -+	test_index_refreshed &&
     ++	test_index_refreshed
      +'
      +test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '
      +	# Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)
      +	# determine refresh behavior
      +
      +	# Config setting
     -+	test_must_fail test_index_refreshed -c reset.quiet=true &&
     -+	test_index_refreshed -c reset.quiet=true &&
     ++	! test_index_refreshed "-c reset.quiet=true" &&
     ++	test_index_refreshed "-c reset.quiet=false" &&
      +
      +	# Command line option
     -+	test_must_fail test_index_refreshed --quiet &&
     -+	test_index_refreshed --no-quiet &&
     ++	! test_index_refreshed "" --quiet &&
     ++	test_index_refreshed "" --no-quiet &&
      +
      +	# Command line option overrides config setting
     -+	test_must_fail test_index_refreshed -c reset.quiet=false --quiet &&
     -+	test_index_refreshed -c reset.refresh=true --no-quiet
     ++	! test_index_refreshed "-c reset.quiet=false" --quiet &&
     ++	test_index_refreshed "-c reset.refresh=true" --no-quiet
      +'
      +
      +test_expect_success '--mixed --[no-]refresh sets refresh behavior' '
      +	# Verify that --[no-]refresh and `reset.refresh` control index refresh
      +
      +	# Config setting
     -+	test_index_refreshed -c reset.refresh=true &&
     -+	test_must_fail test_index_refreshed -c reset.refresh=false &&
     ++	test_index_refreshed "-c reset.refresh=true" &&
     ++	! test_index_refreshed "-c reset.refresh=false" &&
      +
      +	# Command line option
     -+	test_index_refreshed --refresh &&
     -+	test_must_fail test_index_refreshed --no-refresh &&
     ++	test_index_refreshed "" --refresh &&
     ++	! test_index_refreshed "" --no-refresh &&
      +
      +	# Command line option overrides config setting
     -+	test_index_refreshed -c reset.refresh=false --refresh &&
     -+	test_must_fail test_index_refreshed -c reset.refresh=true --no-refresh
     ++	test_index_refreshed "-c reset.refresh=false" --refresh &&
     ++	! test_index_refreshed "-c reset.refresh=true" --no-refresh
      +'
      +
      +test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '
      +	# Verify that *both* --refresh and `reset.refresh` override the
      +	# default non-refresh behavior of --quiet
     -+	test_index_refreshed --refresh --quiet &&
     -+	test_index_refreshed --refresh -c reset.quiet=true &&
     -+	test_index_refreshed -c reset.refresh=true --quiet &&
     -+	test_index_refreshed -c reset.refresh=true -c reset.quiet=true
     ++	test_index_refreshed "" "--quiet --refresh" &&
     ++	test_index_refreshed "-c reset.quiet=true" --refresh &&
     ++	test_index_refreshed "-c reset.refresh=true" --quiet &&
     ++	test_index_refreshed "-c reset.refresh=true -c reset.quiet=true"
       '
       
       test_expect_success '--mixed preserves skip-worktree' '
 3:  63c5ee36feb = 3:  f869723d64f reset: replace '--quiet' with '--no-refresh' in performance advice
 4:  3c65a9f1993 = 4:  cffca0ea5c6 reset: suppress '--no-refresh' advice if logging is silenced
 5:  052499bbc93 ! 5:  3334d4cb6f3 stash: make internal resets quiet and refresh index
     @@ t/t3903-stash.sh: test_expect_success 'apply -q is quiet' '
       test_expect_success 'save -q is quiet' '
       	git stash save --quiet >output.out 2>&1 &&
       	test_must_be_empty output.out
     -
     - ## t/t7102-reset.sh ##
     -@@ t/t7102-reset.sh: test_index_refreshed () {
     - 	git rm --cached file2 &&
     - 
     - 	# Step 2
     --	git reset $1 --mixed HEAD &&
     -+	git reset $@ --mixed HEAD &&
     - 
     - 	# Step 3
     - 	git read-tree -m HEAD~1
     -@@ t/t7102-reset.sh: test_index_refreshed () {
     - test_expect_success '--mixed refreshes the index' '
     - 	# Verify default behavior (with no config settings or command line
     - 	# options)
     --	test_index_refreshed &&
     -+	test_index_refreshed
     - '
     - test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '
     - 	# Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)
     - 	# determine refresh behavior
     - 
     --	# Config setting
     --	test_must_fail test_index_refreshed -c reset.quiet=true &&
     --	test_index_refreshed -c reset.quiet=true &&
     --
     - 	# Command line option
     --	test_must_fail test_index_refreshed --quiet &&
     -+	! test_index_refreshed --quiet &&
     - 	test_index_refreshed --no-quiet &&
     - 
     --	# Command line option overrides config setting
     --	test_must_fail test_index_refreshed -c reset.quiet=false --quiet &&
     --	test_index_refreshed -c reset.refresh=true --no-quiet
     -+	# Config: reset.quiet=false
     -+	test_config reset.quiet false &&
     -+	(
     -+		test_index_refreshed &&
     -+		! test_index_refreshed --quiet
     -+	) &&
     -+
     -+	# Config: reset.quiet=true
     -+	test_config reset.quiet true &&
     -+	(
     -+		! test_index_refreshed &&
     -+		test_index_refreshed --no-quiet
     -+	)
     - '
     - 
     - test_expect_success '--mixed --[no-]refresh sets refresh behavior' '
     - 	# Verify that --[no-]refresh and `reset.refresh` control index refresh
     - 
     --	# Config setting
     --	test_index_refreshed -c reset.refresh=true &&
     --	test_must_fail test_index_refreshed -c reset.refresh=false &&
     --
     - 	# Command line option
     - 	test_index_refreshed --refresh &&
     --	test_must_fail test_index_refreshed --no-refresh &&
     -+	! test_index_refreshed --no-refresh &&
     -+
     -+	# Config: reset.refresh=false
     -+	test_config reset.refresh false &&
     -+	(
     -+		! test_index_refreshed &&
     -+		test_index_refreshed --refresh
     -+	) &&
     - 
     --	# Command line option overrides config setting
     --	test_index_refreshed -c reset.refresh=false --refresh &&
     --	test_must_fail test_index_refreshed -c reset.refresh=true --no-refresh
     -+	# Config: reset.refresh=true
     -+	test_config reset.refresh true &&
     -+	(
     -+		test_index_refreshed &&
     -+		! test_index_refreshed --no-refresh
     -+	)
     - '
     - 
     - test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '
     - 	# Verify that *both* --refresh and `reset.refresh` override the
     - 	# default non-refresh behavior of --quiet
     -+
     - 	test_index_refreshed --refresh --quiet &&
     --	test_index_refreshed --refresh -c reset.quiet=true &&
     --	test_index_refreshed -c reset.refresh=true --quiet &&
     --	test_index_refreshed -c reset.refresh=true -c reset.quiet=true
     -+
     -+	# Config: reset.quiet=true
     -+	test_config reset.quiet true &&
     -+	test_index_refreshed --refresh &&
     -+
     -+	# Config: reset.quiet=true, reset.refresh=true
     -+	test_config reset.refresh true &&
     -+	test_index_refreshed
     - '
     - 
     - test_expect_success '--mixed preserves skip-worktree' '

-- 
gitgitgadget