> > + index_changed = repo_index_has_changes(the_repository, NULL, NULL);
> > + if (allow_empty &&
> > + !(!index_changed && is_empty_or_missing_file(am_path(state, "patch"))))
> > die_user_resolve(state);
>
> Why do we need this?
>
> Intuitively, because "--allow-XYZ" is "we normally die when the
> condition XYZ holds, but with the option, we do not die in such a
> case", any new condition that leads to "die" that only kicks in
> when "--allow-XYZ" is given smell like a BUG.
>
> The condition reads:
>
> unless we saw an empty patch (i.e. "patch" file is missing or
> empty, and did not result in any change to the index), it is an
> error to give "--allow-empty" to the command.
>
> That somehow does not make any sense to me. Whether failed patch
> application and manual tweaking resulted in the same tree as HEAD,
> or a tree different from HEAD, if the user wants to say "I allow
> Git to create an empty commit as necessary, instead of stopping",
> shouldn't the user be allowed to? After all, the option is not
> "create nothing but an empty commit, it is an error if there is
> something to commit." It merely allows creation of an empty one.
>
> This series has been trying to add code to punish users who give
> "--allow-empty" when the command would have worked the same way
> without it at least since the last round, and I am not sure where
> that wish to stop users comes from. Please explain. I am starting
> to think I may be missing something and this extra rigidity may be
> helping a user (but again, I do not see how).
If there is no such code to check, the user can use "--allow-empty" to do the same thing
as "--continue" or "--resolved" after resolving conflicts because they all go into `am_resolve()`.
In my opinion, "--allow-empty" only makes sense to allow the user to create empty commits when:
1. the result is an empty change (`!index_changed`)
2. the input is an empty change (`is_empty_or_missing_file(am_path(state, "patch"))`)
> > + if (!index_changed) {
> > + if (allow_empty) {
>
> The index_changed variable being false is "the result is an empty
> change", and is-empty-or-missing on "patch" file is "the input is an
> empty change". Ideally we would want to create an empty commit only
> when both input and result are empty, but in order to prevent mistakes,
> we may want to react to the case where the result is empty but the
> input is not empty.
>
> Here is where we can use is-empty-or-missing on "patch" and give a
> different message (i.e. "even though the patch file is not empty,
> the result of your patch application did not change anything"), if
> we wanted to. Or you could choose to reject such an attempt, which
> might be simpler. I.e.
>
> if (allow_empty &&
> is_empty_or_missing_file(am_path(state, "patch"))
> "No changes - recorded an empty commit."
> else
> ... the original error for no changes ...
>
>
> Hmm?
Based on the previous checking, there is no need to check
`is_empty_or_missing_file(am_path(state, "patch"))` here because it is permanently true.
