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

Wed Jan 4 11:38:27 EST 2017

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.

>  (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