[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