install: declare knob CLI names using the argparse convention

Replace cli_name, cli_short_name and cli_positional knob arguments with a
single cli_names argument, which allows defining one or more CLI names
using the argparse convention ("--option" for long option name, "-o" for
short option name and "argument" for positional argument name).

Also replace cli_aliases with cli_deprecated_names which uses the same
convention.

https://fedorahosted.org/freeipa/ticket/6392

Reviewed-By: Martin Basti <mbasti@redhat.com>

2 files changed, 109 insertions, 53 deletions

diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 65fc9529f..150d27f87 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -32,11 +32,11 @@ def _get_usage(configurable_class):
 
     for owner_cls, name in configurable_class.knobs():
         knob_cls = getattr(owner_cls, name)
-        if knob_cls.cli_positional:
+        if knob_cls.is_cli_positional():
             if knob_cls.cli_metavar is not None:
                 metavar = knob_cls.cli_metavar
-            elif knob_cls.cli_name is not None:
-                metavar = knob_cls.cli_name.upper()
+            elif knob_cls.cli_names:
+                metavar = knob_cls.cli_names[0].upper()
             else:
                 metavar = name.replace('_', '-').upper()
 
@@ -135,7 +135,7 @@ class ConfigureTool(admintool.AdminTool):
 
         for owner_cls, name in transformed_cls.knobs():
             knob_cls = getattr(owner_cls, name)
-            if knob_cls.cli_positional is not positional:
+            if knob_cls.is_cli_positional() is not positional:
                 continue
 
             group_cls = owner_cls.group()
@@ -198,27 +198,32 @@ class ConfigureTool(admintool.AdminTool):
             if knob_cls.cli_metavar:
                 kwargs['metavar'] = knob_cls.cli_metavar
 
-            if knob_cls.cli_short_name:
-                short_opt_str = '-{0}'.format(knob_cls.cli_short_name)
-            else:
-                short_opt_str = ''
-            cli_name = knob_cls.cli_name or name.replace('_', '-')
-            opt_str = '--{0}'.format(cli_name)
-            if not knob_cls.deprecated:
-                help = knob_cls.description
+            if not positional:
+                cli_info = (
+                    (knob_cls.deprecated, knob_cls.cli_names),
+                    (True, knob_cls.cli_deprecated_names),
+                )
             else:
-                help = optparse.SUPPRESS_HELP
-            opt_group.add_option(
-                short_opt_str, opt_str,
-                help=help,
-                **kwargs
-            )
+                cli_info = (
+                    (knob_cls.deprecated, (None,)),
+                )
+            for hidden, cli_names in cli_info:
+                opt_strs = []
+                for cli_name in cli_names:
+                    if cli_name is None:
+                        cli_name = '--{}'.format(name.replace('_', '-'))
+                    opt_strs.append(cli_name)
+                if not opt_strs:
+                    continue
+
+                if not hidden:
+                    help = knob_cls.description
+                else:
+                    help = optparse.SUPPRESS_HELP
 
-            if knob_cls.cli_aliases:
-                opt_strs = ['--{0}'.format(a) for a in knob_cls.cli_aliases]
                 opt_group.add_option(
                     *opt_strs,
-                    help=optparse.SUPPRESS_HELP,
+                    help=help,
                     **kwargs
                 )
 
@@ -236,15 +241,17 @@ class ConfigureTool(admintool.AdminTool):
 
         for owner_cls, name in self.transformed_cls.knobs():
             knob_cls = getattr(owner_cls, name)
-            if knob_cls.cli_positional:
+            if knob_cls.is_cli_positional():
                 self.positional_arguments.append(name)
 
-        parser = optparse.OptionParser()
-        self.add_options(parser, True)
+        # fake option parser to parse positional arguments
+        # (because optparse does not support positional argument parsing)
+        fake_option_parser = optparse.OptionParser()
+        self.add_options(fake_option_parser, True)
 
-        option_map = {option.dest: option
-                      for group in parser.option_groups
-                      for option in group.option_list}
+        fake_option_map = {option.dest: option
+                           for group in fake_option_parser.option_groups
+                           for option in group.option_list}
 
         for index, name in enumerate(self.positional_arguments):
             try:
@@ -252,11 +259,11 @@ class ConfigureTool(admintool.AdminTool):
             except IndexError:
                 break
 
-            option = option_map[name]
-            option.process('argument {}'.format(index + 1),
-                           value,
-                           self.options,
-                           self.option_parser)
+            fake_option = fake_option_map[name]
+            fake_option.process('argument {}'.format(index + 1),
+                                value,
+                                self.options,
+                                self.option_parser)
 
     def validate_options(self, needs_root=True):
         super(ConfigureTool, self).validate_options(needs_root=needs_root)
@@ -300,7 +307,7 @@ class ConfigureTool(admintool.AdminTool):
             try:
                 index = self.positional_arguments.index(e.name)
             except ValueError:
-                cli_name = knob_cls.cli_name or e.name.replace('_', '-')
+                cli_name = knob_cls.cli_names[0] or e.name.replace('_', '-')
                 desc = "option --{0}".format(cli_name)
             else:
                 desc = "argument {0}".format(index + 1)
diff --git a/ipapython/install/core.py b/ipapython/install/core.py
index 111b71090..338a0aa37 100644
--- a/ipapython/install/core.py
+++ b/ipapython/install/core.py
@@ -115,10 +115,8 @@ class KnobBase(PropertyBase):
     sensitive = False
     deprecated = False
     description = None
-    cli_positional = False
-    cli_name = None
-    cli_short_name = None
-    cli_aliases = None
+    cli_names = (None,)
+    cli_deprecated_names = ()
     cli_metavar = None
 
     def __init__(self, outer):
@@ -128,6 +126,11 @@ class KnobBase(PropertyBase):
         pass
 
     @classmethod
+    def is_cli_positional(cls):
+        return all(n is not None and not n.startswith('-')
+                   for n in cls.cli_names)
+
+    @classmethod
     def default_getter(cls, func):
         @property
         def default(self):
@@ -147,12 +150,21 @@ class KnobBase(PropertyBase):
 
 
 def knob(type_or_base, default=_missing, sensitive=_missing,
-         deprecated=_missing, description=_missing, cli_positional=_missing,
-         cli_name=_missing, cli_short_name=_missing, cli_aliases=_missing,
-         cli_metavar=_missing):
+         deprecated=_missing, description=_missing, cli_names=_missing,
+         cli_deprecated_names=_missing, cli_metavar=_missing):
     if type_or_base is None:
         type_or_base = NoneType
 
+    if cli_names is None or isinstance(cli_names, str):
+        cli_names = (cli_names,)
+    elif cli_names is not _missing:
+        cli_names = tuple(cli_names)
+
+    if isinstance(cli_deprecated_names, str):
+        cli_deprecated_names = (cli_deprecated_names,)
+    elif cli_deprecated_names is not _missing:
+        cli_deprecated_names = tuple(cli_deprecated_names)
+
     assert isinstance(type_or_base, type)
 
     class_dict = {}
@@ -170,14 +182,10 @@ def knob(type_or_base, default=_missing, sensitive=_missing,
         class_dict['deprecated'] = deprecated
     if description is not _missing:
         class_dict['description'] = description
-    if cli_positional is not _missing:
-        class_dict['cli_positional'] = cli_positional
-    if cli_name is not _missing:
-        class_dict['cli_name'] = cli_name
-    if cli_short_name is not _missing:
-        class_dict['cli_short_name'] = cli_short_name
-    if cli_aliases is not _missing:
-        class_dict['cli_aliases'] = cli_aliases
+    if cli_names is not _missing:
+        class_dict['cli_names'] = cli_names
+    if cli_deprecated_names is not _missing:
+        class_dict['cli_deprecated_names'] = cli_deprecated_names
     if cli_metavar is not _missing:
         class_dict['cli_metavar'] = cli_metavar
 
@@ -185,7 +193,7 @@ def knob(type_or_base, default=_missing, sensitive=_missing,
 
 
 def Knob(type_or_base, default=_missing, sensitive=_missing,
-         deprecated=_missing, description=_missing, cli_positional=_missing,
+         deprecated=_missing, description=_missing, cli_positional=False,
          cli_name=_missing, cli_short_name=_missing, cli_aliases=_missing,
          cli_metavar=_missing):
     if isinstance(type_or_base, tuple):
@@ -209,15 +217,56 @@ def Knob(type_or_base, default=_missing, sensitive=_missing,
     else:
         type_or_base = scalar_type
 
+    if cli_name is not _missing or cli_short_name is not _missing:
+        if cli_positional and cli_name is not _missing:
+            cli_positional_name = [cli_name]
+        elif issubclass(type_or_base, KnobBase):
+            cli_positional_name = [
+                n for n in type_or_base.cli_names   # pylint: disable=no-member
+                if n is not None and n[:1] != '-'
+            ]
+        else:
+            cli_positional_name = []
+
+        if cli_short_name is not _missing:
+            cli_short_name = ['-{}'.format(cli_short_name)]
+        elif issubclass(type_or_base, KnobBase):
+            cli_short_name = [
+                n for n in type_or_base.cli_names   # pylint: disable=no-member
+                if n is not None and n[:1] == '-' and n[:2] != '--'
+            ]
+        else:
+            cli_short_name = []
+
+        if not cli_positional:
+            if cli_name is not _missing:
+                cli_long_name = ['--{}'.format(cli_name)]
+            else:
+                cli_long_name = [None]
+        elif issubclass(type_or_base, KnobBase):
+            cli_long_name = [
+                n for n in type_or_base.cli_names   # pylint: disable=no-member
+                if n is None or n[:2] == '--'
+            ]
+        else:
+            cli_long_name = []
+
+        cli_names = cli_positional_name + cli_short_name + cli_long_name
+    else:
+        cli_names = _missing
+
+    if cli_aliases is not _missing:
+        cli_deprecated_names = ['--{}'.format(n) for n in cli_aliases]
+    else:
+        cli_deprecated_names = _missing
+
     return knob(type_or_base,
                 default=default,
                 sensitive=sensitive,
                 deprecated=deprecated,
                 description=description,
-                cli_positional=cli_positional,
-                cli_name=cli_name,
-                cli_short_name=cli_short_name,
-                cli_aliases=cli_aliases,
+                cli_names=cli_names,
+                cli_deprecated_names=cli_deprecated_names,
                 cli_metavar=cli_metavar)