[PATCH] commit: add --allow-empty flag

[Durham Goode]

On 5/8/15 12:30 PM, Pierre-Yves David wrote:
>
>
> On 05/08/2015 11:45 AM, Ryan McElroy wrote:
>> On 5/8/2015 10:41 AM, Pierre-Yves David wrote:
>>>
>>>
>>> On 05/07/2015 03:10 PM, Durham Goode wrote:
>>>> # HG changeset patch
>>>> # User Durham Goode <durham at fb.com>
>>>> # Date 1431034538 25200
>>>> #      Thu May 07 14:35:38 2015 -0700
>>>> # Node ID 9f90e53f8e810db2d6b0e4cf66433166c4d42934
>>>> # Parent  c5d4f9cc8da7bb2068457e96e4f74ff694514ced
>>>> commit: add --allow-empty flag
>>>
>>> I'm -1 on adding the new flag and +1 on the ability in core.
>>>
>>> Given that apparently multiple users exists for that and that
>>> conversion from other VCS requires it, it makes sense to make it
>>> somewhat available.
>>>
>>> However it is still far too much a corner case to be a first class
>>> citizen. Moreover, the most probable usage of that are automation who
>>> do not really care about the extra hurdle of going through something
>>> more complicated.
>>> Commit is an important user facing command and its option tend to
>>> trickle down to a lot of other commands. In my opinion. This feature
>>> is not generic/useful for normal user enough to justify a dedicated 
>>> flag.
>>>
>>> I would go for a config option probably in the [ui] section.
>>>
>>> As Augie mentioned, we also have to make sure
>>> rebase/graft/histedit/etc preserve them.
>>>
>>>
>> I don't like the idea of having it as a config option, and [ui] doesn't
>> seem like that right place for it anyway. Reasons against a config
>> option: as a user, this is something that you want to enable on a
>> per-command basis. Having it as a ui option implies stickiness is
>> desired, but that is generally not the case here (in my experience using
>> this functionality in git -- which I used
>> not-as-rarely-as-you-would-think).
>
> Okay, this is new informations. The only creators of such changesets 
> in usecase have heard about so far are automated. But you seems to 
> implies normal user may want to do so. What is your human usecase for 
> it and why is it so frequent ?
If the problem is UI clutter, then we should solve that problem (put it 
behind -v).  If the feature is hidden down in some config option, we 
might as well not introduce it to core since it will never be discovered 
by anyone except people who see this thread.
>
>> The preservation of empty commits after rebases seems like an
>> interesting concern, and it's absolutely unclear to me what the right
>> choice there is. What are the arguments towards changing the current
>> behavior? I think it would be more confusing to have some commits that
>> stick around when empty after rebase and others that don't.
>
> There is a clear way to go here. Rebase drop changesets -emptied- by 
> the merge. IF there changesets was empty before the rebase, it must be 
> preserved.
If everyone feels rebase and histedit support are a must before this 
feature can be accepted, I'll probably just drop the feature and do it 
as an internal extension.  The additional logic to make rebase and 
histedit work will likely be more complicated than the actual feature is 
and will likely cause more bugs than adding the feature solves.

I don't think rebase and histedit support are required.  If a user 
opts-in to making an empty commit, I think they can expect a few 
oddities with future commands.
>
>> I guess this is all a long way of saying that I don't think we should
>> block on rebase/histedit supporting this because it's non-obvious that
>> they should. And I think the feature should be a flag, perhaps hidden
>> from default help (I don't care about that so much right now).
>
> If we easily allow empty changeset but silently drop them whenever 
> history is rewritten I expect angry user mob on your door step by the 
> end of the month.
I think the mob would be smaller than the one clambering for 
--allow-empty (that is to say, negligible).