Re: [PATCH] builtin/stash: configs keepIndex, includeUntracked
- From
- Ricardo C <rpc01234@gmail.com>
- Date
- Feb 21, 2024, 19:13 UTC
- Message-ID
- <b84e77a1-93e4-4de0-a0cd-4d76a5691a9b@gmail.com>
- In-Reply-To
- <xmqqjzmzqb85.fsf@gitster.g>
On 2/20/24 12:47, Junio C Hamano wrote:
Show 8 quoted lines
> Phillip Wood <phillip.wood123@gmail.com> 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.
I agree. I'll make sure to add testing for it if/when we decide to go through with this change.
Show 15 quoted lines
> 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 year
This feels like quite a lot of work. I guess that the main question now is whether this change is worth the effort. What would publicizing this look like? Updating documentation? Sending out an announcement?
I can also imagine some tools would dislike having to switch to the more manual stash+update-ref, so maybe easier plumbing should be added?
Show 13 quoted lines
> 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.
Permanently enabling keepIndex is mainly intended for people that like to stash their unstaged changes before committing (e.g., for testing them independently of other changes). The main issue with what you recommend is that, if they forget to use `-k`, then the entire state of the index is lost, which is especially problematic if the changes were interactively staged. A report of someone having issues with this workflow is here[1].
Perhaps a better solution would be to provide some mechanism by which to also save the state of the index when stashing changes, and also restore it when applying the stash. I figure this change in behavior would be much less problematic.
Another option would be to go closer to the route of `git switch` and `git restore`, where a separate command with a more user-friendly interface is added. Then all of `git stash` could be kept as plumbing, and this new command would be entirely porcelain. I don't think that this change would be worth introducing a new command for, but if there are other changes that could also be included, then it might make sense.
> And there is always "[alias] stk = stash -k" available ;-)
Yeah, currently I use a shell alias for `git stash -ku`, but that doesn't help when I'm using other git frontends.
[1] https://bsd.network/@ed1conf/111783574839749798