[PATCH] revert: do not reverse hunks in interactive when REV is not parent (issue5096)
Martin von Zweigbergk
martinvonz at google.com
Fri Nov 3 16:33:35 EDT 2017
On Fri, Nov 3, 2017 at 1:09 PM, Augie Fackler <raf at durin42.com> wrote:
> 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`.
>
Thanks! I've amended the commit in the "committed" repo.
>
> 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`?
>
I personally don't even look at the verb. I just look at the patch and pick
which lines to apply. The direction is obvious from looking at the patch.
I'm fine with more documentation, of course (in a separate patch).
>
> 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> wrote:
>
>> # HG changeset patch
>> # User Denis Laxalde <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
>> # hg pull 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
>>
>> 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/90f90056/attachment.html>
More information about the Mercurial-devel
mailing list