Patchwork revert: always display hunks reversed when reverting to parent

# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1488805881 -3600
#      Mon Mar 06 14:11:21 2017 +0100
# Node ID 9f2b436197b02608e1546f57ac6426ec1427c7fd
# Parent  b4cd912d7704cd976e1bee3a3c927e0e578ec88f
# Available At http://hg.logilab.org/users/dlaxalde/hg
#              hg pull http://hg.logilab.org/users/dlaxalde/hg -r 9f2b436197b0
revert: always display hunks reversed when reverting to parent

When reverting to the parent of working directory, operation is "discard" so
we want hunks to be presented in the same order as the diff (i.e. "reversed").
So we do not query the experimental.revertalternateinteractivemode option in
this case and always set "reversehunks" to True.

On Mon, Mar 6, 2017 at 5:25 AM, Denis Laxalde <denis@laxalde.org> wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1488805881 -3600
> #      Mon Mar 06 14:11:21 2017 +0100
> # Node ID 9f2b436197b02608e1546f57ac6426ec1427c7fd
> # Parent  b4cd912d7704cd976e1bee3a3c927e0e578ec88f
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r 9f2b436197b0
> revert: always display hunks reversed when reverting to parent
>
> When reverting to the parent of working directory, operation is "discard" so
> we want hunks to be presented in the same order as the diff (i.e. "reversed").
> So we do not query the experimental.revertalternateinteractivemode option in
> this case and always set "reversehunks" to True.

It's too late in the evening for me to queue this now, but that makes
sense to me.

Relatedly, did you ever resend you patch from last fall that stopped
even reading from revertalternateinteractivemode or was that blocked
by some discussion taking place? I still like my idea of using a
different verb for the use case of copying the state from another
commit as opposed to dropping changes since another commit, but it
seems few (if any) others did. So, given that, and that the majority
seems to want the default of revertalternateinteractivemode to change
to False, I wonder if we shouldn't do that soon.

>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -3258,15 +3258,18 @@ def _performrevert(repo, parents, ctx, a
>          diffopts = patch.difffeatureopts(repo.ui, whitespace=True)
>          diffopts.nodates = True
>          diffopts.git = True
> -        reversehunks = repo.ui.configbool('experimental',
> -                                          'revertalternateinteractivemode',
> -                                          True)
> +        operation = 'discard'
> +        reversehunks = True
> +        if node != parent:
> +            operation = 'revert'
> +            reversehunks = repo.ui.configbool('experimental',
> +                                              'revertalternateinteractivemode',
> +                                              True)
>          if reversehunks:
>              diff = patch.diff(repo, ctx.node(), None, m, opts=diffopts)
>          else:
>              diff = patch.diff(repo, None, ctx.node(), m, opts=diffopts)
>          originalchunks = patch.parsepatch(diff)
> -        operation = 'discard' if node == parent else 'revert'
>
>          try:
>
> diff --git a/tests/test-revert-interactive.t b/tests/test-revert-interactive.t
> --- a/tests/test-revert-interactive.t
> +++ b/tests/test-revert-interactive.t
> @@ -380,29 +380,29 @@ Check the experimental config to invert
>    3 hunks, 3 lines changed
>    examine changes to 'folder1/g'? [Ynesfdaq?] y
>
> -  @@ -1,5 +1,4 @@
> -  -firstline
> +  @@ -1,4 +1,5 @@
> +  +firstline
>     c
>     1
>     2
>     3
>    discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>
> -  @@ -2,7 +1,7 @@
> +  @@ -1,7 +2,7 @@
>     c
>     1
>     2
>     3
> -  - 3
> -  +4
> +  -4
> +  + 3
>     5
>     d
>    discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>
> -  @@ -7,3 +6,2 @@
> +  @@ -6,2 +7,3 @@
>     5
>     d
> -  -lastline
> +  +lastline
>    discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>
>    $ hg diff --nodates
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Martin von Zweigbergk a écrit :
> Relatedly, did you ever resend you patch from last fall that stopped
> even reading from revertalternateinteractivemode or was that blocked
> by some discussion taking place?

No, I didn't resend it and there was no clear conclusion to the
discussion as far as I can tell.

> I still like my idea of using a
> different verb for the use case of copying the state from another
> commit as opposed to dropping changes since another commit, but it
> seems few (if any) others did.

In fact, I do like this over any other proposals (which, if I remember
correctly were either to use another command -- like graft -- with a new
option or have a way to reverse hunks interactively).

On Tue, Mar 07, 2017 at 12:18:14AM -0800, Martin von Zweigbergk via Mercurial-devel wrote:
> On Mon, Mar 6, 2017 at 5:25 AM, Denis Laxalde <denis@laxalde.org> wrote:
> > # HG changeset patch
> > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > # Date 1488805881 -3600
> > #      Mon Mar 06 14:11:21 2017 +0100
> > # Node ID 9f2b436197b02608e1546f57ac6426ec1427c7fd
> > # Parent  b4cd912d7704cd976e1bee3a3c927e0e578ec88f
> > # Available At http://hg.logilab.org/users/dlaxalde/hg
> > #              hg pull http://hg.logilab.org/users/dlaxalde/hg -r 9f2b436197b0
> > revert: always display hunks reversed when reverting to parent
> >
> > When reverting to the parent of working directory, operation is "discard" so
> > we want hunks to be presented in the same order as the diff (i.e. "reversed").
> > So we do not query the experimental.revertalternateinteractivemode option in
> > this case and always set "reversehunks" to True.
>
> It's too late in the evening for me to queue this now, but that makes
> sense to me.

I think this is obviously correct. Queued, thanks.