[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