Re: [PATCH 5/5] stash: make internal resets quiet and refresh index

[Victoria Dye <vdye@github.com>]

Derrick Stolee wrote:
> On 3/11/2022 7:08 PM, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Add the options '-q' and '--refresh' to the 'git reset' executed in
>> 'reset_head()', and '--refresh' to the 'git reset -q' executed in
>> 'do_push_stash(...)'.
> 
>> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
>> index 5e68180f3b2..f2076545133 100755
>> --- a/t/t7102-reset.sh
>> +++ b/t/t7102-reset.sh
>> @@ -482,7 +482,7 @@ test_index_refreshed () {
>>  	git rm --cached file2 &&
>>  
>>  	# Step 2
>> -	git reset $1 --mixed HEAD &&
>> +	git reset $@ --mixed HEAD &&
> 
> I see you change this from "$1" to "$@", which won't help with
> the "-c key=value" issues from earlier.
> 
>>  
>>  	# Step 3
>>  	git read-tree -m HEAD~1
>> @@ -491,48 +491,66 @@ 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
> 
> Ah, I see you fixed this here, probably just a rebase issue, then.
> 

That's exactly what happened, and it will be fixed (and triple-checked) in
V2. :)

>>  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 &&
>> -
> 
> Ah, and the -c changes are removed here. You could still test them
> using the trick I mention in reply to patch 2.
> 
>>  	# 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
>> +	)
> 
> I'm not sure why you need sub-shells here. The test_config is
> not scoped to the shell. These lines could be avoided with the
> -c trick, which should make it a bit simpler to show what you
> intend to be testing here.
> 

This was mostly organizational, but I'll remove them entirely in favor of
the line config option in V2.

> Thanks,
> -Stolee