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

login
register
mail settings
Submitter via Mercurial-devel
Date Nov. 4, 2016, 11:58 p.m.
Message ID <bb80851fe9a6e14263f0.1478303904@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/17342/
State Changes Requested
Headers show

Comments

via Mercurial-devel - Nov. 4, 2016, 11:58 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@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.
Kyle Lippincott - Nov. 5, 2016, 1:09 a.m.
On Fri, Nov 4, 2016 at 4:58 PM, Martin von Zweigbergk via Mercurial-devel <
mercurial-devel@mercurial-scm.org> wrote:

> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1478303512 25200
> #      Fri Nov 04 16:51:52 2016 -0700
> # Node ID bb80851fe9a6e14263f0076074108556377141f9
> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
> fold: disallow multiple revisions without --exact
>

Per conversation on IRC, I think that the opposite would be safer: disallow
single revisions without explicit user consent (i.e. return to the original
behavior, according to https://www.mercurial-scm.org/wiki/EvolveUI#Why.3F).
The situation I'm afraid of with this change is something like this (sorry
for the mild hypotheticals involved here):

    If we assume a series of draft changes with A(t1), B(t1), C(t2), D(t3),
E(t3,@),
    then a hypothetical `hg fold topic(t1)` would error, but `hg fold
topic(t2)` would fold C,D,E.

I had been thinking that `hg fold .^` would be the primary reason for the
current non-exact (so I proposed `hg fold --prev` as equivalent to `hg fold
.^::.`), but https://www.mercurial-scm.org/wiki/EvolveUI#Why.3F says it's
really more like "most-recent N commits".  I think requiring the user to
count would not be ideal and it causes confusion with revision numbers, so
I don't think that --prev would work with an argument.

Instead, if we assume that most people stay tip-most in their work in
progress chains, we could have a `hg fold --from X` as an "alias" for `hg
fold X::.`, except that for safety reasons it should probably check that
max(X) < .

With that, merging the wip is, usually, `hg fold --from <nodeid of base of
wip>`.  I think --to would be unnecessary - the number of times it's useful
would be much lower (again, if we assume most non-power-users stay
tip-most, and power-users can be expected to use revsets properly).





>
> 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.
>
> diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
> --- a/hgext/evolve.py   Wed Nov 02 18:56:44 2016 +0100
> +++ b/hgext/evolve.py   Fri Nov 04 16:51:52 2016 -0700
> @@ -3115,6 +3115,11 @@
>      revs = scmutil.revrange(repo, revs)
>
>      if not opts['exact']:
> +        if len(revs) > 1:
> +            raise error.Abort(_("cannot fold from working directory to "
> +                                "more than one revision"),
> +                              hint=_("did you mean to use --exact?"))
> +
>          # Try to extend given revision starting from the working directory
>          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
>          discardedrevs = [r for r in revs if r not in extrevs]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
> --- a/tests/test-evolve.t       Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-evolve.t       Fri Nov 04 16:51:52 2016 -0700
> @@ -688,7 +688,11 @@
>    $ hg fold -r 4 -r 6 --exact
>    abort: cannot fold non-linear revisions (multiple roots given)
>    [255]
> -  $ hg fold 10 1
> +  $ hg fold 4::5
> +  abort: cannot fold from working directory to more than one revision
> +  (did you mean to use --exact?)
> +  [255]
> +  $ hg fold 1
>    abort: cannot fold non-linear revisions
>    (given revisions are unrelated to parent of working directory)
>    [255]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
> --- a/tests/test-userguide.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-userguide.t    Fri Nov 04 16:51:52 2016 -0700
> @@ -109,7 +109,7 @@
>    7:05e61aab8294  step 1
>    8:be6d5bc8e4cc  step 2
>    9:35f432d9f7c1  step 3
> -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
> +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
>    3 changesets folded
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ hg --hidden shortlog -G -r 6::
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
via Mercurial-devel - Nov. 19, 2016, 7:30 p.m.
On Fri, Nov 4, 2016 at 6:09 PM, Kyle Lippincott <spectral@pewpew.net> wrote:
>
>
> On Fri, Nov 4, 2016 at 4:58 PM, Martin von Zweigbergk via Mercurial-devel
> <mercurial-devel@mercurial-scm.org> wrote:
>>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1478303512 25200
>> #      Fri Nov 04 16:51:52 2016 -0700
>> # Node ID bb80851fe9a6e14263f0076074108556377141f9
>> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>> fold: disallow multiple revisions without --exact
>
>
> Per conversation on IRC, I think that the opposite would be safer: disallow
> single revisions without explicit user consent (i.e. return to the original
> behavior, according to https://www.mercurial-scm.org/wiki/EvolveUI#Why.3F).
> The situation I'm afraid of with this change is something like this (sorry
> for the mild hypotheticals involved here):
>
>     If we assume a series of draft changes with A(t1), B(t1), C(t2), D(t3),
> E(t3,@),
>     then a hypothetical `hg fold topic(t1)` would error, but `hg fold
> topic(t2)` would fold C,D,E.
>
> I had been thinking that `hg fold .^` would be the primary reason for the
> current non-exact (so I proposed `hg fold --prev` as equivalent to `hg fold
> .^::.`), but https://www.mercurial-scm.org/wiki/EvolveUI#Why.3F says it's
> really more like "most-recent N commits".  I think requiring the user to
> count would not be ideal and it causes confusion with revision numbers, so I
> don't think that --prev would work with an argument.
>
> Instead, if we assume that most people stay tip-most in their work in
> progress chains, we could have a `hg fold --from X` as an "alias" for `hg
> fold X::.`, except that for safety reasons it should probably check that
> max(X) < .
>
> With that, merging the wip is, usually, `hg fold --from <nodeid of base of
> wip>`.  I think --to would be unnecessary - the number of times it's useful
> would be much lower (again, if we assume most non-power-users stay tip-most,
> and power-users can be expected to use revsets properly).

I just realized that maybe --exact should be called --change. Compare
to "hg diff" and "hg status". With a single -r, they compare that
revision with wdir(). With --change, they work on that change itself,
ignoring wdir() and '.'.

Note that that's just a comment about the name of the option, not
about the default that I found confusing. But actually, also note that
"hg diff -r $rev::" resolves to a single revision, the diff is between
that revision and wdir(), but if it resolves to a range, the diff is
between the min and the max (?) of that range. So while I very much
agree with Kyle's point about risk of having the behavior depend on
how many revisions a revset resolves to, there is precedent for it.

I'm still not sure how I want this to behave, but hopefully the above
points are relevant.

>
>
>
>
>>
>>
>> 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.
>>
>> diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
>> --- a/hgext/evolve.py   Wed Nov 02 18:56:44 2016 +0100
>> +++ b/hgext/evolve.py   Fri Nov 04 16:51:52 2016 -0700
>> @@ -3115,6 +3115,11 @@
>>      revs = scmutil.revrange(repo, revs)
>>
>>      if not opts['exact']:
>> +        if len(revs) > 1:
>> +            raise error.Abort(_("cannot fold from working directory to "
>> +                                "more than one revision"),
>> +                              hint=_("did you mean to use --exact?"))
>> +
>>          # Try to extend given revision starting from the working
>> directory
>>          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
>>          discardedrevs = [r for r in revs if r not in extrevs]
>> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
>> --- a/tests/test-evolve.t       Wed Nov 02 18:56:44 2016 +0100
>> +++ b/tests/test-evolve.t       Fri Nov 04 16:51:52 2016 -0700
>> @@ -688,7 +688,11 @@
>>    $ hg fold -r 4 -r 6 --exact
>>    abort: cannot fold non-linear revisions (multiple roots given)
>>    [255]
>> -  $ hg fold 10 1
>> +  $ hg fold 4::5
>> +  abort: cannot fold from working directory to more than one revision
>> +  (did you mean to use --exact?)
>> +  [255]
>> +  $ hg fold 1
>>    abort: cannot fold non-linear revisions
>>    (given revisions are unrelated to parent of working directory)
>>    [255]
>> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
>> --- a/tests/test-userguide.t    Wed Nov 02 18:56:44 2016 +0100
>> +++ b/tests/test-userguide.t    Fri Nov 04 16:51:52 2016 -0700
>> @@ -109,7 +109,7 @@
>>    7:05e61aab8294  step 1
>>    8:be6d5bc8e4cc  step 2
>>    9:35f432d9f7c1  step 3
>> -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
>> +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
>>    3 changesets folded
>>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>    $ hg --hidden shortlog -G -r 6::
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
Jordi Gutiérrez Hermoso - Nov. 19, 2016, 8:37 p.m.
On Fri, 2016-11-04 at 16:58 -0700, Martin von Zweigbergk via
Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@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.

There was some kind of explicit reason for why originally Pierre-Yves
convinced me that the current behaviour was correct. Something about
how `hg fold 'draft()'` would be a common thing. I can't remember the
justification anymore, something about how it's impossible to predict
in advance if a revset is a single or multiple commits.

I think I agree with everyone that --exact should probably just become
the default again as it once was. No more `hg fold .^`, you must now
do `hg fold '. + .^'` like before. Everyone seems to really want
--exact and gets surprised by the magic of trying to include "." in
the folded set.
Pierre-Yves David - Dec. 3, 2016, 3:37 a.m.
Note: I've not forgotten about this patch, I'm just extremely backlogged 
and I'll process evolve patches when the mercurial ones are handled.

Sorry about the delay.

On 11/05/2016 12:58 AM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@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.
>
> diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
> --- a/hgext/evolve.py    Wed Nov 02 18:56:44 2016 +0100
> +++ b/hgext/evolve.py    Fri Nov 04 16:51:52 2016 -0700
> @@ -3115,6 +3115,11 @@
>      revs = scmutil.revrange(repo, revs)
>
>      if not opts['exact']:
> +        if len(revs) > 1:
> +            raise error.Abort(_("cannot fold from working directory to "
> +                                "more than one revision"),
> +                              hint=_("did you mean to use --exact?"))
> +
>          # Try to extend given revision starting from the working directory
>          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
>          discardedrevs = [r for r in revs if r not in extrevs]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
> --- a/tests/test-evolve.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-evolve.t    Fri Nov 04 16:51:52 2016 -0700
> @@ -688,7 +688,11 @@
>    $ hg fold -r 4 -r 6 --exact
>    abort: cannot fold non-linear revisions (multiple roots given)
>    [255]
> -  $ hg fold 10 1
> +  $ hg fold 4::5
> +  abort: cannot fold from working directory to more than one revision
> +  (did you mean to use --exact?)
> +  [255]
> +  $ hg fold 1
>    abort: cannot fold non-linear revisions
>    (given revisions are unrelated to parent of working directory)
>    [255]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
> --- a/tests/test-userguide.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-userguide.t    Fri Nov 04 16:51:52 2016 -0700
> @@ -109,7 +109,7 @@
>    7:05e61aab8294  step 1
>    8:be6d5bc8e4cc  step 2
>    9:35f432d9f7c1  step 3
> -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
> +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
>    3 changesets folded
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ hg --hidden shortlog -G -r 6::
Pierre-Yves David - Dec. 17, 2016, 7:53 a.m.
On 11/05/2016 12:58 AM, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@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.

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

> diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
> --- a/hgext/evolve.py    Wed Nov 02 18:56:44 2016 +0100
> +++ b/hgext/evolve.py    Fri Nov 04 16:51:52 2016 -0700
> @@ -3115,6 +3115,11 @@
>      revs = scmutil.revrange(repo, revs)
>
>      if not opts['exact']:
> +        if len(revs) > 1:
> +            raise error.Abort(_("cannot fold from working directory to "
> +                                "more than one revision"),
> +                              hint=_("did you mean to use --exact?"))
> +
>          # Try to extend given revision starting from the working directory
>          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
>          discardedrevs = [r for r in revs if r not in extrevs]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
> --- a/tests/test-evolve.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-evolve.t    Fri Nov 04 16:51:52 2016 -0700
> @@ -688,7 +688,11 @@
>    $ hg fold -r 4 -r 6 --exact
>    abort: cannot fold non-linear revisions (multiple roots given)
>    [255]
> -  $ hg fold 10 1
> +  $ hg fold 4::5
> +  abort: cannot fold from working directory to more than one revision
> +  (did you mean to use --exact?)
> +  [255]
> +  $ hg fold 1
>    abort: cannot fold non-linear revisions
>    (given revisions are unrelated to parent of working directory)
>    [255]
> diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
> --- a/tests/test-userguide.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-userguide.t    Fri Nov 04 16:51:52 2016 -0700
> @@ -109,7 +109,7 @@
>    7:05e61aab8294  step 1
>    8:be6d5bc8e4cc  step 2
>    9:35f432d9f7c1  step 3
> -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
> +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
>    3 changesets folded
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ hg --hidden shortlog -G -r 6::
via Mercurial-devel - Dec. 18, 2016, 5:36 p.m.
On Fri, Dec 16, 2016, 23:53 Pierre-Yves David <
pierre-yves.david@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@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). 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 '.'. "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.



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


> > diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
> > --- a/hgext/evolve.py    Wed Nov 02 18:56:44 2016 +0100
> > +++ b/hgext/evolve.py    Fri Nov 04 16:51:52 2016 -0700
> > @@ -3115,6 +3115,11 @@
> >      revs = scmutil.revrange(repo, revs)
> >
> >      if not opts['exact']:
> > +        if len(revs) > 1:
> > +            raise error.Abort(_("cannot fold from working directory to "
> > +                                "more than one revision"),
> > +                              hint=_("did you mean to use --exact?"))
> > +
> >          # Try to extend given revision starting from the working
> directory
> >          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
> >          discardedrevs = [r for r in revs if r not in extrevs]
> > diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
> > --- a/tests/test-evolve.t    Wed Nov 02 18:56:44 2016 +0100
> > +++ b/tests/test-evolve.t    Fri Nov 04 16:51:52 2016 -0700
> > @@ -688,7 +688,11 @@
> >    $ hg fold -r 4 -r 6 --exact
> >    abort: cannot fold non-linear revisions (multiple roots given)
> >    [255]
> > -  $ hg fold 10 1
> > +  $ hg fold 4::5
> > +  abort: cannot fold from working directory to more than one revision
> > +  (did you mean to use --exact?)
> > +  [255]
> > +  $ hg fold 1
> >    abort: cannot fold non-linear revisions
> >    (given revisions are unrelated to parent of working directory)
> >    [255]
> > diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
> > --- a/tests/test-userguide.t    Wed Nov 02 18:56:44 2016 +0100
> > +++ b/tests/test-userguide.t    Fri Nov 04 16:51:52 2016 -0700
> > @@ -109,7 +109,7 @@
> >    7:05e61aab8294  step 1
> >    8:be6d5bc8e4cc  step 2
> >    9:35f432d9f7c1  step 3
> > -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
> > +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
> >    3 changesets folded
> >    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> >    $ hg --hidden shortlog -G -r 6::
>
>
> --
> Pierre-Yves David
>
Pierre-Yves David - Jan. 4, 2017, 12:53 p.m.
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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>     <mailto:martinvonz@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,
via Mercurial-devel - Jan. 4, 2017, 4:38 p.m.
On Wed, Jan 4, 2017 at 4:53 AM, Pierre-Yves David
<pierre-yves.david@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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>>     <mailto:martinvonz@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
via Mercurial-devel - Jan. 4, 2017, 4:45 p.m.
On Wed, Jan 4, 2017 at 8:38 AM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
> On Wed, Jan 4, 2017 at 4:53 AM, Pierre-Yves David
> <pierre-yves.david@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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>>>     <mailto:martinvonz@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
Pierre-Yves David - Jan. 4, 2017, 7:40 p.m.
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.david@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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>>>     <mailto:martinvonz@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
via Mercurial-devel - Jan. 4, 2017, 9:47 p.m.
On Wed, Jan 4, 2017 at 11:40 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> 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.david@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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>>>>     <mailto:martinvonz@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 ?

Thanks for mentioning rebase. Rebase has various ways of specifying
which revisions to rebase (-b,-s,-r). It also has a default, which I
consider a mistake. So, how about making "hg fold" not have a default?
I suggest we ask the user to say either "hg fold --from=$rev" to get
the behavior they currently get by default, and "hg fold -r $revset"
(or "hg fold -r $rev1 -r $rev2" for beginners). Note that I'm
suggesting that --exact will not be needed (and can be deprecated),
because I don't like to have the argument passed to -r be used for
different things based on that flag. How does that sound?

>
>
> (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
Pierre-Yves David - Jan. 6, 2017, 3:12 p.m.
On 01/04/2017 10:47 PM, Martin von Zweigbergk wrote:
> On Wed, Jan 4, 2017 at 11:40 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>> 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.david@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@ens-lyon.org <mailto:pierre-yves.david@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@google.com
>>>>>     <mailto:martinvonz@google.com>>
>>>>>     > # Date 1478303512 25200
>>>>>     > #      Fri Nov 04 16:51:52 2016 -0700
>>>>>     > # Node ID bb80851fe9a6e14263f0076074108556377141f9
>>>>>     > # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>>>>>     > fold: disallow multiple revisions without --exact
>>>> […]
>>>>
>>>> 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 ?
>
> Thanks for mentioning rebase. Rebase has various ways of specifying
> which revisions to rebase (-b,-s,-r). It also has a default, which I
> consider a mistake.

Actually, rebase has two defaults:

- The default destination:
     It used to be pretty bad, working only in the simplest case and 
being harmful in all the others. However, this actually got fixed last 
year and it now pretty okay. It even has the potential to become pretty 
awesome once we have a good story regarding feature branches and names. 
See associated wiki page for details:
https://www.mercurial-scm.org/wiki/DefaultDestinationPlan

- The default rebase set: (-b .)
     It seems to fit the needs in most case and I don't think I've seen 
much complains about it. We can probably improve that a bit when we get 
a good "current working set" story through feature branch.

I agree that bad default are mistake, but good default are great and 
have been a good advantage of Mercurial. We should keep aiming at good 
default whenever we can.

> So, how about making "hg fold" not have a default?
> I suggest we ask the user to say either "hg fold --from=$rev" to get
> the behavior they currently get by default, and "hg fold -r $revset"
> (or "hg fold -r $rev1 -r $rev2" for beginners). Note that I'm
> suggesting that --exact will not be needed (and can be deprecated),
> because I don't like to have the argument passed to -r be used for
> different things based on that flag. How does that sound?

The --from things is to consider, it seems to match our current 
constraint including the one about not being worse than the existing 
solution (It does not strike me as much better, but at least not worse). 
Not sure about the word since we might want to allow it to specify 
descendant too (not just ancestors).

I'm not sure what you mean with the --rev flag. Do you mean that default 
entry would be revset? Or that --rev would trigger the revset mode? or 
something else? Previous discussion on the topic concluded that having 
mode switch on --rev wasn't a good idea.

In all case, that discussion is getting deeper and we should switch to VC.

>> […]
via Mercurial-devel - Jan. 6, 2017, 5:03 p.m.
On Fri, Jan 6, 2017 at 7:12 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 01/04/2017 10:47 PM, Martin von Zweigbergk wrote:
>>
>> On Wed, Jan 4, 2017 at 11:40 AM, Pierre-Yves David
>> <pierre-yves.david@ens-lyon.org> wrote:
>>>
>>> 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.david@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@ens-lyon.org
>>>>>> <mailto:pierre-yves.david@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@google.com
>>>>>>     <mailto:martinvonz@google.com>>
>>>>>>     > # Date 1478303512 25200
>>>>>>     > #      Fri Nov 04 16:51:52 2016 -0700
>>>>>>     > # Node ID bb80851fe9a6e14263f0076074108556377141f9
>>>>>>     > # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>>>>>>     > fold: disallow multiple revisions without --exact
>>>>>
>>>>> […]
>>>>>
>>>>>
>>>>> 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 ?
>>
>>
>> Thanks for mentioning rebase. Rebase has various ways of specifying
>> which revisions to rebase (-b,-s,-r). It also has a default, which I
>> consider a mistake.
>
>
> Actually, rebase has two defaults:
>
> - The default destination:
>     It used to be pretty bad, working only in the simplest case and being
> harmful in all the others. However, this actually got fixed last year and it
> now pretty okay. It even has the potential to become pretty awesome once we
> have a good story regarding feature branches and names. See associated wiki
> page for details:
> https://www.mercurial-scm.org/wiki/DefaultDestinationPlan

This is the one I was referring to. I'm happy with the default rebase set.

>
> - The default rebase set: (-b .)
>     It seems to fit the needs in most case and I don't think I've seen much
> complains about it. We can probably improve that a bit when we get a good
> "current working set" story through feature branch.
>
> I agree that bad default are mistake, but good default are great and have
> been a good advantage of Mercurial. We should keep aiming at good default
> whenever we can.

Oh, definitely. I liked git's default destination and relied on it all
the time. See https://github.com/git/git/commit/15a147e61898d25ec8b539190e87f3a09592c9c8
:-)

>
>> So, how about making "hg fold" not have a default?
>> I suggest we ask the user to say either "hg fold --from=$rev" to get
>> the behavior they currently get by default, and "hg fold -r $revset"
>> (or "hg fold -r $rev1 -r $rev2" for beginners). Note that I'm
>> suggesting that --exact will not be needed (and can be deprecated),
>> because I don't like to have the argument passed to -r be used for
>> different things based on that flag. How does that sound?
>
>
> The --from things is to consider, it seems to match our current constraint
> including the one about not being worse than the existing solution (It does
> not strike me as much better, but at least not worse). Not sure about the
> word since we might want to allow it to specify descendant too (not just
> ancestors).

We could also add a --to, but that seems like something that would be
used very rarely. Leave it out for now?

>
> I'm not sure what you mean with the --rev flag. Do you mean that default
> entry would be revset? Or that --rev would trigger the revset mode? or
> something else? Previous discussion on the topic concluded that having mode
> switch on --rev wasn't a good idea.

Examples to clarify what I mean:

$ hg fold .^
hg fold: invalid arguments
hg fold [--from REV] [--rev REV]...
$ hg fold --from .^ # folds .^ and .
$ hg fold --from .^ -r .
hg fold: invalid arguments
hg fold [--from REV] [--rev REV]...
$ hg fold -r .^
warning: asked to fold a single revision; nothing to do
$ hg fold -r .~2 -r .~1 #folds those two (and leaves .)
$ hg fold -r .~2::.~1 #folds those two (and leaves .)


>
> In all case, that discussion is getting deeper and we should switch to VC.
>
>>> […]
>
>
> --
> Pierre-Yves David

Patch

diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
--- a/hgext/evolve.py	Wed Nov 02 18:56:44 2016 +0100
+++ b/hgext/evolve.py	Fri Nov 04 16:51:52 2016 -0700
@@ -3115,6 +3115,11 @@ 
      revs = scmutil.revrange(repo, revs)

      if not opts['exact']:
+        if len(revs) > 1:
+            raise error.Abort(_("cannot fold from working directory to "
+                                "more than one revision"),
+                              hint=_("did you mean to use --exact?"))
+
          # Try to extend given revision starting from the working directory
          extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
          discardedrevs = [r for r in revs if r not in extrevs]
diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
--- a/tests/test-evolve.t	Wed Nov 02 18:56:44 2016 +0100
+++ b/tests/test-evolve.t	Fri Nov 04 16:51:52 2016 -0700
@@ -688,7 +688,11 @@ 
    $ hg fold -r 4 -r 6 --exact
    abort: cannot fold non-linear revisions (multiple roots given)
    [255]
-  $ hg fold 10 1
+  $ hg fold 4::5
+  abort: cannot fold from working directory to more than one revision
+  (did you mean to use --exact?)
+  [255]
+  $ hg fold 1
    abort: cannot fold non-linear revisions
    (given revisions are unrelated to parent of working directory)
    [255]
diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
--- a/tests/test-userguide.t	Wed Nov 02 18:56:44 2016 +0100
+++ b/tests/test-userguide.t	Fri Nov 04 16:51:52 2016 -0700
@@ -109,7 +109,7 @@ 
    7:05e61aab8294  step 1
    8:be6d5bc8e4cc  step 2
    9:35f432d9f7c1  step 3
-  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
+  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
    3 changesets folded
    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
    $ hg --hidden shortlog -G -r 6::