Dears Hamano,
I have doubted that since that the default behaviour is that stopping
when meeting commit messages lacking a patch and giving control back
to the user, is that necessary to provide duplicated '--empty=die'?
Should we just provide '--empty=(drop|keep)'?
Aleen
> Sorry, but I am afraid that I still don't get it.
>
> As we can see, the ERR_EMPTY_COMMIT case already exists and that is
> the behaviour we want when we say "create commits from the messages
> with patches, stop and give me control back when you see an empty
> commit, so that I can decide what to do". And one of the things you
> can do at that point is "am --abort" that causes the am_destroy() to
> be called.
>
> That is very much in line with the behaviour the users are used to
> see from "git am" when it detects conditions other than "there is no
> patch" that needs to stop and give control back to the user,
> e.g. when the patch does not apply cleanly or the log message did
> not pass msg hook. am_destroy() is destructive, and limiting such a
> destructive operation to "am --abort" would avoid mistakes and
> surprises. I do not know where you got "In common cases, die should
> stop the whole" from, but it is not a friendly thing to do to our
> users.
>
> Why should the "in addition, stop when there is no patch", which is
> what we already handle just fine, needs to become different? More
> importantly, is it worth forcing the users to be aware of the
> difference and be extra careful to avoid it?
>
> It is my understanding that the ONLY reason the patch proposes to
> add an option other than "skip the step" and "create an empty
> commit" is to allow an earlier "--empty=skip" on the command line to
> be overridden by a later "--empty=die". If that option does not
> make the command behave identically to "--empty=<anything>" option
> on the command line, i.e. ERR_EMPTY_COMMIT case, it does not serve
> the intended purpose of overriding the earlier option to revert the
> behaviour back to the default.
>
> By the way, I agree with an earlier comment (I think it was from Dscho)
> that these names should say "${DO_THIS}_ON_EMPTY_COMMIT"; the above
> without "ON" feels somewhat strange.
>
> Thanks.
