Subject: Re: [PATCH] builtin/stash: configs keepIndex, includeUntracked

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Feb 2024 09:47:22 -0800

Phillip Wood <[email-masked]> writes:

> That's a good idea but I think it would be better to test that "git
> stash create" is not affected by the config as we don't want to change
> the behavior of its behavior.

Yup.  Making sure "stash create" stays the same is a very good
thing.  Thanks for suggesting it.

Regardling the need to have an escape hatch that is well publicized
long before a potentially harmful switch can safely happen, one way
out might be to

 - Declare "git stash create" is a plumbing, but the rest is
   considered Porcelain.  Publicize it well and wait for the word to
   spread out.  Folks who are in favor of adding these configuration
   variables to the system may need to do some legwork, spreading
   the word around at stackoverflow and various other venues,
   scouring public repositories at GitHub and other hosting sites
   and studying third-party uses of "git stash" that should be
   replaced with "git stash create" followed by "git update-ref",
   and raising issues to notify the owners of such projects, etc.

 - Add breaking configuration variables after a few years.

We cannot do the usual "start warning when we detect problematic use
without changing the current behaviour, wait, and then switch" dance
in this case, unfortunately, because we lack a good way to detect a
"problematic use".  We cannot tell a direct use of "git stash" via
the command prompt (which may want to honor the configuration) from
an IDE running "git stash" reacting to the "[Stash Now]" UI element
(which might want to honor the configuration) and from a third-party
tool that acts like "rebase --autostash" that wants to save away all
local changes to clear the slate to do its thing (which we should
definitely not interfare).  If we could do so reliably, then we
wouldn't be having this discussion---we'd be using the same logic as
we would use to tell when to warn and instead of warning, just silently
refraining to honor the configuration variables.

An eve easier way out of course is not to do these variables, of course.

FWIW, I can see how permanently enabling "include untracked" may be
OK in a workflow, but I cannot see the utility of permanently
enabling "keepindex" at all.  Sometimes I'd want to clear the slate
so that I can try building and testing what I added to the index and
that is why I want to use the option.  But a lot of other times, I
want to clear the slate to go back to the very pristine state that
is equal to the HEAD.  As the need to switch between the two modes
happens quite often, the way to defeat configured stash.keepindex
takes quite many keystrokes, and in general I think the regular
stashing happens far more often than keepindex stashing, I'd find
that using shorthand "-k" on the command line occasionally without
having this configuration variable would be what people would end up
using anyway.

And there is always "[alias] stk = stash -k" available ;-)