From patchwork Sat Jun  7 10:24:24 2014
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [1, of, 2,
 V3] shelve: refactor option combination check to easily add new ones
From: Katsunori FUJIWARA <foozy@lares.dti.ne.jp>
X-Patchwork-Id: 4946
Message-Id: <aad9814a936af230ab1c.1402136664@feefifofum>
To: mercurial-devel@selenic.com
Date: Sat, 07 Jun 2014 19:24:24 +0900

# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1402135841 -32400
#      Sat Jun 07 19:10:41 2014 +0900
# Node ID aad9814a936af230ab1c15d38a7d6371470bdbeb
# Parent  d602d23a97d7e294f47cefb1eac90f971496e76f
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 new one.

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

Now, 'checkopt()' only takes one category ('cleanup', 'delete' or
'list'), and checks whether the option allowed for other categories is
specified 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.

  - explicit 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'):