On 01/04/2017 05:38 PM, Martin von Zweigbergk wrote:
On Wed, Jan 4, 2017 at 4:53 AM, Pierre-Yves David
<pierre-yves.da...@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.da...@ens-lyon.org <mailto:pierre-yves.da...@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 <martinv...@google.com
    <mailto:martinv...@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.

The "specify multiple revs" approach "works" but is quickly cumbersome and error prone if you have more than a couple changesets.

There is currently two others tools that someone can use to fold changesets:
 * hg rebase --collapse
 * hg histedit
Both provide the user with a simple way to fold many changesets (eg: by providing a root). So a kind of bottom line here is that it would be really awkward if the command dedicated to folding changeset end up being less user friendly than command dedicated to other usecase (that happen to be able to fold things as a secondary feature).

This made me implicitly discard the option to "list every single changeset to be folded" from the "viable UI" list. Thanks for pointing this out. It gave me the chance to unearth that extra criteria: "be a better UI than the existing solution".

What do you think ?


(note: a third way would be 'hg revert -r X; hg commit' and also provides a simple mean to select a range)



 (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

--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to