Aleen 徐沛文 <pwxu@xxxxxxxxxxx> writes: >> 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()`. Yes, exactly. That is the point of "allow"; we continue normally, and even in a narrow situation (i.e. input is empty, and no change is made to the index) where we normally stop, we allow it to continue by creating an empty commit. > 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"))`) Then you are forcing the user to use it ONLY whe nhe input is empty and no other time. Whatever that is, that is not "allow" 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. I think you misunderstood. If we correct the overly strict check that makes "--allow-empty" to mean "only create empty, anything else is an error", the "is patch an empty file?" must be checked here.
