add argparse conflict handler "ignore"

Update our version of ArgumentParser with a conflict resolution
handler called "ignore" to ignore options from commands if they would
conflict with options already registered. An error is reported if the
action associated with the option would not be registered at all
because all of its names conflict. A warning is reported for each
option string that is being ignored.

Change-Id: I99c62d5772017333136527f7f509c776623641a1
Signed-off-by: Doug Hellmann <doug@doughellmann.com>

3 files changed, 106 insertions, 5 deletions

diff --git a/cliff/_argparse.py b/cliff/_argparse.py
index e48dc79..5358e72 100644
--- a/cliff/_argparse.py
+++ b/cliff/_argparse.py
@@ -14,11 +14,14 @@
 
 from __future__ import absolute_import
 from argparse import *  # noqa
+import argparse
 import sys
+import warnings
 
 
-if sys.version_info < (3, 5):
-    class ArgumentParser(ArgumentParser):  # noqa
+class ArgumentParser(argparse.ArgumentParser):
+
+    if sys.version_info < (3, 5):
         def __init__(self, *args, **kwargs):
             self.allow_abbrev = kwargs.pop("allow_abbrev", True)
             super(ArgumentParser, self).__init__(*args, **kwargs)
@@ -28,3 +31,76 @@ if sys.version_info < (3, 5):
                 return super(ArgumentParser, self)._get_option_tuples(
                     option_string)
             return ()
+
+    # NOTE(dhellmann): We have to override the methods for creating
+    # groups to return our objects that know how to deal with the
+    # special conflict handler.
+
+    def add_argument_group(self, *args, **kwargs):
+        group = _ArgumentGroup(self, *args, **kwargs)
+        self._action_groups.append(group)
+        return group
+
+    def add_mutually_exclusive_group(self, **kwargs):
+        group = _MutuallyExclusiveGroup(self, **kwargs)
+        self._mutually_exclusive_groups.append(group)
+        return group
+
+    def _handle_conflict_ignore(self, action, conflicting_actions):
+        _handle_conflict_ignore(
+            self,
+            self._option_string_actions,
+            action,
+            conflicting_actions,
+        )
+
+
+def _handle_conflict_ignore(container, option_string_actions,
+                            new_action, conflicting_actions):
+
+    # Remember the option strings the new action starts with so we can
+    # restore them as part of error reporting if we need to.
+    original_option_strings = new_action.option_strings
+
+    # Remove all of the conflicting option strings from the new action
+    # and report an error if none are left at the end.
+    for option_string, action in conflicting_actions:
+
+        # remove the conflicting option from the new action
+        new_action.option_strings.remove(option_string)
+        warnings.warn(
+            ('Ignoring option string {} for new action '
+             'because it conflicts with an existing option.').format(
+                 option_string))
+
+        # if the option now has no option string, remove it from the
+        # container holding it
+        if not new_action.option_strings:
+            new_action.option_strings = original_option_strings
+            raise argparse.ArgumentError(
+                new_action,
+                ('Cannot resolve conflicting option string, '
+                 'all names conflict.'),
+            )
+
+
+class _ArgumentGroup(argparse._ArgumentGroup):
+
+    def _handle_conflict_ignore(self, action, conflicting_actions):
+        _handle_conflict_ignore(
+            self,
+            self._option_string_actions,
+            action,
+            conflicting_actions,
+        )
+
+
+class _MutuallyExclusiveGroup(argparse._MutuallyExclusiveGroup):
+
+    def _handle_conflict_ignore(self, action, conflicting_actions):
+        _handle_conflict_ignore(
+            self,
+            self._option_string_actions,
+            action,
+            conflicting_actions,
+        )
diff --git a/cliff/command.py b/cliff/command.py
index 859ac47..0a63f88 100644
--- a/cliff/command.py
+++ b/cliff/command.py
@@ -156,6 +156,7 @@ class Command(object):
             epilog=self.get_epilog(),
             prog=prog_name,
             formatter_class=_SmartHelpFormatter,
+            conflict_handler='ignore',
         )
         for hook in self._hooks:
             hook.obj.get_parser(parser)
diff --git a/cliff/tests/test_command.py b/cliff/tests/test_command.py
index 6aecff3..29c8c33 100644
--- a/cliff/tests/test_command.py
+++ b/cliff/tests/test_command.py
@@ -45,7 +45,9 @@ class TestCommand(command.Command):
         )
         parser.add_argument(
             '-z',
-            help='used in TestArgumentParser',
+            dest='zippy',
+            default='zippy-default',
+            help='defined in TestCommand and used in TestArgumentParser',
         )
         return parser
 
@@ -141,10 +143,32 @@ class TestArgumentParser(base.TestBase):
         cmd = TestCommand(None, None)
         parser = cmd.get_parser('NAME')
         # We should have an exception registering an option with a
-        # name that already exists because we do not want commands to
-        # override global options.
+        # name that already exists because we configure the argument
+        # parser to ignore conflicts but this option has no other name
+        # to be used.
         self.assertRaises(
             argparse.ArgumentError,
             parser.add_argument,
             '-z',
         )
+
+    def test_option_name_collision_with_alias(self):
+        cmd = TestCommand(None, None)
+        parser = cmd.get_parser('NAME')
+        # We not should have an exception registering an option with a
+        # name that already exists because we configure the argument
+        # parser to ignore conflicts and this option can be added as
+        # --zero even if the -z is ignored.
+        parser.add_argument('-z', '--zero')
+
+    def test_resolve_option_with_name_collision(self):
+        cmd = TestCommand(None, None)
+        parser = cmd.get_parser('NAME')
+        parser.add_argument(
+            '-z', '--zero',
+            dest='zero',
+            default='zero-default',
+        )
+        args = parser.parse_args(['-z', 'foo', 'a', 'b'])
+        self.assertEqual(args.zippy, 'foo')
+        self.assertEqual(args.zero, 'zero-default')