Merge lp:~jtv/maas/maascli-extract-parser into lp:maas/trunk

[Jeroen T. Vermeulen]

Extract the arguments-parsing part of maascli into its own module.

This was bloating the diff for my user-interface changes to maascli. In follow-up branches, the parser will learn a new --profile option; register API functions directly into its main namespace; and move the CLI management commands down into a new "cli" namespace.

Jeroen

[Gavin Panella (allenap)]

Looks good.

[1]

+def get_profile_option(argv):
...
+    specialized_parser = ArgumentParser(add_help=False)
+    specialized_parser.add_argument('--profile', metavar='PROFILE')
+    provisional_options = specialized_parser.parse_known_args(argv)[0]
+    return provisional_options.profile

This throws away the left-over args. I suspect you might want them
later on, otherwise you'll need to allow for parsing the --profile
option again.

[2]

+    module = __import__('maascli.api', fromlist=True)

This is a hang-over from ye olden days; you can probably just import
the module.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Yes, I'll probably want to get more cleverer about keeping the profile option. Haven't quite got that finalized yet, although I have a working prototype.

I fixed the __import__.

[lp:~jtv/maas/maascli-extract-parser]

1224. By Jeroen T. Vermeulen on 2012-10-08

Review suggestion: no need for __import__.

=== modified file 'src/maascli/__init__.py'
--- src/maascli/__init__.py	2012-10-08 08:55:15 +0000
+++ src/maascli/__init__.py	2012-10-08 16:03:32 +0000
@@ -14,41 +14,11 @@
     "main",
     ]
 
-import argparse
-from argparse import RawDescriptionHelpFormatter
 import locale
 import sys
 
 from bzrlib import osutils
-from maascli.api import register_api_commands
-from maascli.cli import register_cli_commands
-from maascli.utils import parse_docstring
-
-
-class ArgumentParser(argparse.ArgumentParser):
-    """Specialisation of argparse's parser with better support for subparsers.
-
-    Specifically, the one-shot `add_subparsers` call is disabled, replaced by
-    a lazily evaluated `subparsers` property.
-    """
-
-    def __init__(self, *args, **kwargs):
-        kwargs.setdefault("formatter_class", RawDescriptionHelpFormatter)
-        super(ArgumentParser, self).__init__(*args, **kwargs)
-
-    def add_subparsers(self):
-        raise NotImplementedError(
-            "add_subparsers has been disabled")
-
-    @property
-    def subparsers(self):
-        try:
-            return self.__subparsers
-        except AttributeError:
-            parent = super(ArgumentParser, self)
-            self.__subparsers = parent.add_subparsers(title="drill down")
-            self.__subparsers.metavar = "COMMAND"
-            return self.__subparsers
+from maascli.parser import prepare_parser
 
 
 def main(argv=None):
@@ -58,13 +28,7 @@
     if argv is None:
         argv = sys.argv[:1] + osutils.get_unicode_argv()
 
-    module = __import__('maascli.api', fromlist=True)
-    help_title, help_body = parse_docstring(module)
-    parser = ArgumentParser(
-        description=help_body, prog=argv[0],
-        epilog="http://maas.ubuntu.com/")
-    register_cli_commands(parser)
-    register_api_commands(parser)
+    parser = prepare_parser(argv)
 
     # Run, doing polite things with exceptions.
     try:

=== added file 'src/maascli/parser.py'
--- src/maascli/parser.py	1970-01-01 00:00:00 +0000
+++ src/maascli/parser.py	2012-10-08 16:03:32 +0000
@@ -0,0 +1,72 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Arguments parser for `maascli`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'prepare_parser',
+    ]
+
+import argparse
+
+from maascli import api
+from maascli.cli import register_cli_commands
+from maascli.utils import parse_docstring
+
+
+class ArgumentParser(argparse.ArgumentParser):
+    """Specialisation of argparse's parser with better support for subparsers.
+
+    Specifically, the one-shot `add_subparsers` call is disabled, replaced by
+    a lazily evaluated `subparsers` property.
+    """
+
+    def __init__(self, *args, **kwargs):
+        kwargs.setdefault(
+            "formatter_class", argparse.RawDescriptionHelpFormatter)
+        super(ArgumentParser, self).__init__(*args, **kwargs)
+
+    def add_subparsers(self):
+        raise NotImplementedError(
+            "add_subparsers has been disabled")
+
+    @property
+    def subparsers(self):
+        try:
+            return self.__subparsers
+        except AttributeError:
+            parent = super(ArgumentParser, self)
+            self.__subparsers = parent.add_subparsers(title="drill down")
+            self.__subparsers.metavar = "COMMAND"
+            return self.__subparsers
+
+
+def get_profile_option(argv):
+    """Parse the `--profile` option in `argv`; ignore the rest."""
+    # Create a specialized parser just to extract this one option.
+    # If we call parse_known_args on the real arguments parser, the
+    # --help option will do its work and cause the process to exit
+    # before we can even add the sub-parsers that the user may be asking
+    # for help about.
+    specialized_parser = ArgumentParser(add_help=False)
+    specialized_parser.add_argument('--profile', metavar='PROFILE')
+    provisional_options = specialized_parser.parse_known_args(argv)[0]
+    return provisional_options.profile
+
+
+def prepare_parser(argv):
+    """Create and populate an arguments parser for the maascli command."""
+    help_title, help_body = parse_docstring(api)
+    parser = ArgumentParser(
+        description=help_body, prog=argv[0],
+        epilog="http://maas.ubuntu.com/")
+    register_cli_commands(parser)
+    api.register_api_commands(parser)
+    return parser

=== modified file 'src/maascli/tests/test_api.py'
--- src/maascli/tests/test_api.py	2012-10-08 13:30:28 +0000
+++ src/maascli/tests/test_api.py	2012-10-08 16:03:32 +0000
@@ -18,12 +18,10 @@
 import json
 
 import httplib2
-from maascli import (
-    api,
-    ArgumentParser,
-    )
+from maascli import api
 from maascli.command import CommandError
 from maascli.config import ProfileConfig
+from maascli.parser import ArgumentParser
 from maascli.testing.config import make_configs
 from maascli.utils import (
     handler_command_name,

=== modified file 'src/maascli/tests/test_cli.py'
--- src/maascli/tests/test_cli.py	2012-10-08 06:30:25 +0000
+++ src/maascli/tests/test_cli.py	2012-10-08 16:03:32 +0000
@@ -12,10 +12,8 @@
 __metaclass__ = type
 __all__ = []
 
-from maascli import (
-    ArgumentParser,
-    cli,
-    )
+from maascli import cli
+from maascli.parser import ArgumentParser
 from maastesting.testcase import TestCase
 
 

=== renamed file 'src/maascli/tests/test_init.py' => 'src/maascli/tests/test_parser.py'
--- src/maascli/tests/test_init.py	2012-10-05 05:12:01 +0000
+++ src/maascli/tests/test_parser.py	2012-10-08 16:03:32 +0000
@@ -12,11 +12,16 @@
 __metaclass__ = type
 __all__ = []
 
-from maascli import ArgumentParser
+from maascli.parser import (
+    ArgumentParser,
+    get_profile_option,
+    )
+from maastesting.factory import factory
 from maastesting.testcase import TestCase
 
 
 class TestArgumentParser(TestCase):
+    """Tests for `ArgumentParser`."""
 
     def test_add_subparsers_disabled(self):
         parser = ArgumentParser()
@@ -36,3 +41,29 @@
         # The subparsers property, once populated, always returns the same
         # object.
         self.assertIs(subparsers, parser.subparsers)
+
+
+class TestGetProfileOption(TestCase):
+    """Tests for `get_profile_option`."""
+
+    def test_parses_profile_option(self):
+        profile = factory.make_name('profile')
+        self.assertEqual(profile, get_profile_option(['--profile', profile]))
+
+    def test_ignores_other_options(self):
+        profile = factory.make_name('profile')
+        self.assertEqual(
+            profile,
+            get_profile_option([
+                '--unrelated', 'option',
+                '--profile', profile,
+                factory.getRandomString(),
+                ]))
+
+    def test_ignores_help_option(self):
+        # This is a bit iffy: the most likely symptom if this fails is
+        # actually that the test process exits!
+        profile = factory.make_name('profile')
+        self.assertEqual(
+            profile,
+            get_profile_option(['--help', '--profile', profile]))