[PATCH] revert: do not reverse hunks in interactive when REV is not parent (issue5096)

Augie Fackler raf at durin42.com
Fri Nov 3 16:09:27 EDT 2017


Should we add a release notes directive like this:

.. feature::

  When interactive revert is run against a revision other than the working
  directory parent, the diff shown is the diff to *apply* to the working directory, 
  rather than the diff to *discard* from the working copy. This is in line with
  related user experiences with `git` and appears to be less confusing with
  `ui.interface=curses`.


The verb is significantly less prominent in the curses interface, which I don't use, so it might make sense do document this more heavily? I don't feel strongly about showing (revert, reverse diff) vs (apply, forward diff)[0], but it took several minutes discussion with Martin before I understood the intent of this patch again. What do you think? Should we add something to the help of `hg revert`?

Thanks,
Augie


0: My mental model has long been that the word "revert" means "what you see will go through patch -p1 --reverse", whereas apply means "what you see will go through patch -p1". I have no idea if that makes sense to anyone, but the prominence of the verb in the plain-text UI probably has been saving me from confusion a bit.

> On Nov 3, 2017, at 12:39, Martin von Zweigbergk via Mercurial-devel <mercurial-devel at mercurial-scm.org> wrote:
> 
> Queued, thanks!
> 
> On Fri, Nov 3, 2017 at 9:34 AM, Denis Laxalde <denis at laxalde.org <mailto:denis at laxalde.org>> wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde at logilab.fr <mailto:denis.laxalde at logilab.fr>>
> # Date 1509716857 -3600
> #      Fri Nov 03 14:47:37 2017 +0100
> # Node ID 095ef7f12cb36330d5e2b35b6c93fe27f6c1aacc
> # Parent  7ebf850d3166a64ff33b4b85adb481b533ddbf86
> # Available At http://hg.logilab.org/users/dlaxalde/hg <http://hg.logilab.org/users/dlaxalde/hg>
> #              hg pull http://hg.logilab.org/users/dlaxalde/hg <http://hg.logilab.org/users/dlaxalde/hg> -r 095ef7f12cb3
> # EXP-Topic revert-interactive
> revert: do not reverse hunks in interactive when REV is not parent (issue5096)
> 
> And introduce a new "apply" operation verb for this case as suggested in
> issue5096. This replaces the no longer used "revert" operation.
> 
> In interactive revert, when reverting to something else that the parent
> revision, display an "apply this change" message with a diff that is not
> reversed.
> 
> The rationale is that `hg revert -i -r REV` will show hunks of the diff from
> the working directory to REV and prompt the user to select them for applying
> (to working directory). This contradicts dcc56e10c23b in which it was
> decided to have the "direction" of prompted hunks reversed. Later on
> [1], there was a broad consensus (but no decision) towards the "as to
> be applied direction". Now that --interactive is no longer experimental
> (5910db5d1913), it's time to switch and thus we drop no longer used
> "experimental.revertalternateinteractivemode" configuration option.
> 
> [1]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090142.html <https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090142.html>
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -3790,9 +3790,8 @@ def _performrevert(repo, parents, ctx, a
>          operation = 'discard'
>          reversehunks = True
>          if node != parent:
> -            operation = 'revert'
> -            reversehunks = repo.ui.configbool('experimental',
> -                'revertalternateinteractivemode')
> +            operation = 'apply'
> +            reversehunks = False
>          if reversehunks:
>              diff = patch.diff(repo, ctx.node(), None, m, opts=diffopts)
>          else:
> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> --- a/mercurial/configitems.py
> +++ b/mercurial/configitems.py
> @@ -439,9 +439,6 @@ coreconfigitem('experimental', 'obsmarke
>  coreconfigitem('experimental', 'rebase.multidest',
>      default=False,
>  )
> -coreconfigitem('experimental', 'revertalternateinteractivemode',
> -    default=True,
> -)
>  coreconfigitem('experimental', 'revlogv2',
>      default=None,
>  )
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -997,16 +997,26 @@ class recordhunk(object):
>  def getmessages():
>      return {
>          'multiple': {
> +            'apply': _("apply change %d/%d to '%s'?"),
>              'discard': _("discard change %d/%d to '%s'?"),
>              'record': _("record change %d/%d to '%s'?"),
> -            'revert': _("revert change %d/%d to '%s'?"),
>          },
>          'single': {
> +            'apply': _("apply this change to '%s'?"),
>              'discard': _("discard this change to '%s'?"),
>              'record': _("record this change to '%s'?"),
> -            'revert': _("revert this change to '%s'?"),
>          },
>          'help': {
> +            'apply': _('[Ynesfdaq?]'
> +                         '$$ &Yes, apply this change'
> +                         '$$ &No, skip this change'
> +                         '$$ &Edit this change manually'
> +                         '$$ &Skip remaining changes to this file'
> +                         '$$ Apply remaining changes to this &file'
> +                         '$$ &Done, skip remaining changes and files'
> +                         '$$ Apply &all changes to all remaining files'
> +                         '$$ &Quit, applying no changes'
> +                         '$$ &? (display help)'),
>              'discard': _('[Ynesfdaq?]'
>                           '$$ &Yes, discard this change'
>                           '$$ &No, skip this change'
> @@ -1027,16 +1037,6 @@ def getmessages():
>                          '$$ Record &all changes to all remaining files'
>                          '$$ &Quit, recording no changes'
>                          '$$ &? (display help)'),
> -            'revert': _('[Ynesfdaq?]'
> -                        '$$ &Yes, revert this change'
> -                        '$$ &No, skip this change'
> -                        '$$ &Edit this change manually'
> -                        '$$ &Skip remaining changes to this file'
> -                        '$$ Revert remaining changes to this &file'
> -                        '$$ &Done, skip remaining changes and files'
> -                        '$$ Revert &all changes to all remaining files'
> -                        '$$ &Quit, reverting no changes'
> -                        '$$ &? (display help)')
>          }
>      }
> 
> 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
> @@ -60,56 +60,56 @@ 10 run the same test than 8 from within
>    2 hunks, 2 lines changed
>    examine changes to 'f'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +1,6 @@
> -  +a
> +  @@ -1,6 +1,5 @@
> +  -a
>     1
>     2
>     3
>     4
>     5
> -  revert change 1/6 to 'f'? [Ynesfdaq?] y
> +  apply change 1/6 to 'f'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +2,6 @@
> +  @@ -2,6 +1,5 @@
>     1
>     2
>     3
>     4
>     5
> -  +b
> -  revert change 2/6 to 'f'? [Ynesfdaq?] y
> +  -b
> +  apply change 2/6 to 'f'? [Ynesfdaq?] y
> 
>    diff --git a/folder1/g b/folder1/g
>    2 hunks, 2 lines changed
>    examine changes to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +1,6 @@
> -  +c
> +  @@ -1,6 +1,5 @@
> +  -c
>     1
>     2
>     3
>     4
>     5
> -  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] ?
> +  apply change 3/6 to 'folder1/g'? [Ynesfdaq?] ?
> 
> -  y - yes, revert this change
> +  y - yes, apply this change
>    n - no, skip this change
>    e - edit this change manually
>    s - skip remaining changes to this file
> -  f - revert remaining changes to this file
> +  f - apply remaining changes to this file
>    d - done, skip remaining changes and files
> -  a - revert all changes to all remaining files
> -  q - quit, reverting no changes
> +  a - apply all changes to all remaining files
> +  q - quit, applying no changes
>    ? - ? (display help)
> -  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> +  apply change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +2,6 @@
> +  @@ -2,6 +1,5 @@
>     1
>     2
>     3
>     4
>     5
> -  +d
> -  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> +  -d
> +  apply change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> 
>    diff --git a/folder2/h b/folder2/h
>    2 hunks, 2 lines changed
> @@ -157,12 +157,12 @@ Test that a noop revert doesn't do an un
>    1 hunks, 1 lines changed
>    examine changes to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -3,3 +3,4 @@
> +  @@ -3,4 +3,3 @@
>     3
>     4
>     5
> -  +d
> -  revert this change to 'folder1/g'? [Ynesfdaq?] n
> +  -d
> +  apply this change to 'folder1/g'? [Ynesfdaq?] n
> 
>    $ ls folder1/
>    g
> @@ -173,12 +173,12 @@ Test --no-backup
>    1 hunks, 1 lines changed
>    examine changes to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -3,3 +3,4 @@
> +  @@ -3,4 +3,3 @@
>     3
>     4
>     5
> -  +d
> -  revert this change to 'folder1/g'? [Ynesfdaq?] y
> +  -d
> +  apply this change to 'folder1/g'? [Ynesfdaq?] y
> 
>    $ ls folder1/
>    g
> @@ -206,45 +206,45 @@ Test --no-backup
>    2 hunks, 2 lines changed
>    examine changes to 'f'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +1,6 @@
> -  +a
> +  @@ -1,6 +1,5 @@
> +  -a
>     1
>     2
>     3
>     4
>     5
> -  revert change 1/6 to 'f'? [Ynesfdaq?] y
> +  apply change 1/6 to 'f'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +2,6 @@
> +  @@ -2,6 +1,5 @@
>     1
>     2
>     3
>     4
>     5
> -  +b
> -  revert change 2/6 to 'f'? [Ynesfdaq?] y
> +  -b
> +  apply change 2/6 to 'f'? [Ynesfdaq?] y
> 
>    diff --git a/folder1/g b/folder1/g
>    2 hunks, 2 lines changed
>    examine changes to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +1,6 @@
> -  +c
> +  @@ -1,6 +1,5 @@
> +  -c
>     1
>     2
>     3
>     4
>     5
> -  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> +  apply change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> 
> -  @@ -1,5 +2,6 @@
> +  @@ -2,6 +1,5 @@
>     1
>     2
>     3
>     4
>     5
> -  +d
> -  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> +  -d
> +  apply change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> 
>    diff --git a/folder2/h b/folder2/h
>    2 hunks, 2 lines changed
> @@ -368,77 +368,6 @@ 3) Use interactive revert with editing (
>    $ cat k
>    42
> 
> -Check the experimental config to invert the selection:
> -  $ cat <<EOF >> $HGRCPATH
> -  > [experimental]
> -  > revertalternateinteractivemode=False
> -  > EOF
> -
> -
> -  $ hg up -C .
> -  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  $ printf 'firstline\nc\n1\n2\n3\n 3\n5\nd\nlastline\n' > folder1/g
> -  $ hg diff --nodates
> -  diff -r a3d963a027aa folder1/g
> -  --- a/folder1/g
> -  +++ b/folder1/g
> -  @@ -1,7 +1,9 @@
> -  +firstline
> -   c
> -   1
> -   2
> -   3
> -  -4
> -  + 3
> -   5
> -   d
> -  +lastline
> -  $ hg revert -i <<EOF
> -  > y
> -  > y
> -  > y
> -  > n
> -  > EOF
> -  reverting folder1/g (glob)
> -  diff --git a/folder1/g b/folder1/g
> -  3 hunks, 3 lines changed
> -  examine changes to 'folder1/g'? [Ynesfdaq?] y
> -
> -  @@ -1,4 +1,5 @@
> -  +firstline
> -   c
> -   1
> -   2
> -   3
> -  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
> -
> -  @@ -1,7 +2,7 @@
> -   c
> -   1
> -   2
> -   3
> -  -4
> -  + 3
> -   5
> -   d
> -  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
> -
> -  @@ -6,2 +7,3 @@
> -   5
> -   d
> -  +lastline
> -  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
> -
> -  $ hg diff --nodates
> -  diff -r a3d963a027aa folder1/g
> -  --- a/folder1/g
> -  +++ b/folder1/g
> -  @@ -5,3 +5,4 @@
> -   4
> -   5
> -   d
> -  +lastline
> -
>    $ hg update -C .
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ hg purge
> @@ -463,11 +392,6 @@ Check the experimental config to invert
> 
>  When a line without EOL is selected during "revert -i" (issue5651)
> 
> -  $ cat <<EOF >> $HGRCPATH
> -  > [experimental]
> -  > %unset revertalternateinteractivemode
> -  > EOF
> -
>    $ hg init $TESTTMP/revert-i-eol
>    $ cd $TESTTMP/revert-i-eol
>    $ echo 0 > a
> @@ -487,11 +411,11 @@ When a line without EOL is selected duri
>    1 hunks, 1 lines changed
>    examine changes to 'a'? [Ynesfdaq?] y
> 
> -  @@ -1,1 +1,2 @@
> +  @@ -1,2 +1,1 @@
>     0
> -  +1
> +  -1
>    \ No newline at end of file
> -  revert this change to 'a'? [Ynesfdaq?] y
> +  apply this change to 'a'? [Ynesfdaq?] y
> 
>    $ cat a
>    0
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20171103/0b257851/attachment.html>


More information about the Mercurial-devel mailing list