Re: [PATCH] rebase -i: restore autostash on abort

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On Tue, Jun 28, 2016 at 02:13:49PM -0700, Junio C Hamano wrote:
> 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.

> > 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]