Fix nested argument groups with ignore conflict handler

Cliff provides an additional conflict handler for argparse - 'ignore'.
To make this work it subclasses the argparse argument container classes
and adds a handle_conflict_ignore method. It also overrides the group
creation methods to return a subclassed argument group or mutually
exclusive argument group objects. This works, until you add a nested
argument group, at which point a standard argument group object is
returned which does not support the ignore conflict handler.

This change fixes the issue by returning subclassed group objects for
nested groups.

Change-Id: I517c61f24ba6194ff6181e115a3a23adbce3ea53
Story: 2005891
Task: 33749

2 files changed, 72 insertions, 30 deletions

diff --git a/cliff/_argparse.py b/cliff/_argparse.py
index 1f1be0f..3fce01f 100644
--- a/cliff/_argparse.py
+++ b/cliff/_argparse.py
@@ -19,18 +19,7 @@ import sys
 import warnings
 
 
-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)
-
-        def _get_option_tuples(self, option_string):
-            if self.allow_abbrev:
-                return super(ArgumentParser, self)._get_option_tuples(
-                    option_string)
-            return ()
+class _ArgumentContainerMixIn(object):
 
     # NOTE(dhellmann): We have to override the methods for creating
     # groups to return our objects that know how to deal with the
@@ -55,6 +44,20 @@ class ArgumentParser(argparse.ArgumentParser):
         )
 
 
+class ArgumentParser(_ArgumentContainerMixIn, 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)
+
+        def _get_option_tuples(self, option_string):
+            if self.allow_abbrev:
+                return super(ArgumentParser, self)._get_option_tuples(
+                    option_string)
+            return ()
+
+
 def _handle_conflict_ignore(container, option_string_actions,
                             new_action, conflicting_actions):
 
@@ -84,26 +87,13 @@ def _handle_conflict_ignore(container, option_string_actions,
             )
 
 
-class _ArgumentGroup(argparse._ArgumentGroup):
-
-    def _handle_conflict_ignore(self, action, conflicting_actions):
-        _handle_conflict_ignore(
-            self,
-            self._option_string_actions,
-            action,
-            conflicting_actions,
-        )
-
+class _ArgumentGroup(_ArgumentContainerMixIn, argparse._ArgumentGroup):
+    pass
 
-class _MutuallyExclusiveGroup(argparse._MutuallyExclusiveGroup):
 
-    def _handle_conflict_ignore(self, action, conflicting_actions):
-        _handle_conflict_ignore(
-            self,
-            self._option_string_actions,
-            action,
-            conflicting_actions,
-        )
+class _MutuallyExclusiveGroup(_ArgumentContainerMixIn,
+                              argparse._MutuallyExclusiveGroup):
+    pass
 
 
 class SmartHelpFormatter(argparse.HelpFormatter):
diff --git a/cliff/tests/test__argparse.py b/cliff/tests/test__argparse.py
new file mode 100644
index 0000000..ab1e9b8
--- /dev/null
+++ b/cliff/tests/test__argparse.py
@@ -0,0 +1,52 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+#  Licensed under the Apache License, Version 2.0 (the "License"); you may
+#  not use this file except in compliance with the License. You may obtain
+#  a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#  License for the specific language governing permissions and limitations
+#  under the License.
+
+import unittest
+
+from cliff import _argparse
+
+
+class TestArgparse(unittest.TestCase):
+
+    def test_argument_parser(self):
+        _argparse.ArgumentParser(conflict_handler='ignore')
+
+    def test_argument_parser_add_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        parser.add_argument_group()
+
+    def test_argument_parser_add_mutually_exclusive_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        parser.add_mutually_exclusive_group()
+
+    def test_argument_parser_add_nested_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        group = parser.add_argument_group()
+        group.add_argument_group()
+
+    def test_argument_parser_add_nested_mutually_exclusive_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        group = parser.add_argument_group()
+        group.add_mutually_exclusive_group()
+
+    def test_argument_parser_add_mx_nested_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        group = parser.add_mutually_exclusive_group()
+        group.add_argument_group()
+
+    def test_argument_parser_add_mx_nested_mutually_exclusive_group(self):
+        parser = _argparse.ArgumentParser(conflict_handler='ignore')
+        group = parser.add_mutually_exclusive_group()
+        group.add_mutually_exclusive_group()