Patchwork revert: warn of usage of --all and -i together

login
register
mail settings
Submitter Laurent Charignon
Date April 16, 2015, 9:34 p.m.
Message ID <ada0cc881c7c9c13b09f.1429220043@dev919.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8729/
State Rejected
Headers show

Comments

Laurent Charignon - April 16, 2015, 9:34 p.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1429210221 25200
#      Thu Apr 16 11:50:21 2015 -0700
# Node ID ada0cc881c7c9c13b09f00ffaa4a05968d67a43c
# Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
revert: warn of usage of --all and -i together

When using hg revert -i allows to select interactively the changes, if --all is
used in combination with -i, it is redundant and we should warn the user about
it.

Fix the test accordingly to:
- not use --all when it is not needed
- check that the warning is shown when using --all and -i together
Durham Goode - April 16, 2015, 9:44 p.m.
On 4/16/15 5:34 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1429210221 25200
> #      Thu Apr 16 11:50:21 2015 -0700
> # Node ID ada0cc881c7c9c13b09f00ffaa4a05968d67a43c
> # Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
> revert: warn of usage of --all and -i together
>
> When using hg revert -i allows to select interactively the changes, if --all is
> used in combination with -i, it is redundant and we should warn the user about
> it.
>
> Fix the test accordingly to:
> - not use --all when it is not needed
> - check that the warning is shown when using --all and -i together
>
If the result is the same either way, why bother warning the user about it?
Laurent Charignon - April 16, 2015, 10:07 p.m.
I thought it was useful so that the next time they type the command they
don¹t have to add -‹all and save time

On 4/16/15, 2:44 PM, "Durham Goode" <durham@fb.com> wrote:

>On 4/16/15 5:34 PM, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon@fb.com>
>> # Date 1429210221 25200
>> #      Thu Apr 16 11:50:21 2015 -0700
>> # Node ID ada0cc881c7c9c13b09f00ffaa4a05968d67a43c
>> # Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
>> revert: warn of usage of --all and -i together
>>
>> When using hg revert -i allows to select interactively the changes, if
>>--all is
>> used in combination with -i, it is redundant and we should warn the
>>user about
>> it.
>>
>> Fix the test accordingly to:
>> - not use --all when it is not needed
>> - check that the warning is shown when using --all and -i together
>>
>If the result is the same either way, why bother warning the user about
>it?
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel@selenic.com
>http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - April 16, 2015, 10:52 p.m.
On Thu, 2015-04-16 at 14:34 -0700, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1429210221 25200
> #      Thu Apr 16 11:50:21 2015 -0700
> # Node ID ada0cc881c7c9c13b09f00ffaa4a05968d67a43c
> # Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
> revert: warn of usage of --all and -i together

I don't think I'm going to take this one. We don't usually tell the user
when they're wasting keystrokes.
Laurent Charignon - April 17, 2015, 4:54 p.m.
Okay, got it.

Thanks,

Laurent

On 4/16/15, 3:52 PM, "Matt Mackall" <mpm@selenic.com> wrote:

>On Thu, 2015-04-16 at 14:34 -0700, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon@fb.com>
>> # Date 1429210221 25200
>> #      Thu Apr 16 11:50:21 2015 -0700
>> # Node ID ada0cc881c7c9c13b09f00ffaa4a05968d67a43c
>> # Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
>> revert: warn of usage of --all and -i together
>
>I don't think I'm going to take this one. We don't usually tell the user
>when they're wasting keystrokes.
>
>-- 
>Mathematics is the supreme nostalgia of our time.
>
>

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5500,6 +5500,11 @@ 
 
     Returns 0 on success.
     """
+    if opts.get('interactive') and opts.get('all'):
+        # Here we assume that the user did not mean all but only some of the
+        # changes as he uses the interactive flag
+        ui.warn(_("(--interactive works on all changes no need for --all)\n"))
+        opts.pop('all')
 
     if opts.get("date"):
         if opts.get("rev"):
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
@@ -48,6 +48,7 @@ 
   > n
   > n
   > EOF
+  (--interactive works on all changes no need for --all)
   reverting f
   reverting folder1/g (glob)
   removing folder1/i (glob)
@@ -138,7 +139,7 @@ 
 
   $ hg update -C 6
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  $ hg revert -i -r 2 --all -- << EOF
+  $ hg revert -i -r 2 -- << EOF
   > y
   > y
   > y