On Tue, Jun 28, 2016 at 02:13:49PM -0700, Junio C Hamano wrote: > Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> 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:
signature.asc
Description: PGP signature