[PATCH evolve-ext] fold: disallow multiple revisions without --exact

Wed Jan 4 11:45:53 EST 2017

On Wed, Jan 4, 2017 at 8:38 AM, Martin von Zweigbergk
<martinvonz at google.com> wrote:
> On Wed, Jan 4, 2017 at 4:53 AM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>>
>>
>> On 12/18/2016 06:36 PM, Martin von Zweigbergk via Mercurial-devel wrote:
>>>
>>>
>>>
>>> On Fri, Dec 16, 2016, 23:53 Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
>>> wrote:
>>>
>>>
>>>
>>>     On 11/05/2016 12:58 AM, Martin von Zweigbergk via Mercurial-devel
>>> wrote:
>>>     > # HG changeset patch
>>>     > # User Martin von Zweigbergk <martinvonz at google.com
>>>     <mailto:martinvonz at google.com>>
>>>     > # Date 1478303512 25200
>>>     > #      Fri Nov 04 16:51:52 2016 -0700
>>>     > # Node ID bb80851fe9a6e14263f0076074108556377141f9
>>>     > # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>>>     > fold: disallow multiple revisions without --exact
>>>     >
>>>     > It's very easy to think that "hg fold 4::6" will fold exactly those
>>>     > revisions. In reality, it will fold those *and* any revisions
>>> between
>>>     > them and the working copy. It seems very likely that users who pass
>>>     > more than one revision wants to fold exactly those revisions, so
>>> let's
>>>     > abort and hint that they may be looking for --exact.
>>>
>>>     That seems a good first step to take in all cases. Unfortunately, the
>>>     patch fails to apply for an unclear reason. Can you send me a new
>>>     version?
>>>
>>>     In addition, I suggest the following changes:
>>>
>>>     - we should allow multiple revision if '.' is one of them. They will
>>> be
>>>     no surprise in this case.
>>>
>>>
>>> Even if there is a gap between the revisions and '.'?
>>
>>> I wonder if we should fail only if the given commits form a
>>> contiguous range (I assume that's required for --exact).
>>
>> Non-continuous set are currently disallowed, I'm not suggesting we change
>> that. The current behavior without --exact create continuous range
>> "automatically" from the way it compute the fold-set.
>>
>> (note that supporting non-contiguous range is something we might do in the
>> future, but that's another topic)
>>
>> [reworked the quoted part a bit for clarity]
>>>
>>> So
>>
>>> "hg fold .^^" would work because it is a single revision.
>>> "hg fold .^^ .^" would fail because it's multiple revisions.
>>> "hg fold .^ ." would work because it includes '.'.
>>
>> So far, this is what I was suggesting in my previous email.
>> Following your "contiguous" point I would say that
>>
>> "hg fold .+.~1+.~3" would fail because the set is non contiguous
>>
>>> "hg fold .^ .^2" works because it's not a contiguous range (without the
>>> implicit
>>> '.'). It does feel like a little too much logic too the degree that it
>>> may surprise users, but I think the behavior without it may surprise
>>> users even more.
>>
>>
>> I do not understand what you mean with your last example. According to my
>> previous proposal. That would fail because it is multiple revision without
>> ".".
>>
>> So I'm a bit confused about what you tried to says here. That help making a
>> point about user confusion. We might need to take a step back and think a
>> bit more about what we building here.
>>
>>>     - update to the commit message,
>>>
>>>     abort: multiple revisions specified
>>>     (do you want --exact?, see "hg help fold" for details)
>>>
>>>
>>>     (I'll tell more about possible changes to the default in the rest of
>>> the
>>>     thread)
>>>
>>> Having said the above, I'm more in favor of making --exact the default
>>> and not needing the protection mentioned above, so I'm looking forward
>>> to hearing what you have to say here.
>>
>>
>> There are two extra important points that lead to the current UI choice:
>>
>> (1) revset is an advanced feature, its knowledge should not be required for
>> using a command.
>>     Revset are a very cool feature and all developers on this list are
>> pretty familiar with its power. However, many Mercurial users in the real
>> world have never needed revset and survive pretty well without it. We have
>> to build a simple UI for simple people first, and then make it able to fit
>> the more advance usecase of more advance people.
>>
>> (2) Many Mercurial command are working copy (or working copy parent) centric
>> by default (eg: diff). Especially when it comes to history editing (eg:
>> rebase, histedit, amend, split).The common case is usually to do something
>> with the current working context of the user. That's why it is usually used
>> for the default action. It would be nice to keep 'hg fold' consistent with
>> these other commands.
>> (note: there is also commands with full repo approach, like `hg push` and
>> `hg log`… and complains about them not being more working copy centric)
>>
>>
>>
>> There is a couple of other things to take in account:
>>
>> (3) I stay convinced that the common use-case will we working copy centric
>> (with ancestors or with descendant). Evolution help stack centric workflow
>> and improve our ability to works within that stack. In that context, fold
>> working from the working copy, take advantage of that "in-stack" workflow
>> and also helps reinforce it in a consistent way.
>>
>> (4) On the other hand I understand that their have also been people
>> surprised and bitten by the current behavior and I agree we must fix that.
>>
>> ---
>>
>> Currently I would weight "constraint" that way
>> (more important to least important):
>>
>>  (1) no revset knowledge required:
>>          simplicity is very important,
>
> That makes sense, but note that "hg fold -r foo -r bar" is already
> supported and does not require the user to know about revsets (and one
> can also drop the "-r"s in that command). I feel like you're assuming
> that defaulting to --exact requires the user to know about revsets. I
> don't see it that way.

Oh, I should clarify that my point here is only what I said: that I
don't think that using --exact requires revset knowledge.
Specifically, I'm not implying that that means we have to make --exact
the default. I'm still debating that in my mind.

>
>>  (4) stop bitting users:
>>          people loosing trust in their tool is -bad-,
>>  (2) consistency with other command:
>>          consistency is important in the great scheme of things,
>>  (3) common case is "." related:
>>          having good default is great, but should not get in the way of
>>          more important things.
>>
>> The current behavior is too bad in regard with (2), there is regular report
>> of people being bitten by that. So we need changes it.
>>
>> In my opinion, the proposal of using --exact as the default is incompatible
>> with (1) because defining the exact set requires people to learn some of
>> revset (and with that, some will end up using the cursed "x:y" that will
>> pretend to do what they want until it won't).
>>
>> My proposal to move forward was to keep the current behavior, but be more
>> strick regarding the input it accept to mitigate the risk of (2).
>>
>> Here is a table is a basic summary table:
>>
>>                    | (1) | (2) | (3) | (4) |
>> current            |  x  |  x  |  x  |     |
>> exact as default   |     |     |     |  x  |
>> current restricted |  x  |  x  |  x  |  x  |
>>
>>
>> As you pointed out the weak point of "current restricted" is probably the
>> "magic" or "confusing logic" of what is allowed or not. (This is not covered
>> in the summary table).
>>
>> If someone want to work within the constraint space and come up with a
>> better proposal I would be very happy to discuss it (reminder: direct use of
>> "--exact" as the current default seems off table because of the (1)
>> restriction (should not requires revset knowledge)).
>>
>> Cheers,
>>
>> --
>> Pierre-Yves David