From 043c262ce48a0d667e914c315e21e6e1b3862202 Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: Nov 11 2016 11:17:25 +0000 Subject: 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 --- diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py index 65fc952..150d27f 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 111b710..338a0aa 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)