Patchwork UI discussions for revert --interactive and uncommit --interactive

login
register
mail settings
Submitter Laurent Charignon
Date May 22, 2015, 10:37 p.m.
Message ID <D184FDC9.8036%lcharignon@fb.com>
Download mbox | patch
Permalink /patch/9248/
State Not Applicable
Delegated to: Augie Fackler
Headers show

Comments

Laurent Charignon - May 22, 2015, 10:37 p.m.
Hi,

As you may have seen I worked on revert --interactive and soon I will send some patches for hg uncommit --interactive.

Both these commands raise a UI question that I have been debating with marmoute and sid0.
The --interactive session let your choose what changes are reverted. There is two main way to ask the question
We wanted to have your thoughts on it, so I put a minimal example below with the two propositions.
For the record, I want proposition 1 and marmoute wants proposition 2

# First commit an empty file
$  touch x ; hg add x ; hg commit -m "adding x"
# Add 4 lines to this file and commit
$  printf "1\n2\n3\n4\n" > x ; hg commit -m "change x"
# Show the diff of the last commit
$  hg diff -c .

# Both propositions end up with x being an empty file

# proposition 1: show the changes to be applied to the workdir
$ hg revert -i -r .^
reverting x
diff -r 38385f891157 x
1 hunks, 4 lines changed
examine changes to 'x'? [Ynesfdaq?] y

@@ -1,4 +0,0 @@
-1
-2
-3
-4
\ No newline at end of file
apply this change to 'x'? [Ynesfdaq?] y
# end of proposition 1

# proposition 2: show the change revert will cancel
$ hg revert -i -r .^
reverting x
diff -r 38385f891157 x
1 hunks, 4 lines changed
examine changes to 'x'? [Ynesfdaq?] y

@@ -1,4 +0,0 @@
+1
+2
+3
+4
\ No newline at end of file
revert this change to 'x'? [Ynesfdaq?] y
# end of proposition 2

Why proposition 1 (from Laurent)?
You see what change will be applied.
If you select a line starting with a minus, it will remove the line in the workdir.

Why proposition 2 (from Pierre-Yves)?
Proposition 2 as the advantage of using the same output as the one in `hg diff`. This allow the following mind flows:

Mind Flow 1, nuking silly changes you just spotted
1) call `hg diff`
2) spot change you want reverted
3) call `hg revert -i`
4) spot the very same change again, select them, remove them.

Having the diff in the same order in `hg diff` and `hg revert` allow you to spot the same one quickly. Otherwise the patch inversion will swap the order of line and render pattern magic quick difficult (color change is also another offuscation)

Mind Flow 2, direct cleanup:
1) You already know there is a lot of debut output in your code
2) call `hg revert -i`
3) triage before valide change and invalid change

Having the diff in the "change" order here allow you to do the triage directly, without having to reverse it in your head.

What are your thoughts on this?

Thanks!

Laurent
Pierre-Yves David - May 22, 2015, 10:50 p.m.
On 05/22/2015 05:37 PM, Laurent Charignon wrote:
> Hi,
>
> As you may have seen I worked on revert --interactive and soon I will
> send some patches for hg uncommit --interactive.
>
> Both these commands raise a UI question that I have been debating with
> marmoute and sid0.
> The --interactive session let your choose what changes are reverted.
> There is two main way to ask the question
> We wanted to have your thoughts on it, so I put a minimal example below
> with the two propositions.
> For the record, I want *proposition 1* and marmoute wants *proposition 2*

The way I phrase the two solutions is as follow:

1) Show the action revert will perform (current implementation),
2) Show the change revert will cancel,

Here is an example I find more complete as it show some of the "line 
reordering" the diff reversion induces.

   $ hg diff
   --- a/mercurial/commands.py
   +++ b/mercurial/commands.py
   @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
        ('C', 'no-backup', None, _('do not save backup copies of files')),
        ('i', 'interactive', None,
   -            _('interactively select the changes (EXPERIMENTAL)')),
   +            _('interactively select the changes')),
        ] + walkopts + dryrunopts,
        _('[OPTION]... [-r REV] [NAME]...'))


Solution (1) show action:

   @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
        ('C', 'no-backup', None, _('do not save backup copies of files')),
        ('i', 'interactive', None,
   -            _('interactively select the changes')),
   +            _('interactively select the changes (EXPERIMENTAL)')),
        ] + walkopts + dryrunopts,
        _('[OPTION]... [-r REV] [NAME]...'))

   apply this change to 'mercurial/commands.py'? [Ynesfdaq?]


Solution (2) show change:

   --- a/mercurial/commands.py
   +++ b/mercurial/commands.py
   @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
        ('C', 'no-backup', None, _('do not save backup copies of files')),
        ('i', 'interactive', None,
   -            _('interactively select the changes (EXPERIMENTAL)')),
   +            _('interactively select the changes')),
        ] + walkopts + dryrunopts,
        _('[OPTION]... [-r REV] [NAME]...'))

   revert this change of 'mercurial/commands.py'? [Ynesfdaq?]

a variant of Solution could be to turn the question around

   Keep this change of 'mercurial/commands.py'? [yNesfdaq?]
Martin von Zweigbergk - May 22, 2015, 11:01 p.m.
On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 05/22/2015 05:37 PM, Laurent Charignon wrote:
> > Hi,
> >
> > As you may have seen I worked on revert --interactive and soon I will
> > send some patches for hg uncommit --interactive.
> >
> > Both these commands raise a UI question that I have been debating with
> > marmoute and sid0.
> > The --interactive session let your choose what changes are reverted.
> > There is two main way to ask the question
> > We wanted to have your thoughts on it, so I put a minimal example below
> > with the two propositions.
> > For the record, I want *proposition 1* and marmoute wants *proposition 2*
>
> The way I phrase the two solutions is as follow:
>
> 1) Show the action revert will perform (current implementation),
> 2) Show the change revert will cancel,
>

When reverting to another revision (picking pieces from that other
revision), I'm quite sure I want 1). And, therefore, I think I want 1) in
all cases for consistency.


>
> Here is an example I find more complete as it show some of the "line
> reordering" the diff reversion induces.
>
>    $ hg diff
>    --- a/mercurial/commands.py
>    +++ b/mercurial/commands.py
>    @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
>         ('C', 'no-backup', None, _('do not save backup copies of files')),
>         ('i', 'interactive', None,
>    -            _('interactively select the changes (EXPERIMENTAL)')),
>    +            _('interactively select the changes')),
>         ] + walkopts + dryrunopts,
>         _('[OPTION]... [-r REV] [NAME]...'))
>
>
> Solution (1) show action:
>
>    @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
>         ('C', 'no-backup', None, _('do not save backup copies of files')),
>         ('i', 'interactive', None,
>    -            _('interactively select the changes')),
>    +            _('interactively select the changes (EXPERIMENTAL)')),
>         ] + walkopts + dryrunopts,
>         _('[OPTION]... [-r REV] [NAME]...'))
>
>    apply this change to 'mercurial/commands.py'? [Ynesfdaq?]
>
>
> Solution (2) show change:
>
>    --- a/mercurial/commands.py
>    +++ b/mercurial/commands.py
>    @@ -5469,5 +5469,5 @@ def resolve(ui, repo, *pats, **opts):
>         ('C', 'no-backup', None, _('do not save backup copies of files')),
>         ('i', 'interactive', None,
>    -            _('interactively select the changes (EXPERIMENTAL)')),
>    +            _('interactively select the changes')),
>         ] + walkopts + dryrunopts,
>         _('[OPTION]... [-r REV] [NAME]...'))
>
>    revert this change of 'mercurial/commands.py'? [Ynesfdaq?]
>
> a variant of Solution could be to turn the question around
>
>    Keep this change of 'mercurial/commands.py'? [yNesfdaq?]
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Durham Goode - May 22, 2015, 11:18 p.m.
On 5/22/15 4:01 PM, Martin von Zweigbergk wrote:
>
>
> On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org 
> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>
>
>
>     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
>     > Hi,
>     >
>     > As you may have seen I worked on revert --interactive and soon I
>     will
>     > send some patches for hg uncommit --interactive.
>     >
>     > Both these commands raise a UI question that I have been
>     debating with
>     > marmoute and sid0.
>     > The --interactive session let your choose what changes are reverted.
>     > There is two main way to ask the question
>     > We wanted to have your thoughts on it, so I put a minimal
>     example below
>     > with the two propositions.
>     > For the record, I want *proposition 1* and marmoute wants
>     *proposition 2*
>
>     The way I phrase the two solutions is as follow:
>
>     1) Show the action revert will perform (current implementation),
>     2) Show the change revert will cancel,
>
>
> When reverting to another revision (picking pieces from that other 
> revision), I'm quite sure I want 1). And, therefore, I think I want 1) 
> in all cases for consistency.
'commit -i' currently indicates the actions that will be applied to the 
commit.  I think it makes sense to have revert/uncommit/etc work the 
same way for consistency (i.e. #1).

You can think of it as requiring less mind-state for the user.  If I sat 
down at a terminal that had the crecord UI already open, with #1 I know 
that I need to select the lines that show the changes I want (regardless 
of if this was a commit, revert, whatever).  With #2 I have to be aware 
that this is a revert and that what I'm seeing is actually me choosing 
things I don't want.  The double negative feels confusing to me.
Gilles Moris - May 23, 2015, 7:16 a.m.
Le 23/05/2015 01:18, Durham Goode a écrit :
>
>
> On 5/22/15 4:01 PM, Martin von Zweigbergk wrote:
>>
>>
>> On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David 
>> <pierre-yves.david@ens-lyon.org 
>> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>>
>>
>>
>>     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
>>     > Hi,
>>     >
>>     > As you may have seen I worked on revert --interactive and soon
>>     I will
>>     > send some patches for hg uncommit --interactive.
>>     >
>>     > Both these commands raise a UI question that I have been
>>     debating with
>>     > marmoute and sid0.
>>     > The --interactive session let your choose what changes are
>>     reverted.
>>     > There is two main way to ask the question
>>     > We wanted to have your thoughts on it, so I put a minimal
>>     example below
>>     > with the two propositions.
>>     > For the record, I want *proposition 1* and marmoute wants
>>     *proposition 2*
>>
>>     The way I phrase the two solutions is as follow:
>>
>>     1) Show the action revert will perform (current implementation),
>>     2) Show the change revert will cancel,
>>
>>
>> When reverting to another revision (picking pieces from that other 
>> revision), I'm quite sure I want 1). And, therefore, I think I want 
>> 1) in all cases for consistency.
> 'commit -i' currently indicates the actions that will be applied to 
> the commit.  I think it makes sense to have revert/uncommit/etc work 
> the same way for consistency (i.e. #1).
>
> You can think of it as requiring less mind-state for the user.  If I 
> sat down at a terminal that had the crecord UI already open, with #1 I 
> know that I need to select the lines that show the changes I want 
> (regardless of if this was a commit, revert, whatever).  With #2 I 
> have to be aware that this is a revert and that what I'm seeing is 
> actually me choosing things I don't want. The double negative feels 
> confusing to me.

I prefer proposition #2 because I would like hg commit -i to present me 
exactly the same thing as hg revert -i. I was also very confused by the 
current implementation.
At least when not using --rev. If using --rev, I think I would do it in 
two steps probably, first with --rev, then with --interactive.
May be this is exacerbating because I am using crecord, so I wonder what 
I shall check/uncheck...
For uncommit -i, I would expect crecord to present me the same thing as 
commit -i, and what I am unchecking is reverted in the working directory 
for instance.

Could it be made configurable? Then the discussion would only fallback 
to the default.

Regards.
Gilles.
Pierre-Yves David - May 24, 2015, 10:08 p.m.
On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>
>
> On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>
>
>     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
>      > Hi,
>      >
>      > As you may have seen I worked on revert --interactive and soon I will
>      > send some patches for hg uncommit --interactive.
>      >
>      > Both these commands raise a UI question that I have been debating
>     with
>      > marmoute and sid0.
>      > The --interactive session let your choose what changes are reverted.
>      > There is two main way to ask the question
>      > We wanted to have your thoughts on it, so I put a minimal example
>     below
>      > with the two propositions.
>      > For the record, I want *proposition 1* and marmoute wants
>     *proposition 2*
>
>     The way I phrase the two solutions is as follow:
>
>     1) Show the action revert will perform (current implementation),
>     2) Show the change revert will cancel,
>
>
> When reverting to another revision (picking pieces from that other
> revision), I'm quite sure I want 1). And, therefore, I think I want 1)
> in all cases for consistency.

Your sentence make me realized we can use revert in two ways.
a) reverting against an ancestor: (actually revert changes)
b) reverting against a "descendant": (fetching from it).

In the ancestor case (a), `hg diff --rev (a)` would be consistent with 
what solution (2) show during revert.
In the descendant case (b), `hg diff --rev (b)` would be consistent with 
what solution (1) show during revert.

I've to admit that I barely use the (b) case anymore. I used to need it 
to rework stack of patch, but uncommit/fold/etc are fitting this need 
for me now. My biggest usage of `hg revert` is without a revision to 
clean up silly change (debug print) or against an ancestors to try to 
get something to work again.

The consistency argument is interesting here, let's widen it. The most 
important point to me is the consistency of the experience across all 
commands. Mercurial is a VCS, vcs are mostly about handling "changes" 
(hunks in this context). The vast majority of command present such 
change in the same direction, from parent to child.

commands          | direction
hg diff           | parent -> child
hg diff -r (a)    | parent -> child
hg diff -r (b)    | child  -> parent <- one exception
hg diff -r .::(b) | parent -> child
hg commit -i      | parent -> child
hg amend -i       | parent -> child
hg export         | parent -> child

So I would argues having the revert inverting (solution 1) this 
direction to be inconsistent. Solution 2 would keep the pattern of 
display consistent:

hg diff             | parent -> child
hg diff -r (a)      | parent -> child
hg diff -r (b)      | child  -> parent
hg revert           | parent -> child
hg revert -r (a)    | parent -> child
hg revert -r (b)    | child  -> parent
hg uncommit         | parent -> child
hg uncommit -r (a)  | parent -> child
hg uncommit -r (b)  | child  -> parent


This inconsistency is especially problematic because it make the hunk 
looks different and therefor much harder to recognize. This inversion is 
not just about switching the "-" and "+" char around. They implies a 
deep visual changes:

- inversion in the green/red color , especially the size of block:
   (I want to revert this big green block -> huuu there is just a big 
red block now).

- inversion in the order of line: all additions because deletion and are 
set as first line, changing the shape of the "hunk".

Personally this deep visual changes induce a massive cognitive overhead 
for me. With solution (1) every other command I use show me changes in a 
given way but `revert -i` (with curse UI). I cannot rely on pattern 
recognition anymore.


If `hg revert` were consistent with the other commands especially `hg 
diff`, I could combine some operation in my workflow. For example, being 
able to review your changes `hg diff` and clean up debug output and 
other silliness `hg revert`. The curse UI is a very powerful tool to 
navigate and review your change. I fire up `hg commit -i` instead of `hg 
diff; hg commit # if all okay` on a regular basis. Having `hg revert -i` 
display the same data let me perform such cleanup the same way.

This way to use the UI can be pushed further. I remember some 
discussions at a sprint about a `hg split -i` UI which would use a 
crecord like UI and let you assign number to chunk (that one is in the 
first commit, that one in the second one), having a "forget about this 
change" (uncommit|revert it) category in such command would fit well, 
and it will have to be "parent" -> child" direction.


(yep, I know, this email is getting quite long, but I've a last point to 
make)

I understand this argument of solution (1) is about"showing what is the 
action revert will do". But I think it is a bit to low level. As I said 
before, Mercurial is mostly about managing "changes"

- hg diff -> show your change current uncommited (and other)
- hg commit -> put your change into a commit
- hg amend -> add more change to the current commit
- hg uncommit -> move change from commit to working copu
- hg revert -> remove current changes (and other)

And so solution (2) focus on showing these changes. The fact that 
"change" makes more sense than "action" become more visible when looking 
at 'uncommit'. `hg uncommit -i` is going to take a "change" a move it 
from the current commit to the working copy. The action, performed on 
the commit is "apply the reverse of that change", but at the same time 
it (kind of) apply the same change back to your working copy (in 
practice, file are untouched, but `hg diff` gained that change). So 
`uncommit` is doing "action"s in both direction and "picking the 
direction the action is done" will not help picking a side here (And we 
want `hg uncommit -i` and `hg revert -i` to be consistent, especially 
since `hg uncommit` may gain a --revert option). So showing "the change 
to be reverted" is a sensible things to do, in particular since the 
command is called `hg revert` not `hg apply reverse-diff-from`.


To highlight this "change" centric UI and mitigate the "revert show the 
inverse of the action" concern, the two variants of solution (2) have to 
be looked at.

- The basic approach (2.1): Select change to be reverted.
   (here is the work you've have done, do you want to throw it away)
- The alternative approach (2.2):
   We revert everything, select change to be preserved.

The alternative approach mean different questions and default value, 
with the same core display for the change. This probably get use best of 
both world:

- Diff display match the action performed (preserve X)
- Diff are not displayed reversed.

(The hg revert -ir (b) get maybe a bit awkward, but it is not the core 
usage of `hg revert`)

To conclude, if we cannot read a simple consensus quickly, I advice for 
having an experimental config options and get each side of the debate to 
test the other approach until 3.5 freeze.
Martin von Zweigbergk - May 26, 2015, 8:25 p.m.
On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
> >
> >
> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> > wrote:
> >
> >
> >
> >     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
> >      > Hi,
> >      >
> >      > As you may have seen I worked on revert --interactive and soon I
> will
> >      > send some patches for hg uncommit --interactive.
> >      >
> >      > Both these commands raise a UI question that I have been debating
> >     with
> >      > marmoute and sid0.
> >      > The --interactive session let your choose what changes are
> reverted.
> >      > There is two main way to ask the question
> >      > We wanted to have your thoughts on it, so I put a minimal example
> >     below
> >      > with the two propositions.
> >      > For the record, I want *proposition 1* and marmoute wants
> >     *proposition 2*
> >
> >     The way I phrase the two solutions is as follow:
> >
> >     1) Show the action revert will perform (current implementation),
> >     2) Show the change revert will cancel,
> >
> >
> > When reverting to another revision (picking pieces from that other
> > revision), I'm quite sure I want 1). And, therefore, I think I want 1)
> > in all cases for consistency.
>
> Your sentence make me realized we can use revert in two ways.
> a) reverting against an ancestor: (actually revert changes)
> b) reverting against a "descendant": (fetching from it).
>

There are revisions that are neither of those (siblings, uncles and such).


> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
> what solution (2) show during revert.
> In the descendant case (b), `hg diff --rev (b)` would be consistent with
> what solution (1) show during revert.
>
> I've to admit that I barely use the (b) case anymore. I used to need it
> to rework stack of patch, but uncommit/fold/etc are fitting this need
> for me now. My biggest usage of `hg revert` is without a revision to
> clean up silly change (debug print) or against an ancestors to try to
> get something to work again.
>
> The consistency argument is interesting here, let's widen it. The most
> important point to me is the consistency of the experience across all
> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
> (hunks in this context). The vast majority of command present such
> change in the same direction, from parent to child.
>
> commands          | direction
> hg diff           | parent -> child
> hg diff -r (a)    | parent -> child
> hg diff -r (b)    | child  -> parent <- one exception
>

With this history:
x---a---c
     \
      @---b

Currently, it behaves this way (with clean working copy):
hg diff -r a | diff from a to @
hg diff r b  | diff from b to @
hg diff -r c | diff from c to @

I think you're suggesting:
hg diff -r a | diff from a to @
hg diff r b  | diff from @ to b
hg diff -r c | diff from @ to c

I fear that a "hg diff -r a" followed by a "hg diff -r c" would surprise
people, because the hunks corresponding to changes between @ and 'a' would
be reversed between the two invocations.



> hg diff -r .::(b) | parent -> child
> hg commit -i      | parent -> child
> hg amend -i       | parent -> child
> hg export         | parent -> child
>
> So I would argues having the revert inverting (solution 1) this
> direction to be inconsistent. Solution 2 would keep the pattern of
> display consistent:
>
> hg diff             | parent -> child
> hg diff -r (a)      | parent -> child
> hg diff -r (b)      | child  -> parent
> hg revert           | parent -> child
> hg revert -r (a)    | parent -> child
> hg revert -r (b)    | child  -> parent
> hg uncommit         | parent -> child
> hg uncommit -r (a)  | parent -> child
> hg uncommit -r (b)  | child  -> parent
>
>
> This inconsistency is especially problematic because it make the hunk
> looks different and therefor much harder to recognize. This inversion is
> not just about switching the "-" and "+" char around. They implies a
> deep visual changes:
>
> - inversion in the green/red color , especially the size of block:
>    (I want to revert this big green block -> huuu there is just a big
> red block now).
>
> - inversion in the order of line: all additions because deletion and are
> set as first line, changing the shape of the "hunk".
>
> Personally this deep visual changes induce a massive cognitive overhead
> for me.


I do understand your confusion :-(. I was tempted to suggest a new command
for the "pick pieces from other revision" operation, but then a new command
for each of your examples would be needed. I don't really have a good
answer. Sorry.


> With solution (1) every other command I use show me changes in a
> given way but `revert -i` (with curse UI). I cannot rely on pattern
> recognition anymore.
>
>
> If `hg revert` were consistent with the other commands especially `hg
> diff`, I could combine some operation in my workflow. For example, being
> able to review your changes `hg diff` and clean up debug output and
> other silliness `hg revert`. The curse UI is a very powerful tool to
> navigate and review your change. I fire up `hg commit -i` instead of `hg
> diff; hg commit # if all okay` on a regular basis. Having `hg revert -i`
> display the same data let me perform such cleanup the same way.
>
> This way to use the UI can be pushed further. I remember some
> discussions at a sprint about a `hg split -i` UI which would use a
> crecord like UI and let you assign number to chunk (that one is in the
> first commit, that one in the second one), having a "forget about this
> change" (uncommit|revert it) category in such command would fit well,
> and it will have to be "parent" -> child" direction.
>
>
> (yep, I know, this email is getting quite long, but I've a last point to
> make)
>
> I understand this argument of solution (1) is about"showing what is the
> action revert will do". But I think it is a bit to low level. As I said
> before, Mercurial is mostly about managing "changes"
>
> - hg diff -> show your change current uncommited (and other)
> - hg commit -> put your change into a commit
> - hg amend -> add more change to the current commit
> - hg uncommit -> move change from commit to working copu
> - hg revert -> remove current changes (and other)
>
> And so solution (2) focus on showing these changes. The fact that
> "change" makes more sense than "action" become more visible when looking
> at 'uncommit'. `hg uncommit -i` is going to take a "change" a move it
> from the current commit to the working copy. The action, performed on
> the commit is "apply the reverse of that change", but at the same time
> it (kind of) apply the same change back to your working copy (in
> practice, file are untouched, but `hg diff` gained that change). So
> `uncommit` is doing "action"s in both direction and "picking the
> direction the action is done" will not help picking a side here (And we
> want `hg uncommit -i` and `hg revert -i` to be consistent, especially
> since `hg uncommit` may gain a --revert option). So showing "the change
> to be reverted" is a sensible things to do, in particular since the
> command is called `hg revert` not `hg apply reverse-diff-from`.
>
>
> To highlight this "change" centric UI and mitigate the "revert show the
> inverse of the action" concern, the two variants of solution (2) have to
> be looked at.
>
> - The basic approach (2.1): Select change to be reverted.
>    (here is the work you've have done, do you want to throw it away)
> - The alternative approach (2.2):
>    We revert everything, select change to be preserved.
>
> The alternative approach mean different questions and default value,
> with the same core display for the change. This probably get use best of
> both world:
>
> - Diff display match the action performed (preserve X)
> - Diff are not displayed reversed.
>
> (The hg revert -ir (b) get maybe a bit awkward, but it is not the core
> usage of `hg revert`)
>
> To conclude, if we cannot read a simple consensus quickly, I advice for
> having an experimental config options and get each side of the debate to
> test the other approach until 3.5 freeze.
>
> --
> Pierre-Yves David
>
Pierre-Yves David - May 26, 2015, 8:42 p.m.
On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
> On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org 
> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>
>
>
>     On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>     >
>     >
>     > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
>     > <pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>
>     > wrote:
>     >
>     >
>     >
>     >     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
>     >      > Hi,
>     >      >
>     >      > As you may have seen I worked on revert --interactive and
>     soon I will
>     >      > send some patches for hg uncommit --interactive.
>     >      >
>     >      > Both these commands raise a UI question that I have been
>     debating
>     >     with
>     >      > marmoute and sid0.
>     >      > The --interactive session let your choose what changes
>     are reverted.
>     >      > There is two main way to ask the question
>     >      > We wanted to have your thoughts on it, so I put a minimal
>     example
>     >     below
>     >      > with the two propositions.
>     >      > For the record, I want *proposition 1* and marmoute wants
>     >     *proposition 2*
>     >
>     >     The way I phrase the two solutions is as follow:
>     >
>     >     1) Show the action revert will perform (current implementation),
>     >     2) Show the change revert will cancel,
>     >
>     >
>     > When reverting to another revision (picking pieces from that other
>     > revision), I'm quite sure I want 1). And, therefore, I think I
>     want 1)
>     > in all cases for consistency.
>
>     Your sentence make me realized we can use revert in two ways.
>     a) reverting against an ancestor: (actually revert changes)
>     b) reverting against a "descendant": (fetching from it).
>
>
> There are revisions that are neither of those (siblings, uncles and such).

They fall intro the (b) case (this is what the quote around descendant 
were meant for, okay it was sneaky). I categorize the same way because 
your intend is "fetch change from REV".

(I'm also a bit curious about how often you need it a what you use it for)

>
>     In the ancestor case (a), `hg diff --rev (a)` would be consistent with
>     what solution (2) show during revert.
>     In the descendant case (b), `hg diff --rev (b)` would be
>     consistent with
>     what solution (1) show during revert.
>
>     I've to admit that I barely use the (b) case anymore. I used to
>     need it
>     to rework stack of patch, but uncommit/fold/etc are fitting this need
>     for me now. My biggest usage of `hg revert` is without a revision to
>     clean up silly change (debug print) or against an ancestors to try to
>     get something to work again.
>
>     The consistency argument is interesting here, let's widen it. The most
>     important point to me is the consistency of the experience across all
>     commands. Mercurial is a VCS, vcs are mostly about handling "changes"
>     (hunks in this context). The vast majority of command present such
>     change in the same direction, from parent to child.
>
>     commands          | direction
>     hg diff           | parent -> child
>     hg diff -r (a)    | parent -> child
>     hg diff -r (b)    | child  -> parent <- one exception
>
>
> With this history:
> x---a---c
>      \
>       @---b
>
> Currently, it behaves this way (with clean working copy):
> hg diff -r a | diff from a to @
> hg diff r b  | diff from b to @
> hg diff -r c | diff from c to @
>
> I think you're suggesting:
> hg diff -r a | diff from a to @
> hg diff r b  | diff from @ to b
> hg diff -r c | diff from @ to c
>
> I fear that a "hg diff -r a" followed by a "hg diff -r c" would 
> surprise people, because the hunks corresponding to changes between @ 
> and 'a' would be reversed between the two invocations.

I'm confused.  I'm not suggesting changing `hg diff` at all. I'm just 
pointing at how `hg diff` behave currently and how I think it is fine to 
have `hg revert` match this behavior.

Did you misunderstood me?
Martin von Zweigbergk - May 26, 2015, 9:02 p.m.
On Tue, May 26, 2015 at 1:43 PM Pierre-Yves David <pyd@fb.com> wrote:

>
>
> On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
>
>  On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
> pierre-yves.david@ens-lyon.org> wrote:
>
>>
>>
>> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>> >
>> >
>> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
>> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org
>> >>
>> > wrote:
>> >
>> >
>> >
>> >     On 05/22/2015 05:37 PM, Laurent Charignon wrote:
>> >      > Hi,
>> >      >
>> >      > As you may have seen I worked on revert --interactive and soon I
>> will
>> >      > send some patches for hg uncommit --interactive.
>> >      >
>> >      > Both these commands raise a UI question that I have been debating
>> >     with
>> >      > marmoute and sid0.
>> >      > The --interactive session let your choose what changes are
>> reverted.
>> >      > There is two main way to ask the question
>> >      > We wanted to have your thoughts on it, so I put a minimal example
>> >     below
>> >      > with the two propositions.
>> >      > For the record, I want *proposition 1* and marmoute wants
>> >     *proposition 2*
>> >
>> >     The way I phrase the two solutions is as follow:
>> >
>> >     1) Show the action revert will perform (current implementation),
>> >     2) Show the change revert will cancel,
>> >
>> >
>> > When reverting to another revision (picking pieces from that other
>> > revision), I'm quite sure I want 1). And, therefore, I think I want 1)
>> > in all cases for consistency.
>>
>> Your sentence make me realized we can use revert in two ways.
>> a) reverting against an ancestor: (actually revert changes)
>> b) reverting against a "descendant": (fetching from it).
>>
>
>   There are revisions that are neither of those (siblings, uncles and
> such).
>
>
> They fall intro the (b) case (this is what the quote around descendant
> were meant for, okay it was sneaky). I categorize the same way because your
> intend is "fetch change from REV".
>

I suspected that was the case, but I wanted to make it explicit.


>
> (I'm also a bit curious about how often you need it a what you use it for)
>

I think I usually use when I have a branch and I think of an alternative
approach, so I start from scratch, but I still want some pieces from the
old branch (perhaps test cases).


>
>
>
>> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
>> what solution (2) show during revert.
>> In the descendant case (b), `hg diff --rev (b)` would be consistent with
>> what solution (1) show during revert.
>>
>> I've to admit that I barely use the (b) case anymore. I used to need it
>> to rework stack of patch, but uncommit/fold/etc are fitting this need
>> for me now. My biggest usage of `hg revert` is without a revision to
>> clean up silly change (debug print) or against an ancestors to try to
>> get something to work again.
>>
>> The consistency argument is interesting here, let's widen it. The most
>> important point to me is the consistency of the experience across all
>> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
>> (hunks in this context). The vast majority of command present such
>> change in the same direction, from parent to child.
>>
>> commands          | direction
>> hg diff           | parent -> child
>> hg diff -r (a)    | parent -> child
>> hg diff -r (b)    | child  -> parent <- one exception
>>
>
>  With this history:
> x---a---c
>       \
>       @---b
>
>  Currently, it behaves this way (with clean working copy):
>  hg diff -r a | diff from a to @
>  hg diff r b  | diff from b to @
> hg diff -r c | diff from c to @
>
>  I think you're suggesting:
>  hg diff -r a | diff from a to @
> hg diff r b  | diff from @ to b
> hg diff -r c | diff from @ to c
>
>  I fear that a "hg diff -r a" followed by a "hg diff -r c" would surprise
> people, because the hunks corresponding to changes between @ and 'a' would
> be reversed between the two invocations.
>
>
> I'm confused.  I'm not suggesting changing `hg diff` at all. I'm just
> pointing at how `hg diff` behave currently and how I think it is fine to
> have `hg revert` match this behavior.
>
> Did you misunderstood me?
>

Maybe I did misunderstand you. I thought you were arguing that we 'hg
revert -i -r $rev' should show the diff *from* $rev is $rev is an ancestor
(and answering 'yes' *drops* the hunk), but it should show the diff *to*
$rev if it is not an ancestor (and answering 'yes' *applies* the hunk).
Correct?
Jordi Gutiérrez Hermoso - May 26, 2015, 9:24 p.m.
As a data point on what other people are doing, Emacs's magit has the
following interface for git:

 * All changes to the working directory appear as diff that are about
   to be applied.
   
 * A hunk can be staged by selecting it and pressing "s".

 * A hunk can be deleted by selecting it and pressing "k".

There is never a reverse diff for applying or undoing changes. Diffs
are always changes potentially applied to the repo, never changes
potentially potentially applied to the working directory.

The most consistent approach to me would be that the record interface
for revert should ask "keep this change?" (instead of "apply this
change?"), and the curses interface should show everything as applied
and you pick the changes that you want to disappear (and save into
.orig files).
Pierre-Yves David - May 26, 2015, 9:35 p.m.
On 05/26/2015 02:02 PM, Martin von Zweigbergk wrote:
>
>
> On Tue, May 26, 2015 at 1:43 PM Pierre-Yves David <pyd@fb.com 
> <mailto:pyd@fb.com>> wrote:
>
>
>
>     On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
>>     On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David
>>     <pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>>
>>
>>
>>         On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>>         >
>>         >
>>         > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
>>         > <pierre-yves.david@ens-lyon.org
>>         <mailto:pierre-yves.david@ens-lyon.org>
>>         <mailto:pierre-yves.david@ens-lyon.org
>>         <mailto:pierre-yves.david@ens-lyon.org>>>
>>
>     (I'm also a bit curious about how often you need it a what you use
>     it for)
>
>
> I think I usually use when I have a branch and I think of an 
> alternative approach, so I start from scratch, but I still want some 
> pieces from the old branch (perhaps test cases).
And will you often use `-i` for that?

>
>
>>
>>         In the ancestor case (a), `hg diff --rev (a)` would be
>>         consistent with
>>         what solution (2) show during revert.
>>         In the descendant case (b), `hg diff --rev (b)` would be
>>         consistent with
>>         what solution (1) show during revert.
>>
>>         I've to admit that I barely use the (b) case anymore. I used
>>         to need it
>>         to rework stack of patch, but uncommit/fold/etc are fitting
>>         this need
>>         for me now. My biggest usage of `hg revert` is without a
>>         revision to
>>         clean up silly change (debug print) or against an ancestors
>>         to try to
>>         get something to work again.
>>
>>         The consistency argument is interesting here, let's widen it.
>>         The most
>>         important point to me is the consistency of the experience
>>         across all
>>         commands. Mercurial is a VCS, vcs are mostly about handling
>>         "changes"
>>         (hunks in this context). The vast majority of command present
>>         such
>>         change in the same direction, from parent to child.
>>
>>         commands          | direction
>>         hg diff           | parent -> child
>>         hg diff -r (a)    | parent -> child
>>         hg diff -r (b)    | child  -> parent <- one exception
>>
>>
>>     With this history:
>>     x---a---c
>>          \
>>           @---b
>>
>>     Currently, it behaves this way (with clean working copy):
>>     hg diff -r a | diff from a to @
>>     hg diff r b  | diff from b to @
>>     hg diff -r c | diff from c to @
>>
>>     I think you're suggesting:
>>     hg diff -r a | diff from a to @
>>     hg diff r b  | diff from @ to b
>>     hg diff -r c | diff from @ to c
>>
>>     I fear that a "hg diff -r a" followed by a "hg diff -r c" would
>>     surprise people, because the hunks corresponding to changes
>>     between @ and 'a' would be reversed between the two invocations.
>
>     I'm confused.  I'm not suggesting changing `hg diff` at all. I'm
>     just pointing at how `hg diff` behave currently and how I think it
>     is fine to have `hg revert` match this behavior.
>
>     Did you misunderstood me?
>
>
> Maybe I did misunderstand you. I thought you were arguing that we 'hg 
> revert -i -r $rev' should show the diff *from* $rev is $rev is an 
> ancestor (and answering 'yes' *drops* the hunk), but it should show 
> the diff *to* $rev if it is not an ancestor (and answering 'yes' 
> *applies* the hunk). Correct?

I'm saying:

- `hg revert -i --rev X` should show the same diff as `hg diff --rev X`
- `hg diff --rev X ` should stay unchanged

This mean "reverse change" when C is a "descendant" From there, it can 
be either:

- Drop the hunk [Yn]
- Keep this hunk [yN]

(also keep curse UI in mind)

This mean the "descendant" case show a strange hunk, but so does `hg 
diff` in this case.
Martin von Zweigbergk - May 26, 2015, 9:48 p.m.
On Tue, May 26, 2015 at 2:35 PM Pierre-Yves David <pyd@fb.com> wrote:

>
>
> On 05/26/2015 02:02 PM, Martin von Zweigbergk wrote:
>
>
>
> On Tue, May 26, 2015 at 1:43 PM Pierre-Yves David <pyd@fb.com> wrote:
>
>>
>>
>> On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
>>
>>  On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
>> pierre-yves.david@ens-lyon.org> wrote:
>>
>>>
>>>
>>> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>>> >
>>> >
>>> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
>>> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org
>>> >>
>>>
>>        (I'm also a bit curious about how often you need it a what you
>> use it for)
>>
>
>  I think I usually use when I have a branch and I think of an alternative
> approach, so I start from scratch, but I still want some pieces from the
> old branch (perhaps test cases).
>
>  And will you often use `-i` for that?
>

Is -i already supported? If it is, I probably will use it for that.


>
>
>
>
>>
>>
>>
>>> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
>>> what solution (2) show during revert.
>>> In the descendant case (b), `hg diff --rev (b)` would be consistent with
>>> what solution (1) show during revert.
>>>
>>> I've to admit that I barely use the (b) case anymore. I used to need it
>>> to rework stack of patch, but uncommit/fold/etc are fitting this need
>>> for me now. My biggest usage of `hg revert` is without a revision to
>>> clean up silly change (debug print) or against an ancestors to try to
>>> get something to work again.
>>>
>>> The consistency argument is interesting here, let's widen it. The most
>>> important point to me is the consistency of the experience across all
>>> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
>>> (hunks in this context). The vast majority of command present such
>>> change in the same direction, from parent to child.
>>>
>>> commands          | direction
>>> hg diff           | parent -> child
>>> hg diff -r (a)    | parent -> child
>>> hg diff -r (b)    | child  -> parent <- one exception
>>>
>>
>>  With this history:
>> x---a---c
>>       \
>>       @---b
>>
>>  Currently, it behaves this way (with clean working copy):
>>  hg diff -r a | diff from a to @
>>  hg diff r b  | diff from b to @
>> hg diff -r c | diff from c to @
>>
>>  I think you're suggesting:
>>  hg diff -r a | diff from a to @
>> hg diff r b  | diff from @ to b
>> hg diff -r c | diff from @ to c
>>
>>  I fear that a "hg diff -r a" followed by a "hg diff -r c" would
>> surprise people, because the hunks corresponding to changes between @ and
>> 'a' would be reversed between the two invocations.
>>
>>
>>  I'm confused.  I'm not suggesting changing `hg diff` at all. I'm just
>> pointing at how `hg diff` behave currently and how I think it is fine to
>> have `hg revert` match this behavior.
>>
>> Did you misunderstood me?
>>
>
>  Maybe I did misunderstand you. I thought you were arguing that we 'hg
> revert -i -r $rev' should show the diff *from* $rev is $rev is an ancestor
> (and answering 'yes' *drops* the hunk), but it should show the diff *to*
> $rev if it is not an ancestor (and answering 'yes' *applies* the hunk).
> Correct?
>
>
> I'm saying:
>
> - `hg revert -i --rev X` should show the same diff as `hg diff --rev X`
> - `hg diff --rev X ` should stay unchanged
>
> This mean "reverse change" when C is a "descendant" From there, it can be
> either:
>
> - Drop the hunk [Yn]
> - Keep this hunk [yN]
>

Okay, I see what you're saying (the same thing as you said from the
beginning, but I somehow thought you changed your mind a bit). In that
case, I maintain that I would be surprised if "hg revert --rev X" where X
is a sibling were to show me a diff from X to @. I would expect a diff from
@ to X.

I understand if the word 'revert' makes it sound like you're undoing
something, but when X is a sibling, that's not the case (at least for my
uses).


> (also keep curse UI in mind)
>
> This mean the "descendant" case show a strange hunk, but so does `hg diff`
> in this case.
>
>
> --
> Pierre-Yves David
>
>
>
>
Martin von Zweigbergk - May 26, 2015, 11:01 p.m.
Pierre-Yves and I spent some time talking about this on IRC. I see what I
was missing before. I agree with Pierre-Yves that it's most important to
get the common 'hg revert -i' case right to start with and we agree that it
should ask the user which hunks to discard, not which hunks to apply. It
would thus match the output of 'hg diff'.

The 'hg revert -i -r sibling' case is less important, but I care more about
this case than Pierre-Yves does. If we don't do anything special, it will
also ask the user which hunks to drop. I think this is backwards, even
though it's consistent with 'hg diff -r sibling'. Since it's an interactive
command, I think it's fine to make it less consistent and less surprising
by instead making it ask the user what hunks to apply. This can be done
later if someone cares enough. And since it's an interactive command, I
think it should be fine from a BC perspective to change it later.

Other thoughts?

On Tue, May 26, 2015 at 2:48 PM Martin von Zweigbergk <martinvonz@google.com>
wrote:

> On Tue, May 26, 2015 at 2:35 PM Pierre-Yves David <pyd@fb.com> wrote:
>
>>
>>
>> On 05/26/2015 02:02 PM, Martin von Zweigbergk wrote:
>>
>>
>>
>> On Tue, May 26, 2015 at 1:43 PM Pierre-Yves David <pyd@fb.com> wrote:
>>
>>>
>>>
>>> On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
>>>
>>>  On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
>>> pierre-yves.david@ens-lyon.org> wrote:
>>>
>>>>
>>>>
>>>> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
>>>> >
>>>> >
>>>> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
>>>> > <pierre-yves.david@ens-lyon.org <mailto:
>>>> pierre-yves.david@ens-lyon.org>>
>>>>
>>>        (I'm also a bit curious about how often you need it a what you
>>> use it for)
>>>
>>
>>  I think I usually use when I have a branch and I think of an
>> alternative approach, so I start from scratch, but I still want some pieces
>> from the old branch (perhaps test cases).
>>
>>  And will you often use `-i` for that?
>>
>
> Is -i already supported? If it is, I probably will use it for that.
>
>
>>
>>
>>
>>
>>>
>>>
>>>
>>>> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
>>>> what solution (2) show during revert.
>>>> In the descendant case (b), `hg diff --rev (b)` would be consistent with
>>>> what solution (1) show during revert.
>>>>
>>>> I've to admit that I barely use the (b) case anymore. I used to need it
>>>> to rework stack of patch, but uncommit/fold/etc are fitting this need
>>>> for me now. My biggest usage of `hg revert` is without a revision to
>>>> clean up silly change (debug print) or against an ancestors to try to
>>>> get something to work again.
>>>>
>>>> The consistency argument is interesting here, let's widen it. The most
>>>> important point to me is the consistency of the experience across all
>>>> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
>>>> (hunks in this context). The vast majority of command present such
>>>> change in the same direction, from parent to child.
>>>>
>>>> commands          | direction
>>>> hg diff           | parent -> child
>>>> hg diff -r (a)    | parent -> child
>>>> hg diff -r (b)    | child  -> parent <- one exception
>>>>
>>>
>>>  With this history:
>>> x---a---c
>>>       \
>>>       @---b
>>>
>>>  Currently, it behaves this way (with clean working copy):
>>>  hg diff -r a | diff from a to @
>>>  hg diff r b  | diff from b to @
>>> hg diff -r c | diff from c to @
>>>
>>>  I think you're suggesting:
>>>  hg diff -r a | diff from a to @
>>> hg diff r b  | diff from @ to b
>>> hg diff -r c | diff from @ to c
>>>
>>>  I fear that a "hg diff -r a" followed by a "hg diff -r c" would
>>> surprise people, because the hunks corresponding to changes between @ and
>>> 'a' would be reversed between the two invocations.
>>>
>>>
>>>  I'm confused.  I'm not suggesting changing `hg diff` at all. I'm just
>>> pointing at how `hg diff` behave currently and how I think it is fine to
>>> have `hg revert` match this behavior.
>>>
>>> Did you misunderstood me?
>>>
>>
>>  Maybe I did misunderstand you. I thought you were arguing that we 'hg
>> revert -i -r $rev' should show the diff *from* $rev is $rev is an ancestor
>> (and answering 'yes' *drops* the hunk), but it should show the diff *to*
>> $rev if it is not an ancestor (and answering 'yes' *applies* the hunk).
>> Correct?
>>
>>
>> I'm saying:
>>
>> - `hg revert -i --rev X` should show the same diff as `hg diff --rev X`
>> - `hg diff --rev X ` should stay unchanged
>>
>> This mean "reverse change" when C is a "descendant" From there, it can be
>> either:
>>
>> - Drop the hunk [Yn]
>> - Keep this hunk [yN]
>>
>
> Okay, I see what you're saying (the same thing as you said from the
> beginning, but I somehow thought you changed your mind a bit). In that
> case, I maintain that I would be surprised if "hg revert --rev X" where X
> is a sibling were to show me a diff from X to @. I would expect a diff from
> @ to X.
>
> I understand if the word 'revert' makes it sound like you're undoing
> something, but when X is a sibling, that's not the case (at least for my
> uses).
>
>
>> (also keep curse UI in mind)
>>
>> This mean the "descendant" case show a strange hunk, but so does `hg
>> diff` in this case.
>>
>>
>> --
>> Pierre-Yves David
>>
>>
>>
>>
Augie Fackler - June 1, 2015, 11:01 p.m.
On Tue, May 26, 2015 at 11:01:33PM +0000, Martin von Zweigbergk wrote:
> Pierre-Yves and I spent some time talking about this on IRC. I see what I
> was missing before. I agree with Pierre-Yves that it's most important to
> get the common 'hg revert -i' case right to start with and we agree that it
> should ask the user which hunks to discard, not which hunks to apply. It
> would thus match the output of 'hg diff'.
>

This seems like the obvious shape of the command to me, but I'd also
like to try it before feeling sure.

>
> The 'hg revert -i -r sibling' case is less important, but I care more about
> this case than Pierre-Yves does. If we don't do anything special, it will
> also ask the user which hunks to drop. I think this is backwards, even
> though it's consistent with 'hg diff -r sibling'. Since it's an interactive
> command, I think it's fine to make it less consistent and less surprising
> by instead making it ask the user what hunks to apply. This can be done
> later if someone cares enough. And since it's an interactive command, I
> think it should be fine from a BC perspective to change it later.
>
> Other thoughts?

I honestly have no idea how revert -ir should work. It never even
occurred to me that it could work differently. I'm not sure that the
output should be different from diff -r, but it's probably worth
testing.

Not sure how I feel about allowing BC in an interactive command,
mostly because of the command server (which I think makes it easy-ish
to wrap stuff like this).

Sorry for thread necromancy, last week was too-busy for me.

>
> On Tue, May 26, 2015 at 2:48 PM Martin von Zweigbergk <martinvonz@google.com>
> wrote:
>
> > On Tue, May 26, 2015 at 2:35 PM Pierre-Yves David <pyd@fb.com> wrote:
> >
> >>
> >>
> >> On 05/26/2015 02:02 PM, Martin von Zweigbergk wrote:
> >>
> >>
> >>
> >> On Tue, May 26, 2015 at 1:43 PM Pierre-Yves David <pyd@fb.com> wrote:
> >>
> >>>
> >>>
> >>> On 05/26/2015 01:25 PM, Martin von Zweigbergk wrote:
> >>>
> >>>  On Sun, May 24, 2015 at 3:08 PM Pierre-Yves David <
> >>> pierre-yves.david@ens-lyon.org> wrote:
> >>>
> >>>>
> >>>>
> >>>> On 05/22/2015 06:01 PM, Martin von Zweigbergk wrote:
> >>>> >
> >>>> >
> >>>> > On Fri, May 22, 2015 at 3:51 PM Pierre-Yves David
> >>>> > <pierre-yves.david@ens-lyon.org <mailto:
> >>>> pierre-yves.david@ens-lyon.org>>
> >>>>
> >>>        (I'm also a bit curious about how often you need it a what you
> >>> use it for)
> >>>
> >>
> >>  I think I usually use when I have a branch and I think of an
> >> alternative approach, so I start from scratch, but I still want some pieces
> >> from the old branch (perhaps test cases).
> >>
> >>  And will you often use `-i` for that?
> >>
> >
> > Is -i already supported? If it is, I probably will use it for that.
> >
> >
> >>
> >>
> >>
> >>
> >>>
> >>>
> >>>
> >>>> In the ancestor case (a), `hg diff --rev (a)` would be consistent with
> >>>> what solution (2) show during revert.
> >>>> In the descendant case (b), `hg diff --rev (b)` would be consistent with
> >>>> what solution (1) show during revert.
> >>>>
> >>>> I've to admit that I barely use the (b) case anymore. I used to need it
> >>>> to rework stack of patch, but uncommit/fold/etc are fitting this need
> >>>> for me now. My biggest usage of `hg revert` is without a revision to
> >>>> clean up silly change (debug print) or against an ancestors to try to
> >>>> get something to work again.
> >>>>
> >>>> The consistency argument is interesting here, let's widen it. The most
> >>>> important point to me is the consistency of the experience across all
> >>>> commands. Mercurial is a VCS, vcs are mostly about handling "changes"
> >>>> (hunks in this context). The vast majority of command present such
> >>>> change in the same direction, from parent to child.
> >>>>
> >>>> commands          | direction
> >>>> hg diff           | parent -> child
> >>>> hg diff -r (a)    | parent -> child
> >>>> hg diff -r (b)    | child  -> parent <- one exception
> >>>>
> >>>
> >>>  With this history:
> >>> x---a---c
> >>>       \
> >>>       @---b
> >>>
> >>>  Currently, it behaves this way (with clean working copy):
> >>>  hg diff -r a | diff from a to @
> >>>  hg diff r b  | diff from b to @
> >>> hg diff -r c | diff from c to @
> >>>
> >>>  I think you're suggesting:
> >>>  hg diff -r a | diff from a to @
> >>> hg diff r b  | diff from @ to b
> >>> hg diff -r c | diff from @ to c
> >>>
> >>>  I fear that a "hg diff -r a" followed by a "hg diff -r c" would
> >>> surprise people, because the hunks corresponding to changes between @ and
> >>> 'a' would be reversed between the two invocations.
> >>>
> >>>
> >>>  I'm confused.  I'm not suggesting changing `hg diff` at all. I'm just
> >>> pointing at how `hg diff` behave currently and how I think it is fine to
> >>> have `hg revert` match this behavior.
> >>>
> >>> Did you misunderstood me?
> >>>
> >>
> >>  Maybe I did misunderstand you. I thought you were arguing that we 'hg
> >> revert -i -r $rev' should show the diff *from* $rev is $rev is an ancestor
> >> (and answering 'yes' *drops* the hunk), but it should show the diff *to*
> >> $rev if it is not an ancestor (and answering 'yes' *applies* the hunk).
> >> Correct?
> >>
> >>
> >> I'm saying:
> >>
> >> - `hg revert -i --rev X` should show the same diff as `hg diff --rev X`
> >> - `hg diff --rev X ` should stay unchanged
> >>
> >> This mean "reverse change" when C is a "descendant" From there, it can be
> >> either:
> >>
> >> - Drop the hunk [Yn]
> >> - Keep this hunk [yN]
> >>
> >
> > Okay, I see what you're saying (the same thing as you said from the
> > beginning, but I somehow thought you changed your mind a bit). In that
> > case, I maintain that I would be surprised if "hg revert --rev X" where X
> > is a sibling were to show me a diff from X to @. I would expect a diff from
> > @ to X.
> >
> > I understand if the word 'revert' makes it sound like you're undoing
> > something, but when X is a sibling, that's not the case (at least for my
> > uses).
> >
> >
> >> (also keep curse UI in mind)
> >>
> >> This mean the "descendant" case show a strange hunk, but so does `hg
> >> diff` in this case.
> >>
> >>
> >> --
> >> Pierre-Yves David
> >>
> >>
> >>
> >>

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/x b/x
--- a/x
+++ b/x
@@ -0,0 +1,4 @@ 
+1
+2
+3
+4