From: Phillip Wood Date: Tue, 20 Feb 2024 11:01:30 GMT Subject: Re: [PATCH] builtin/stash: configs keepIndex, includeUntracked Message-ID: <78a8733b-c74a-4017-8905-d29b2e05adb1@gmail.com> In-Reply-To: Hi Ricardo On 18/02/2024 17:54, Ricardo C wrote: > Hi Phillip, > > On 2/18/24 05:32, Phillip Wood wrote: >> How does "stash.keepIndex" interact with "git rebase --autostash" and >> "git merge --autostash"? I think both those commands expect a clean >> index after running "git stash". They could just override the config >> setting but it might get a bit confusing if some commands respect the >> config and others don't. > > Both `git rebase --autostash` and `git merge --autostash` seem to be > hardcoded to clean the index, regardless of the configuration or CLI > flags. They do not use regular `git stash` to do so, but rather `git > stash create`. This is unaffected by my changes, since it follows a > different code path and does not accept `--keep-index` nor > `--include-untracked`. Ah, I'd forgotten that we hand "git stash create" as well as "git stash push". It's good that these changes do not affect "git stash create" but I think Junio is right to be concerned about the effect on tools that are using "git stash push" where they arguably ought to be using "git stash create". > I'll add some tests for `git rebase --autostash` > and `git merge --autostash`, just in case this behavior is changed in > the future and causes breakage. 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. >> I've only given the patch a very quick scan, but it looked sensible. >> The only thing that jumped out at me was that quite a few tests seem >> to do >> >>      git init repo && >>      ( >>          cd repo && >>          # test things >>      ) && >> >> Our normal practice is to run all the tests in the same file in the >> same repository rather than setting up a new one each time. > > I was doing this because it makes comparing different commands easier, > but looking through other tests again, it seems like I should be > comparing the outputs to hardcoded files anyway. Yes, that is our usual practice. Best Wishes Phillip