Re: [PATCH] rebase -i: restore autostash on abort
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 29, 2016, 06:28 UTC
- Message-ID
- <20160629062818.GA416@pks-pc.localdomain>
- In-Reply-To
- <xmqqshvx6mpe.fsf@gitster.mtv.corp.google.com>
On Tue, Jun 28, 2016 at 02:13:49PM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes: > > > It is "interesting" if you mean "matches real-life use-case", as it > > corresponds to the case where the user killed the editor (as reported by > > Daniel Hahler indeed, "Abort with ":cq", which will make Vim exit > > non-zero"). > > Yes. It is an interesting failure mode in that sense. But breakage > of such a basic mode is something an end-user is likely to notice > immediately, so in that sense, having such a test alone is not all > that interesting.
Well, given that the bug has been lingering since autostashing has been implemented it seems users didn't catch the breakage as fast ;) I guess it's just a little-used feature _or_ the breakage is too obscure to regularly happen. At least I have never cause my editor to return an error when editing the ISN-sheet.
But still, I agree that a test for conflicting autostashes is beneficial, even though the scenario is even more unlikely (the user's editor has to return an error code as well as that a stashed file needs to be modified while editing the ISN-sheet). I've just sent out a second version of the patch.
Thank you both for your input.
Show 6 quoted lines
> > If you mean "likely to trigger nasty bugs", then indeed testing the case > > when apply_autostash fails is interesting: for example, calling > > die_abort when "stash apply" fails is tempting, but would lead to > > infinite recursion (it doesn't seem to be the case, but a test would be > > nice). Setting the editor to something that modifies uncommited-content > > before 'false' should do the trick.