Patchwork revert: use "discard"/"revert" verb when reverting interactively (issue5143)

login
register
mail settings
Submitter Denis Laxalde
Date June 3, 2016, 2:20 p.m.
Message ID <3157d25e58ac0c1648c0.1464963605@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/15381/
State Accepted
Headers show

Comments

Denis Laxalde - June 3, 2016, 2:20 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1464962107 -7200
#      Fri Jun 03 15:55:07 2016 +0200
# Node ID 3157d25e58ac0c1648c006336e808decb98e02b2
# Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
revert: use "discard"/"revert" verb when reverting interactively (issue5143)

Instead of "record this change to 'FILE'?" now prompt with:

* "discard this change to 'FILE'?" when reverting to the parent of working
  directory, and,
* "revert this change to 'FILE'?" otherwise.
Augie Fackler - June 4, 2016, 3:19 a.m.
On Fri, Jun 03, 2016 at 04:20:05PM +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1464962107 -7200
> #      Fri Jun 03 15:55:07 2016 +0200
> # Node ID 3157d25e58ac0c1648c006336e808decb98e02b2
> # Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
> revert: use "discard"/"revert" verb when reverting interactively (issue5143)

Huge fan. Queued.

>
> Instead of "record this change to 'FILE'?" now prompt with:
>
> * "discard this change to 'FILE'?" when reverting to the parent of working
>   directory, and,
> * "revert this change to 'FILE'?" otherwise.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -3301,10 +3301,12 @@ def _performrevert(repo, parents, ctx, a
>          else:
>              diff = patch.diff(repo, None, ctx.node(), m, opts=diffopts)
>          originalchunks = patch.parsepatch(diff)
> +        operation = _('discard') if node == parent else _('revert')
>
>          try:
>
> -            chunks, opts = recordfilter(repo.ui, originalchunks)
> +            chunks, opts = recordfilter(repo.ui, originalchunks,
> +                                        operation=operation)
>              if reversehunks:
>                  chunks = patch.reversehunks(chunks)
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1109,11 +1109,12 @@ the hunk is left unchanged.
>              if skipfile is None and skipall is None:
>                  chunk.pretty(ui)
>              if total == 1:
> -                msg = _("record this change to '%s'?") % chunk.filename()
> +                msg = _("%s this change to '%s'?") % (operation,
> +                                                      chunk.filename())
>              else:
>                  idx = pos - len(h.hunks) + i
> -                msg = _("record change %d/%d to '%s'?") % (idx, total,
> -                                                           chunk.filename())
> +                msg = _("%s change %d/%d to '%s'?") % (operation, idx, total,
> +                                                       chunk.filename())
>              r, skipfile, skipall, newpatches = prompt(skipfile,
>                      skipall, msg, chunk)
>              if r:
> 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
> @@ -64,7 +64,7 @@ 10 run the same test than 8 from within
>     3
>     4
>     5
> -  record change 1/6 to 'f'? [Ynesfdaq?] y
> +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>
>    @@ -1,5 +2,6 @@
>     1
> @@ -73,7 +73,7 @@ 10 run the same test than 8 from within
>     4
>     5
>    +b
> -  record change 2/6 to 'f'? [Ynesfdaq?] y
> +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>
>    diff --git a/folder1/g b/folder1/g
>    2 hunks, 2 lines changed
> @@ -86,7 +86,7 @@ 10 run the same test than 8 from within
>     3
>     4
>     5
> -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>
>    @@ -1,5 +2,6 @@
>     1
> @@ -95,7 +95,7 @@ 10 run the same test than 8 from within
>     4
>     5
>    +d
> -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>
>    diff --git a/folder2/h b/folder2/h
>    2 hunks, 2 lines changed
> @@ -163,7 +163,7 @@ Test that --interactive lift the need fo
>     3
>     4
>     5
> -  record change 1/6 to 'f'? [Ynesfdaq?] y
> +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>
>    @@ -1,5 +2,6 @@
>     1
> @@ -172,7 +172,7 @@ Test that --interactive lift the need fo
>     4
>     5
>    +b
> -  record change 2/6 to 'f'? [Ynesfdaq?] y
> +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>
>    diff --git a/folder1/g b/folder1/g
>    2 hunks, 2 lines changed
> @@ -185,7 +185,7 @@ Test that --interactive lift the need fo
>     3
>     4
>     5
> -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>
>    @@ -1,5 +2,6 @@
>     1
> @@ -194,7 +194,7 @@ Test that --interactive lift the need fo
>     4
>     5
>    +d
> -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>
>    diff --git a/folder2/h b/folder2/h
>    2 hunks, 2 lines changed
> @@ -242,7 +242,7 @@ Test that --interactive lift the need fo
>     3
>     4
>     5
> -  record change 1/2 to 'f'? [Ynesfdaq?] y
> +  discard change 1/2 to 'f'? [Ynesfdaq?] y
>
>    @@ -2,6 +1,5 @@
>     1
> @@ -251,7 +251,7 @@ Test that --interactive lift the need fo
>     4
>     5
>    -b
> -  record change 2/2 to 'f'? [Ynesfdaq?] n
> +  discard change 2/2 to 'f'? [Ynesfdaq?] n
>
>    $ hg st
>    M f
> @@ -303,7 +303,7 @@ 3) Use interactive revert with editing (
>    -1
>    +0
>    +2
> -  record this change to 'k'? [Ynesfdaq?] e
> +  discard this change to 'k'? [Ynesfdaq?] e
>
>    $ cat k
>    42
> @@ -350,7 +350,7 @@ Check the experimental config to invert
>     1
>     2
>     3
> -  record change 1/3 to 'folder1/g'? [Ynesfdaq?] y
> +  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>
>    @@ -2,7 +1,7 @@
>     c
> @@ -361,13 +361,13 @@ Check the experimental config to invert
>    +4
>     5
>     d
> -  record change 2/3 to 'folder1/g'? [Ynesfdaq?] y
> +  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>
>    @@ -7,3 +6,2 @@
>     5
>     d
>    -lastline
> -  record change 3/3 to 'folder1/g'? [Ynesfdaq?] n
> +  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>
>    $ hg diff --nodates
>    diff -r a3d963a027aa folder1/g
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
timeless - June 5, 2016, 5:36 p.m.
Fwiw localizers would typically object to this... It doesn't give them
enough flexibility of wording two distinct messages.
On Jun 3, 2016 11:19 PM, "Augie Fackler" <raf@durin42.com> wrote:

> On Fri, Jun 03, 2016 at 04:20:05PM +0200, Denis Laxalde wrote:
> > # HG changeset patch
> > # User Denis Laxalde <denis.laxalde@logilab.fr>
> > # Date 1464962107 -7200
> > #      Fri Jun 03 15:55:07 2016 +0200
> > # Node ID 3157d25e58ac0c1648c006336e808decb98e02b2
> > # Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
> > revert: use "discard"/"revert" verb when reverting interactively
> (issue5143)
>
> Huge fan. Queued.
>
> >
> > Instead of "record this change to 'FILE'?" now prompt with:
> >
> > * "discard this change to 'FILE'?" when reverting to the parent of
> working
> >   directory, and,
> > * "revert this change to 'FILE'?" otherwise.
> >
> > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> > --- a/mercurial/cmdutil.py
> > +++ b/mercurial/cmdutil.py
> > @@ -3301,10 +3301,12 @@ def _performrevert(repo, parents, ctx, a
> >          else:
> >              diff = patch.diff(repo, None, ctx.node(), m, opts=diffopts)
> >          originalchunks = patch.parsepatch(diff)
> > +        operation = _('discard') if node == parent else _('revert')
> >
> >          try:
> >
> > -            chunks, opts = recordfilter(repo.ui, originalchunks)
> > +            chunks, opts = recordfilter(repo.ui, originalchunks,
> > +                                        operation=operation)
> >              if reversehunks:
> >                  chunks = patch.reversehunks(chunks)
> >
> > diff --git a/mercurial/patch.py b/mercurial/patch.py
> > --- a/mercurial/patch.py
> > +++ b/mercurial/patch.py
> > @@ -1109,11 +1109,12 @@ the hunk is left unchanged.
> >              if skipfile is None and skipall is None:
> >                  chunk.pretty(ui)
> >              if total == 1:
> > -                msg = _("record this change to '%s'?") %
> chunk.filename()
> > +                msg = _("%s this change to '%s'?") % (operation,
> > +                                                      chunk.filename())
> >              else:
> >                  idx = pos - len(h.hunks) + i
> > -                msg = _("record change %d/%d to '%s'?") % (idx, total,
> > -
>  chunk.filename())
> > +                msg = _("%s change %d/%d to '%s'?") % (operation, idx,
> total,
> > +                                                       chunk.filename())
> >              r, skipfile, skipall, newpatches = prompt(skipfile,
> >                      skipall, msg, chunk)
> >              if r:
> > 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
> > @@ -64,7 +64,7 @@ 10 run the same test than 8 from within
> >     3
> >     4
> >     5
> > -  record change 1/6 to 'f'? [Ynesfdaq?] y
> > +  revert change 1/6 to 'f'? [Ynesfdaq?] y
> >
> >    @@ -1,5 +2,6 @@
> >     1
> > @@ -73,7 +73,7 @@ 10 run the same test than 8 from within
> >     4
> >     5
> >    +b
> > -  record change 2/6 to 'f'? [Ynesfdaq?] y
> > +  revert change 2/6 to 'f'? [Ynesfdaq?] y
> >
> >    diff --git a/folder1/g b/folder1/g
> >    2 hunks, 2 lines changed
> > @@ -86,7 +86,7 @@ 10 run the same test than 8 from within
> >     3
> >     4
> >     5
> > -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> > +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> >
> >    @@ -1,5 +2,6 @@
> >     1
> > @@ -95,7 +95,7 @@ 10 run the same test than 8 from within
> >     4
> >     5
> >    +d
> > -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> > +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> >
> >    diff --git a/folder2/h b/folder2/h
> >    2 hunks, 2 lines changed
> > @@ -163,7 +163,7 @@ Test that --interactive lift the need fo
> >     3
> >     4
> >     5
> > -  record change 1/6 to 'f'? [Ynesfdaq?] y
> > +  revert change 1/6 to 'f'? [Ynesfdaq?] y
> >
> >    @@ -1,5 +2,6 @@
> >     1
> > @@ -172,7 +172,7 @@ Test that --interactive lift the need fo
> >     4
> >     5
> >    +b
> > -  record change 2/6 to 'f'? [Ynesfdaq?] y
> > +  revert change 2/6 to 'f'? [Ynesfdaq?] y
> >
> >    diff --git a/folder1/g b/folder1/g
> >    2 hunks, 2 lines changed
> > @@ -185,7 +185,7 @@ Test that --interactive lift the need fo
> >     3
> >     4
> >     5
> > -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> > +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
> >
> >    @@ -1,5 +2,6 @@
> >     1
> > @@ -194,7 +194,7 @@ Test that --interactive lift the need fo
> >     4
> >     5
> >    +d
> > -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> > +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
> >
> >    diff --git a/folder2/h b/folder2/h
> >    2 hunks, 2 lines changed
> > @@ -242,7 +242,7 @@ Test that --interactive lift the need fo
> >     3
> >     4
> >     5
> > -  record change 1/2 to 'f'? [Ynesfdaq?] y
> > +  discard change 1/2 to 'f'? [Ynesfdaq?] y
> >
> >    @@ -2,6 +1,5 @@
> >     1
> > @@ -251,7 +251,7 @@ Test that --interactive lift the need fo
> >     4
> >     5
> >    -b
> > -  record change 2/2 to 'f'? [Ynesfdaq?] n
> > +  discard change 2/2 to 'f'? [Ynesfdaq?] n
> >
> >    $ hg st
> >    M f
> > @@ -303,7 +303,7 @@ 3) Use interactive revert with editing (
> >    -1
> >    +0
> >    +2
> > -  record this change to 'k'? [Ynesfdaq?] e
> > +  discard this change to 'k'? [Ynesfdaq?] e
> >
> >    $ cat k
> >    42
> > @@ -350,7 +350,7 @@ Check the experimental config to invert
> >     1
> >     2
> >     3
> > -  record change 1/3 to 'folder1/g'? [Ynesfdaq?] y
> > +  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
> >
> >    @@ -2,7 +1,7 @@
> >     c
> > @@ -361,13 +361,13 @@ Check the experimental config to invert
> >    +4
> >     5
> >     d
> > -  record change 2/3 to 'folder1/g'? [Ynesfdaq?] y
> > +  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
> >
> >    @@ -7,3 +6,2 @@
> >     5
> >     d
> >    -lastline
> > -  record change 3/3 to 'folder1/g'? [Ynesfdaq?] n
> > +  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
> >
> >    $ hg diff --nodates
> >    diff -r a3d963a027aa folder1/g
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - June 6, 2016, 5:04 a.m.
On 06/05/2016 07:36 PM, timeless wrote:
> Fwiw localizers would typically object to this... It doesn't give them
> enough flexibility of wording two distinct messages.

(Thanks for looking at this rephrasing, it was long overdue!)

However, I agree with timeless that we need two full messages here for
i18n purpose. That said? why do we need two messages at all? Could a
single one work?

> 
> On Jun 3, 2016 11:19 PM, "Augie Fackler" <raf@durin42.com
> <mailto:raf@durin42.com>> wrote:
> 
>     On Fri, Jun 03, 2016 at 04:20:05PM +0200, Denis Laxalde wrote:
>     > # HG changeset patch
>     > # User Denis Laxalde <denis.laxalde@logilab.fr
>     <mailto:denis.laxalde@logilab.fr>>
>     > # Date 1464962107 -7200
>     > #      Fri Jun 03 15:55:07 2016 +0200
>     > # Node ID 3157d25e58ac0c1648c006336e808decb98e02b2
>     > # Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
>     > revert: use "discard"/"revert" verb when reverting interactively
>     (issue5143)
> 
>     Huge fan. Queued.
> 
>     >
>     > Instead of "record this change to 'FILE'?" now prompt with:
>     >
>     > * "discard this change to 'FILE'?" when reverting to the parent of
>     working
>     >   directory, and,
>     > * "revert this change to 'FILE'?" otherwise.
>     >
>     > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>     > --- a/mercurial/cmdutil.py
>     > +++ b/mercurial/cmdutil.py
>     > @@ -3301,10 +3301,12 @@ def _performrevert(repo, parents, ctx, a
>     >          else:
>     >              diff = patch.diff(repo, None, ctx.node(), m,
>     opts=diffopts)
>     >          originalchunks = patch.parsepatch(diff)
>     > +        operation = _('discard') if node == parent else _('revert')
>     >
>     >          try:
>     >
>     > -            chunks, opts = recordfilter(repo.ui, originalchunks)
>     > +            chunks, opts = recordfilter(repo.ui, originalchunks,
>     > +                                        operation=operation)
>     >              if reversehunks:
>     >                  chunks = patch.reversehunks(chunks)
>     >
>     > diff --git a/mercurial/patch.py b/mercurial/patch.py
>     > --- a/mercurial/patch.py
>     > +++ b/mercurial/patch.py
>     > @@ -1109,11 +1109,12 @@ the hunk is left unchanged.
>     >              if skipfile is None and skipall is None:
>     >                  chunk.pretty(ui)
>     >              if total == 1:
>     > -                msg = _("record this change to '%s'?") %
>     chunk.filename()
>     > +                msg = _("%s this change to '%s'?") % (operation,
>     > +                                                     
>     chunk.filename())
>     >              else:
>     >                  idx = pos - len(h.hunks) + i
>     > -                msg = _("record change %d/%d to '%s'?") % (idx,
>     total,
>     > -                                                         
>      chunk.filename())
>     > +                msg = _("%s change %d/%d to '%s'?") % (operation,
>     idx, total,
>     > +                                                     
>      chunk.filename())
>     >              r, skipfile, skipall, newpatches = prompt(skipfile,
>     >                      skipall, msg, chunk)
>     >              if r:
>     > 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
>     > @@ -64,7 +64,7 @@ 10 run the same test than 8 from within
>     >     3
>     >     4
>     >     5
>     > -  record change 1/6 to 'f'? [Ynesfdaq?] y
>     > +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>     >
>     >    @@ -1,5 +2,6 @@
>     >     1
>     > @@ -73,7 +73,7 @@ 10 run the same test than 8 from within
>     >     4
>     >     5
>     >    +b
>     > -  record change 2/6 to 'f'? [Ynesfdaq?] y
>     > +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>     >
>     >    diff --git a/folder1/g b/folder1/g
>     >    2 hunks, 2 lines changed
>     > @@ -86,7 +86,7 @@ 10 run the same test than 8 from within
>     >     3
>     >     4
>     >     5
>     > -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>     > +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>     >
>     >    @@ -1,5 +2,6 @@
>     >     1
>     > @@ -95,7 +95,7 @@ 10 run the same test than 8 from within
>     >     4
>     >     5
>     >    +d
>     > -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>     > +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>     >
>     >    diff --git a/folder2/h b/folder2/h
>     >    2 hunks, 2 lines changed
>     > @@ -163,7 +163,7 @@ Test that --interactive lift the need fo
>     >     3
>     >     4
>     >     5
>     > -  record change 1/6 to 'f'? [Ynesfdaq?] y
>     > +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>     >
>     >    @@ -1,5 +2,6 @@
>     >     1
>     > @@ -172,7 +172,7 @@ Test that --interactive lift the need fo
>     >     4
>     >     5
>     >    +b
>     > -  record change 2/6 to 'f'? [Ynesfdaq?] y
>     > +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>     >
>     >    diff --git a/folder1/g b/folder1/g
>     >    2 hunks, 2 lines changed
>     > @@ -185,7 +185,7 @@ Test that --interactive lift the need fo
>     >     3
>     >     4
>     >     5
>     > -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>     > +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>     >
>     >    @@ -1,5 +2,6 @@
>     >     1
>     > @@ -194,7 +194,7 @@ Test that --interactive lift the need fo
>     >     4
>     >     5
>     >    +d
>     > -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>     > +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>     >
>     >    diff --git a/folder2/h b/folder2/h
>     >    2 hunks, 2 lines changed
>     > @@ -242,7 +242,7 @@ Test that --interactive lift the need fo
>     >     3
>     >     4
>     >     5
>     > -  record change 1/2 to 'f'? [Ynesfdaq?] y
>     > +  discard change 1/2 to 'f'? [Ynesfdaq?] y
>     >
>     >    @@ -2,6 +1,5 @@
>     >     1
>     > @@ -251,7 +251,7 @@ Test that --interactive lift the need fo
>     >     4
>     >     5
>     >    -b
>     > -  record change 2/2 to 'f'? [Ynesfdaq?] n
>     > +  discard change 2/2 to 'f'? [Ynesfdaq?] n
>     >
>     >    $ hg st
>     >    M f
>     > @@ -303,7 +303,7 @@ 3) Use interactive revert with editing (
>     >    -1
>     >    +0
>     >    +2
>     > -  record this change to 'k'? [Ynesfdaq?] e
>     > +  discard this change to 'k'? [Ynesfdaq?] e
>     >
>     >    $ cat k
>     >    42
>     > @@ -350,7 +350,7 @@ Check the experimental config to invert
>     >     1
>     >     2
>     >     3
>     > -  record change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>     > +  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>     >
>     >    @@ -2,7 +1,7 @@
>     >     c
>     > @@ -361,13 +361,13 @@ Check the experimental config to invert
>     >    +4
>     >     5
>     >     d
>     > -  record change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>     > +  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>     >
>     >    @@ -7,3 +6,2 @@
>     >     5
>     >     d
>     >    -lastline
>     > -  record change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>     > +  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>     >
>     >    $ hg diff --nodates
>     >    diff -r a3d963a027aa folder1/g
>     > _______________________________________________
>     > Mercurial-devel mailing list
>     > Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> 
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - June 6, 2016, 5:06 a.m.
> On Jun 6, 2016, at 1:04 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 06/05/2016 07:36 PM, timeless wrote:
>> Fwiw localizers would typically object to this... It doesn't give them
>> enough flexibility of wording two distinct messages.
> 
> (Thanks for looking at this rephrasing, it was long overdue!)
> 
> However, I agree with timeless that we need two full messages here for
> i18n purpose. That said? why do we need two messages at all? Could a
> single one work?

I think three messages (“record/discard/revert”) is better than one by a wide margin. I’m not sure how I feel about two (“record/revert”), but it’s still clearly an improvement over 3.8. I’d be delighted to see a followup that’ll make i18n happy. :)

> 
>> 
>> On Jun 3, 2016 11:19 PM, "Augie Fackler" <raf@durin42.com
>> <mailto:raf@durin42.com>> wrote:
>> 
>>    On Fri, Jun 03, 2016 at 04:20:05PM +0200, Denis Laxalde wrote:
>>> # HG changeset patch
>>> # User Denis Laxalde <denis.laxalde@logilab.fr
>>    <mailto:denis.laxalde@logilab.fr>>
>>> # Date 1464962107 -7200
>>> #      Fri Jun 03 15:55:07 2016 +0200
>>> # Node ID 3157d25e58ac0c1648c006336e808decb98e02b2
>>> # Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
>>> revert: use "discard"/"revert" verb when reverting interactively
>>    (issue5143)
>> 
>>    Huge fan. Queued.
>> 
>>> 
>>> Instead of "record this change to 'FILE'?" now prompt with:
>>> 
>>> * "discard this change to 'FILE'?" when reverting to the parent of
>>    working
>>>  directory, and,
>>> * "revert this change to 'FILE'?" otherwise.
>>> 
>>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>>> --- a/mercurial/cmdutil.py
>>> +++ b/mercurial/cmdutil.py
>>> @@ -3301,10 +3301,12 @@ def _performrevert(repo, parents, ctx, a
>>>         else:
>>>             diff = patch.diff(repo, None, ctx.node(), m,
>>    opts=diffopts)
>>>         originalchunks = patch.parsepatch(diff)
>>> +        operation = _('discard') if node == parent else _('revert')
>>> 
>>>         try:
>>> 
>>> -            chunks, opts = recordfilter(repo.ui, originalchunks)
>>> +            chunks, opts = recordfilter(repo.ui, originalchunks,
>>> +                                        operation=operation)
>>>             if reversehunks:
>>>                 chunks = patch.reversehunks(chunks)
>>> 
>>> diff --git a/mercurial/patch.py b/mercurial/patch.py
>>> --- a/mercurial/patch.py
>>> +++ b/mercurial/patch.py
>>> @@ -1109,11 +1109,12 @@ the hunk is left unchanged.
>>>             if skipfile is None and skipall is None:
>>>                 chunk.pretty(ui)
>>>             if total == 1:
>>> -                msg = _("record this change to '%s'?") %
>>    chunk.filename()
>>> +                msg = _("%s this change to '%s'?") % (operation,
>>> +
>>    chunk.filename())
>>>             else:
>>>                 idx = pos - len(h.hunks) + i
>>> -                msg = _("record change %d/%d to '%s'?") % (idx,
>>    total,
>>> -
>>     chunk.filename())
>>> +                msg = _("%s change %d/%d to '%s'?") % (operation,
>>    idx, total,
>>> +
>>     chunk.filename())
>>>             r, skipfile, skipall, newpatches = prompt(skipfile,
>>>                     skipall, msg, chunk)
>>>             if r:
>>> 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
>>> @@ -64,7 +64,7 @@ 10 run the same test than 8 from within
>>>    3
>>>    4
>>>    5
>>> -  record change 1/6 to 'f'? [Ynesfdaq?] y
>>> +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>>> 
>>>   @@ -1,5 +2,6 @@
>>>    1
>>> @@ -73,7 +73,7 @@ 10 run the same test than 8 from within
>>>    4
>>>    5
>>>   +b
>>> -  record change 2/6 to 'f'? [Ynesfdaq?] y
>>> +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>>> 
>>>   diff --git a/folder1/g b/folder1/g
>>>   2 hunks, 2 lines changed
>>> @@ -86,7 +86,7 @@ 10 run the same test than 8 from within
>>>    3
>>>    4
>>>    5
>>> -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>>> +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>>> 
>>>   @@ -1,5 +2,6 @@
>>>    1
>>> @@ -95,7 +95,7 @@ 10 run the same test than 8 from within
>>>    4
>>>    5
>>>   +d
>>> -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>>> +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>>> 
>>>   diff --git a/folder2/h b/folder2/h
>>>   2 hunks, 2 lines changed
>>> @@ -163,7 +163,7 @@ Test that --interactive lift the need fo
>>>    3
>>>    4
>>>    5
>>> -  record change 1/6 to 'f'? [Ynesfdaq?] y
>>> +  revert change 1/6 to 'f'? [Ynesfdaq?] y
>>> 
>>>   @@ -1,5 +2,6 @@
>>>    1
>>> @@ -172,7 +172,7 @@ Test that --interactive lift the need fo
>>>    4
>>>    5
>>>   +b
>>> -  record change 2/6 to 'f'? [Ynesfdaq?] y
>>> +  revert change 2/6 to 'f'? [Ynesfdaq?] y
>>> 
>>>   diff --git a/folder1/g b/folder1/g
>>>   2 hunks, 2 lines changed
>>> @@ -185,7 +185,7 @@ Test that --interactive lift the need fo
>>>    3
>>>    4
>>>    5
>>> -  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>>> +  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
>>> 
>>>   @@ -1,5 +2,6 @@
>>>    1
>>> @@ -194,7 +194,7 @@ Test that --interactive lift the need fo
>>>    4
>>>    5
>>>   +d
>>> -  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>>> +  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
>>> 
>>>   diff --git a/folder2/h b/folder2/h
>>>   2 hunks, 2 lines changed
>>> @@ -242,7 +242,7 @@ Test that --interactive lift the need fo
>>>    3
>>>    4
>>>    5
>>> -  record change 1/2 to 'f'? [Ynesfdaq?] y
>>> +  discard change 1/2 to 'f'? [Ynesfdaq?] y
>>> 
>>>   @@ -2,6 +1,5 @@
>>>    1
>>> @@ -251,7 +251,7 @@ Test that --interactive lift the need fo
>>>    4
>>>    5
>>>   -b
>>> -  record change 2/2 to 'f'? [Ynesfdaq?] n
>>> +  discard change 2/2 to 'f'? [Ynesfdaq?] n
>>> 
>>>   $ hg st
>>>   M f
>>> @@ -303,7 +303,7 @@ 3) Use interactive revert with editing (
>>>   -1
>>>   +0
>>>   +2
>>> -  record this change to 'k'? [Ynesfdaq?] e
>>> +  discard this change to 'k'? [Ynesfdaq?] e
>>> 
>>>   $ cat k
>>>   42
>>> @@ -350,7 +350,7 @@ Check the experimental config to invert
>>>    1
>>>    2
>>>    3
>>> -  record change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>>> +  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
>>> 
>>>   @@ -2,7 +1,7 @@
>>>    c
>>> @@ -361,13 +361,13 @@ Check the experimental config to invert
>>>   +4
>>>    5
>>>    d
>>> -  record change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>>> +  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
>>> 
>>>   @@ -7,3 +6,2 @@
>>>    5
>>>    d
>>>   -lastline
>>> -  record change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>>> +  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
>>> 
>>>   $ hg diff --nodates
>>>   diff -r a3d963a027aa folder1/g
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>    <mailto:Mercurial-devel@mercurial-scm.org>
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>    _______________________________________________
>>    Mercurial-devel mailing list
>>    Mercurial-devel@mercurial-scm.org
>>    <mailto:Mercurial-devel@mercurial-scm.org>
>>    https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>> 
>> 
>> 
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - June 6, 2016, 6:07 a.m.
On 06/06/2016 07:06 AM, Augie Fackler wrote:
> 
>> On Jun 6, 2016, at 1:04 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>>
>> On 06/05/2016 07:36 PM, timeless wrote:
>>> Fwiw localizers would typically object to this... It doesn't give them
>>> enough flexibility of wording two distinct messages.
>>
>> (Thanks for looking at this rephrasing, it was long overdue!)
>>
>> However, I agree with timeless that we need two full messages here for
>> i18n purpose. That said? why do we need two messages at all? Could a
>> single one work?
> 
> I think three messages (“record/discard/revert”) is better than one by a wide margin. I’m not sure how I feel about two (“record/revert”), but it’s still clearly an improvement over 3.8. I’d be delighted to see a followup that’ll make i18n happy. :)

Can you elaborate on the value on having "discard" distinct from "revert"?

Cheers
Denis Laxalde - June 6, 2016, 6:59 a.m.
Augie Fackler a écrit :
>>> Fwiw localizers would typically object to this... It doesn't give
>>> them enough flexibility of wording two distinct messages.
>>
>> (Thanks for looking at this rephrasing, it was long overdue!)
>>
>> However, I agree with timeless that we need two full messages here
>> for i18n purpose. That said? why do we need two messages at all?
>> Could a single one work?
>
> I think three messages (“record/discard/revert”) is better than one
> by a wide margin. I’m not sure how I feel about two
> (“record/revert”), but it’s still clearly an improvement over 3.8.
> I’d be delighted to see a followup that’ll make i18n happy. :)
>

I'm also not really satisfied by the "revert" verb, but it at least 
matches with the command name and it's also better than "record" given 
that the user is asked to apply a "reversed" diff (see also 
https://bz.mercurial-scm.org/show_bug.cgi?id=5096).

I'll be glad to update i18n once there's a consensus.
Denis Laxalde - June 6, 2016, 7:02 a.m.
Pierre-Yves David a écrit :
> Can you elaborate on the value on having "discard" distinct from
> "revert"?

Discard is used to make it clearer that one will loose some uncommitted 
change.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3301,10 +3301,12 @@  def _performrevert(repo, parents, ctx, a
         else:
             diff = patch.diff(repo, None, ctx.node(), m, opts=diffopts)
         originalchunks = patch.parsepatch(diff)
+        operation = _('discard') if node == parent else _('revert')
 
         try:
 
-            chunks, opts = recordfilter(repo.ui, originalchunks)
+            chunks, opts = recordfilter(repo.ui, originalchunks,
+                                        operation=operation)
             if reversehunks:
                 chunks = patch.reversehunks(chunks)
 
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1109,11 +1109,12 @@  the hunk is left unchanged.
             if skipfile is None and skipall is None:
                 chunk.pretty(ui)
             if total == 1:
-                msg = _("record this change to '%s'?") % chunk.filename()
+                msg = _("%s this change to '%s'?") % (operation,
+                                                      chunk.filename())
             else:
                 idx = pos - len(h.hunks) + i
-                msg = _("record change %d/%d to '%s'?") % (idx, total,
-                                                           chunk.filename())
+                msg = _("%s change %d/%d to '%s'?") % (operation, idx, total,
+                                                       chunk.filename())
             r, skipfile, skipall, newpatches = prompt(skipfile,
                     skipall, msg, chunk)
             if r:
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
@@ -64,7 +64,7 @@  10 run the same test than 8 from within 
    3
    4
    5
-  record change 1/6 to 'f'? [Ynesfdaq?] y
+  revert change 1/6 to 'f'? [Ynesfdaq?] y
   
   @@ -1,5 +2,6 @@
    1
@@ -73,7 +73,7 @@  10 run the same test than 8 from within 
    4
    5
   +b
-  record change 2/6 to 'f'? [Ynesfdaq?] y
+  revert change 2/6 to 'f'? [Ynesfdaq?] y
   
   diff --git a/folder1/g b/folder1/g
   2 hunks, 2 lines changed
@@ -86,7 +86,7 @@  10 run the same test than 8 from within 
    3
    4
    5
-  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
+  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
   
   @@ -1,5 +2,6 @@
    1
@@ -95,7 +95,7 @@  10 run the same test than 8 from within 
    4
    5
   +d
-  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
+  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
   
   diff --git a/folder2/h b/folder2/h
   2 hunks, 2 lines changed
@@ -163,7 +163,7 @@  Test that --interactive lift the need fo
    3
    4
    5
-  record change 1/6 to 'f'? [Ynesfdaq?] y
+  revert change 1/6 to 'f'? [Ynesfdaq?] y
   
   @@ -1,5 +2,6 @@
    1
@@ -172,7 +172,7 @@  Test that --interactive lift the need fo
    4
    5
   +b
-  record change 2/6 to 'f'? [Ynesfdaq?] y
+  revert change 2/6 to 'f'? [Ynesfdaq?] y
   
   diff --git a/folder1/g b/folder1/g
   2 hunks, 2 lines changed
@@ -185,7 +185,7 @@  Test that --interactive lift the need fo
    3
    4
    5
-  record change 3/6 to 'folder1/g'? [Ynesfdaq?] y
+  revert change 3/6 to 'folder1/g'? [Ynesfdaq?] y
   
   @@ -1,5 +2,6 @@
    1
@@ -194,7 +194,7 @@  Test that --interactive lift the need fo
    4
    5
   +d
-  record change 4/6 to 'folder1/g'? [Ynesfdaq?] n
+  revert change 4/6 to 'folder1/g'? [Ynesfdaq?] n
   
   diff --git a/folder2/h b/folder2/h
   2 hunks, 2 lines changed
@@ -242,7 +242,7 @@  Test that --interactive lift the need fo
    3
    4
    5
-  record change 1/2 to 'f'? [Ynesfdaq?] y
+  discard change 1/2 to 'f'? [Ynesfdaq?] y
   
   @@ -2,6 +1,5 @@
    1
@@ -251,7 +251,7 @@  Test that --interactive lift the need fo
    4
    5
   -b
-  record change 2/2 to 'f'? [Ynesfdaq?] n
+  discard change 2/2 to 'f'? [Ynesfdaq?] n
   
   $ hg st
   M f
@@ -303,7 +303,7 @@  3) Use interactive revert with editing (
   -1
   +0
   +2
-  record this change to 'k'? [Ynesfdaq?] e
+  discard this change to 'k'? [Ynesfdaq?] e
   
   $ cat k
   42
@@ -350,7 +350,7 @@  Check the experimental config to invert 
    1
    2
    3
-  record change 1/3 to 'folder1/g'? [Ynesfdaq?] y
+  discard change 1/3 to 'folder1/g'? [Ynesfdaq?] y
   
   @@ -2,7 +1,7 @@
    c
@@ -361,13 +361,13 @@  Check the experimental config to invert 
   +4
    5
    d
-  record change 2/3 to 'folder1/g'? [Ynesfdaq?] y
+  discard change 2/3 to 'folder1/g'? [Ynesfdaq?] y
   
   @@ -7,3 +6,2 @@
    5
    d
   -lastline
-  record change 3/3 to 'folder1/g'? [Ynesfdaq?] n
+  discard change 3/3 to 'folder1/g'? [Ynesfdaq?] n
   
   $ hg diff --nodates
   diff -r a3d963a027aa folder1/g