Re: [PATCH] stash: don't show irrelevant entry count in status

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> "Miroma via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Miroma <its.miroma@proton.me>
>>
>> Currently, when status.showStash is set, 'stash pop' shows the
>> following, confusing, output:
>>
>>     ...
>>     Your stash currently has 1 entry
>>     Dropped refs/stash@{0} (abc123...)
>>
>
> Right, so your proposal is to not print stash related status information
> when already running a stash command. It would be nice to note that
> here, along why you think so.
>
> Personally, I think it is important to keep this as is, because it tells
> the user the entries left in the stash post the stash operation.

When pop sees a conflict, it keeps the stash entry, and whe it does
not, it discards the stash entry it used just now.  So I agree with
you that the number of remaining entries may be something the user
would want to see, but in do_apply_stash(), I think we are getting
the number before popping, as dropping the entry seems to be the
responsibility of the caller of do_apply_stash(), if I am reading
pop_stash() correctly.  So, I suspect the current output may need
adjustment if we want to keep the message and want it to be useful.

>> @@ -705,6 +705,9 @@ restore_untracked:
>>  			     absolute_path(repo_get_work_tree(the_repository)));
>>  		strvec_pushf(&cp.env, GIT_DIR_ENVIRONMENT"=%s",
>>  			     absolute_path(repo_get_git_dir(the_repository)));
>> +		strvec_push(&cp.env, "GIT_CONFIG_COUNT=1");
>> +		strvec_push(&cp.env, "GIT_CONFIG_KEY_0=status.showStash");
>> +		strvec_push(&cp.env, "GIT_CONFIG_VALUE_0=false");
>>  		strvec_push(&cp.args, "status");
>>  		run_command(&cp);
>>
>
> So this block is run to print the status, unless the '--quiet' option is
> used. So it makes sense to do this here.

Does it?  What happens in existing GIT_CONFIG_{COUNT,KEY,VALUE}*
environment variables passed by the script that invoked us?

If we are spawning "git status" here, can't we do a much simpler and
more obvious thing, i.e. "git -c status.showstash=false status"?

> Small nit: Shouldn't we add a test to validate this change in behavior?

Yes.  I am not yet sure what the right output should be, though.