From patchwork Wed Jun 11 13:50:45 2014
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [1, of, 2,
 V4] shelve: refactor option combination check to easily add new ones
From: Katsunori FUJIWARA <foozy@lares.dti.ne.jp>
X-Patchwork-Id: 4973
Message-Id: <af2b2360b9dc87167b8f.1402494645@feefifofum>
To: mercurial-devel@selenic.com
Date: Wed, 11 Jun 2014 22:50:45 +0900

# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1402494363 -32400
#      Wed Jun 11 22:46:03 2014 +0900
# Node ID af2b2360b9dc87167b8ffe729342794b8acd115a
# Parent  0f73ed6293629f69aa2f01d8940e91faeded49ae
shelve: refactor option combination check to easily add new ones

Before this patch, the name of a newly added option had to be added
into each string that was passed to the 'checkopt()' internal
function: these are white-space-separated list of un-acceptable option
names.

This new option had to be added into multiple strings because each
option could belong to only one category of 'create', 'cleanup',
'delete' or 'list'.

In addition to this redundancy, each string passed to 'checkopt()' was
already too long to include a new one.

This patch refactors option combination check to make it easier to add
a new option in a subsequent patch.

Now, 'checkopt()' only takes one category ('cleanup', 'delete' or
'list'), and checks whether all explicitly activated options are
allowed by it or not (if specified category is activated in 'opts').

The 'date' entry is listed in 'allowableopts', but commented out,
because:

  - 'date' shouldn't be checked for test

    checking 'date' causes unexpected failure of 'test-shelve.t',
    because 'run-test.py' puts "[default] shelve = --date '0 0'" into
    hgrc.

  - explicitly listing it can advertise that ignoring it is intentional

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 passing '--date "0 0"' always in 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'):