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

[Pierre-Yves David]

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