Patchwork [2,of,6] shelve: refactor option combination check for adding options in the future

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 31, 2014, 3:26 p.m.
Message ID <83218ec9a3d9c1c9eb86.1401549972@feefifofum>
Download mbox | patch
Permalink /patch/4904/
State Changes Requested
Headers show

Comments

Katsunori FUJIWARA - May 31, 2014, 3:26 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1401548912 -32400
#      Sun Jun 01 00:08:32 2014 +0900
# Node ID 83218ec9a3d9c1c9eb86b098272a10ecf52bc8bb
# Parent  d383633a89d3f181c0b20ba84140fbef92f2c75f
shelve: refactor option combination check for adding options in the future

Option names as keys in 'allowableopts' are listed as same order as
option list for '@command' annotation of 'shelve' command, for
convenience of comparison.

'date' entry in 'allowableopts' is commented out, because it isn't
checked also before this patch: putting "[default] shelve = --date '0
0'" into hgrc for tests by "run-test.py" causes unexpected failure
even in ordinary ussage, if 'date' is also checked.
Pierre-Yves David - June 5, 2014, 12:46 a.m.
On 05/31/2014 08:26 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1401548912 -32400
> #      Sun Jun 01 00:08:32 2014 +0900
> # Node ID 83218ec9a3d9c1c9eb86b098272a10ecf52bc8bb
> # Parent  d383633a89d3f181c0b20ba84140fbef92f2c75f
> shelve: refactor option combination check for adding options in the future
>
> Option names as keys in 'allowableopts' are listed as same order as
> option list for '@command' annotation of 'shelve' command, for
> convenience of comparison.
>
> 'date' entry in 'allowableopts' is commented out, because it isn't
> checked also before this patch: putting "[default] shelve = --date '0
> 0'" into hgrc for tests by "run-test.py" causes unexpected failure
> even in ordinary ussage, if 'date' is also checked.

I've an hard time reading this description. I think I got that it mean. 
But I would like a native speaker to check if its me or the actual writing.

The change loks good otherwise. But it would be nice to have the new 
test introduced in a prior changeset to make sure we do not regress 
anything.


>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -675,20 +675,31 @@
>       '''
>       cmdutil.checkunfinished(repo)
>
> -    def checkopt(opt, incompatible):
> +    allowableopts = { # 'create' is pseudo option
> +        'addremove': 'create',
> +        'cleanup': 'cleanup',
> +#        'date': 'create', # ignored for '--date "0 0"' in default of tests
> +        'delete': 'delete',
> +        'list': 'list',
> +        'message': 'create',
> +        'name': 'create',
> +        'patch': 'list',
> +        'stat': 'list',
> +    }
> +    def checkopt(opt):
>           if opts[opt]:
> -            for i in incompatible.split():
> -                if opts[i]:
> +            for i, a in allowableopts.iteritems():
> +                if opts[i] and a != opt:
>                       raise util.Abort(_("options '--%s' and '--%s' may not be "
>                                          "used together") % (opt, i))
>               return True
> -    if checkopt('cleanup', 'addremove delete list message name patch stat'):
> +    if checkopt('cleanup'):
>           if pats:
>               raise util.Abort(_("cannot specify names when using '--cleanup'"))
>           return cleanupcmd(ui, repo)
> -    elif checkopt('delete', 'addremove cleanup list message name patch stat'):
> +    elif checkopt('delete'):
>           return deletecmd(ui, repo, pats)
> -    elif checkopt('list', 'addremove cleanup delete message name'):
> +    elif checkopt('list'):
>           return listcmd(ui, repo, pats, opts)
>       else:
>           for i in ('patch', 'stat'):
> diff --git a/tests/test-shelve.t b/tests/test-shelve.t
> --- a/tests/test-shelve.t
> +++ b/tests/test-shelve.t
> @@ -90,6 +90,10 @@
>      a
>     +a
>
> +  $ hg shelve --list --addremove
> +  abort: options '--list' and '--addremove' may not be used together
> +  [255]
> +
>   delete our older shelved change
>
>     $ hg shelve -d default
> @@ -395,6 +399,16 @@
>     $ hg shelve --cleanup
>     $ hg shelve --list
>
> +  $ hg shelve --cleanup --delete
> +  abort: options '--cleanup' and '--delete' may not be used together
> +  [255]
> +  $ hg shelve --cleanup --patch
> +  abort: options '--cleanup' and '--patch' may not be used together
> +  [255]
> +  $ hg shelve --cleanup --message MESSAGE
> +  abort: options '--cleanup' and '--message' may not be used together
> +  [255]
> +
>   test bookmarks
>
>     $ hg bookmark test
> @@ -664,4 +678,11 @@
>     g
>     $ hg shelve --delete default
>
> +  $ hg shelve --delete --stat
> +  abort: options '--delete' and '--stat' may not be used together
> +  [255]
> +  $ hg shelve --delete --name NAME
> +  abort: options '--delete' and '--name' may not be used together
> +  [255]
> +
>     $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -675,20 +675,31 @@ 
     '''
     cmdutil.checkunfinished(repo)
 
-    def checkopt(opt, incompatible):
+    allowableopts = { # 'create' is pseudo option
+        'addremove': 'create',
+        'cleanup': 'cleanup',
+#        'date': 'create', # ignored for '--date "0 0"' in default of tests
+        'delete': 'delete',
+        'list': 'list',
+        'message': 'create',
+        'name': 'create',
+        'patch': 'list',
+        'stat': 'list',
+    }
+    def checkopt(opt):
         if opts[opt]:
-            for i in incompatible.split():
-                if opts[i]:
+            for i, a in allowableopts.iteritems():
+                if opts[i] and a != opt:
                     raise util.Abort(_("options '--%s' and '--%s' may not be "
                                        "used together") % (opt, i))
             return True
-    if checkopt('cleanup', 'addremove delete list message name patch stat'):
+    if checkopt('cleanup'):
         if pats:
             raise util.Abort(_("cannot specify names when using '--cleanup'"))
         return cleanupcmd(ui, repo)
-    elif checkopt('delete', 'addremove cleanup list message name patch stat'):
+    elif checkopt('delete'):
         return deletecmd(ui, repo, pats)
-    elif checkopt('list', 'addremove cleanup delete message name'):
+    elif checkopt('list'):
         return listcmd(ui, repo, pats, opts)
     else:
         for i in ('patch', 'stat'):
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -90,6 +90,10 @@ 
    a
   +a
 
+  $ hg shelve --list --addremove
+  abort: options '--list' and '--addremove' may not be used together
+  [255]
+
 delete our older shelved change
 
   $ hg shelve -d default
@@ -395,6 +399,16 @@ 
   $ hg shelve --cleanup
   $ hg shelve --list
 
+  $ hg shelve --cleanup --delete
+  abort: options '--cleanup' and '--delete' may not be used together
+  [255]
+  $ hg shelve --cleanup --patch
+  abort: options '--cleanup' and '--patch' may not be used together
+  [255]
+  $ hg shelve --cleanup --message MESSAGE
+  abort: options '--cleanup' and '--message' may not be used together
+  [255]
+
 test bookmarks
 
   $ hg bookmark test
@@ -664,4 +678,11 @@ 
   g
   $ hg shelve --delete default
 
+  $ hg shelve --delete --stat
+  abort: options '--delete' and '--stat' may not be used together
+  [255]
+  $ hg shelve --delete --name NAME
+  abort: options '--delete' and '--name' may not be used together
+  [255]
+
   $ cd ..