Merge pull request #3254 from certbot/multi-topic-help

Multi-topic help listings

certbot-apache/certbot_apache/configurator.py

@@ -84,7 +84,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     """
 
-    description = "Apache Web Server - Alpha"
+    description = "Apache Web Server plugin - Beta"
 
     @classmethod
     def add_parser_arguments(cls, add):

certbot/cli.py

@@ -88,8 +88,8 @@ More detailed help:
                         the available topics are:
 
    all, automation, paths, security, testing, or any of the subcommands or
-   plugins (certonly, install, register, nginx, apache, standalone, webroot,
-   etc.)
+   plugins (certonly, renew, install, register, nginx, apache, standalone,
+   webroot, etc.)
 """
 
 
@@ -393,8 +393,7 @@ class HelpfulArgumentParser(object):
                 if getattr(parsed_args, arg):
                     raise errors.Error(
                         ("Conflicting values for displayer."
-                        " {0} conflicts with dialog_mode").format(arg)
-                    )
+                        " {0} conflicts with dialog_mode").format(arg))
         elif parsed_args.verbose_count > flag_default("verbose_count"):
             parsed_args.text_mode = True
 
@@ -502,16 +501,25 @@ class HelpfulArgumentParser(object):
             pass
         return True
 
-    def add(self, topic, *args, **kwargs):
+    def add(self, topics, *args, **kwargs):
         """Add a new command line argument.
 
-        :param str: help topic this should be listed under, can be None for
-                    "always documented"
+        :param topics: str or [str] help topic(s) this should be listed under,
+                       or None for "always documented". The first entry
+                       determines where the flag lives in the "--help all"
+                       output (None -> "optional arguments").
         :param list *args: the names of this argument flag
         :param dict **kwargs: various argparse settings for this argument
 
         """
 
+        if isinstance(topics, list):
+            # if this flag can be listed in multiple sections, try to pick the one
+            # that the user has asked for help about
+            topic = self.help_arg if self.help_arg in topics else topics[0]
+        else:
+            topic = topics  # there's only one
+
         if self.detect_defaults:
             kwargs = self.modify_kwargs_for_default_detection(**kwargs)
 
@@ -612,6 +620,36 @@ class HelpfulArgumentParser(object):
         else:
             return dict([(t, t == chosen_topic) for t in self.help_topics])
 
+def _add_all_groups(helpful):
+    helpful.add_group("automation", description="Arguments for automating execution & other tweaks")
+    helpful.add_group("security", description="Security parameters & server settings")
+    helpful.add_group(
+        "testing", description="The following flags are meant for "
+        "testing purposes only! Do NOT change them, unless you "
+        "really know what you're doing!")
+    # VERBS
+    helpful.add_group(
+        "renew", description="The 'renew' subcommand will attempt to renew all"
+        " certificates (or more precisely, certificate lineages) you have"
+        " previously obtained if they are close to expiry, and print a"
+        " summary of the results. By default, 'renew' will reuse the options"
+        " used to create obtain or most recently successfully renew each"
+        " certificate lineage. You can try it with `--dry-run` first. For"
+        " more fine-grained control, you can renew individual lineages with"
+        " the `certonly` subcommand. Hooks are available to run commands"
+        " before and after renewal; see"
+        " https://certbot.eff.org/docs/using.html#renewal for more"
+        " information on these.")
+
+    helpful.add_group("certonly", description="Options for modifying how a cert is obtained")
+    helpful.add_group("install", description="Options for modifying how a cert is deployed")
+    helpful.add_group("revoke", description="Options for revocation of certs")
+    helpful.add_group("rollback", description="Options for reverting config changes")
+    helpful.add_group("plugins", description='Options for the "plugins" subcommand')
+    helpful.add_group("config_changes",
+                      description="Options for showing a history of config changes")
+    helpful.add_group("paths", description="Arguments changing execution paths & servers")
+
 
 def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: disable=too-many-statements
     """Returns parsed command line arguments.
@@ -627,6 +665,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     # pylint: disable=too-many-statements
 
     helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
+    _add_all_groups(helpful)
 
     # --help is automatically provided by argparse
     helpful.add(
@@ -638,16 +677,25 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         None, "-t", "--text", dest="text_mode", action="store_true",
         help="Use the text output instead of the curses UI.")
     helpful.add(
-        None, "-n", "--non-interactive", "--noninteractive",
+        [None, "automation"], "-n", "--non-interactive", "--noninteractive",
         dest="noninteractive_mode", action="store_true",
         help="Run without ever asking for user input. This may require "
               "additional command line flags; the client will try to explain "
               "which ones are required if it finds one missing")
     helpful.add(
         None, "--dialog", dest="dialog_mode", action="store_true",
-        help="Run using dialog")
+        help="Run using interactive dialog menus")
     helpful.add(
-        None, "--dry-run", action="store_true", dest="dry_run",
+        [None, "run", "certonly"],
+        "-d", "--domains", "--domain", dest="domains",
+        metavar="DOMAIN", action=_DomainsAction, default=[],
+        help="Domain names to apply. For multiple domains you can use "
+             "multiple -d flags or enter a comma separated list of domains "
+             "as a parameter.")
+
+    helpful.add(
+        [None, "testing", "renew", "certonly"],
+        "--dry-run", action="store_true", dest="dry_run",
         help="Perform a test run of the client, obtaining test (invalid) certs"
              " but not saving them to disk. This can currently only be used"
              " with the 'certonly' and 'renew' subcommands. \nNote: Although --dry-run"
@@ -659,7 +707,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              " if they are defined because they may be necessary to accurately simulate"
              " renewal. --renew-hook commands are not called.")
     helpful.add(
-        None, "--register-unsafely-without-email", action="store_true",
+        ["register", "automation"], "--register-unsafely-without-email", action="store_true",
         help="Specifying this flag enables registering an account with no "
              "email address. This is strongly discouraged, because in the "
              "event of key loss or account compromise you will irrevocably "
@@ -674,20 +722,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "with an existing registration, such as the e-mail address, "
              "should be updated, rather than registering a new account.")
     helpful.add(None, "-m", "--email", help=config_help("email"))
-    # positional arg shadows --domains, instead of appending, and
-    # --domains is useful, because it can be stored in config
-    #for subparser in parser_run, parser_auth, parser_install:
-    #    subparser.add_argument("domains", nargs="*", metavar="domain")
-    helpful.add(None, "-d", "--domains", "--domain", dest="domains",
-                metavar="DOMAIN", action=_DomainsAction, default=[],
-                help="Domain names to apply. For multiple domains you can use "
-                "multiple -d flags or enter a comma separated list of domains "
-                "as a parameter.")
-    helpful.add_group(
-        "automation",
-        description="Arguments for automating execution & other tweaks")
     helpful.add(
-        "automation", "--keep-until-expiring", "--keep", "--reinstall",
+        ["automation", "renew", "certonly", "run"],
+        "--keep-until-expiring", "--keep", "--reinstall",
         dest="reinstall", action="store_true",
         help="If the requested cert matches an existing cert, always keep the "
              "existing one until it is due for renewal (for the "
@@ -701,14 +738,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         version="%(prog)s {0}".format(certbot.__version__),
         help="show program's version number and exit")
     helpful.add(
-        "automation", "--force-renewal", "--renew-by-default",
+        ["automation", "renew"],
+        "--force-renewal", "--renew-by-default",
         action="store_true", dest="renew_by_default", help="If a certificate "
              "already exists for the requested domains, renew it now, "
              "regardless of whether it is near expiry. (Often "
              "--keep-until-expiring is more appropriate). Also implies "
              "--expand.")
     helpful.add(
-        "automation", "--allow-subset-of-names", action="store_true",
+        ["automation", "renew", "certonly"],
+        "--allow-subset-of-names", action="store_true",
         help="When performing domain validation, do not consider it a failure "
              "if authorizations can not be obtained for a strict subset of "
              "the requested domains. This may be useful for allowing renewals for "
@@ -732,14 +771,14 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help="(certbot-auto only) prevent the certbot-auto script from"
              " upgrading itself to newer released versions")
     helpful.add(
-        "automation", "-q", "--quiet", dest="quiet", action="store_true",
+        ["automation", "renew", "certonly"],
+        "-q", "--quiet", dest="quiet", action="store_true",
         help="Silence all output except errors. Useful for automation via cron."
              " Implies --non-interactive.")
-
-    helpful.add_group(
-        "testing", description="The following flags are meant for "
-        "testing purposes only! Do NOT change them, unless you "
-        "really know what you're doing!")
+    # overwrites server, handled in HelpfulArgumentParser.parse_args()
+    helpful.add("testing", "--test-cert", "--staging", action='store_true', dest='staging',
+        help='Use the staging server to obtain test (invalid) certs; equivalent'
+             ' to --server ' + constants.STAGING_URI)
     helpful.add(
         "testing", "--debug", action="store_true",
         help="Show tracebacks in case of errors, and allow certbot-auto "
@@ -759,8 +798,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         "testing", "--break-my-certs", action="store_true",
         help="Be willing to replace or renew valid certs with invalid "
              "(testing/staging) certs")
-    helpful.add_group(
-        "security", description="Security parameters & server settings")
     helpful.add(
         "security", "--rsa-key-size", type=int, metavar="N",
         default=flag_default("rsa_key_size"), help=config_help("rsa_key_size"))
@@ -803,26 +840,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         "security", "--no-staple-ocsp", action="store_false",
         help="Do not automatically enable OCSP Stapling.",
         dest="staple", default=None)
-
-
     helpful.add(
         "security", "--strict-permissions", action="store_true",
         help="Require that all configuration files are owned by the current "
              "user; only needed if your config is somewhere unsafe like /tmp/")
-
-    helpful.add_group(
-        "renew", description="The 'renew' subcommand will attempt to renew all"
-        " certificates (or more precisely, certificate lineages) you have"
-        " previously obtained if they are close to expiry, and print a"
-        " summary of the results. By default, 'renew' will reuse the options"
-        " used to create obtain or most recently successfully renew each"
-        " certificate lineage. You can try it with `--dry-run` first. For"
-        " more fine-grained control, you can renew individual lineages with"
-        " the `certonly` subcommand. Hooks are available to run commands"
-        " before and after renewal; see"
-        " https://certbot.eff.org/docs/using.html#renewal for more"
-        " information on these.")
-
     helpful.add(
         "renew", "--pre-hook",
         help="Command to be run in a shell before obtaining any certificates."
@@ -868,13 +889,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
 
 
 def _create_subparsers(helpful):
-    helpful.add_group("certonly", description="Options for modifying how a cert is obtained")
-    helpful.add_group("install", description="Options for modifying how a cert is deployed")
-    helpful.add_group("revoke", description="Options for revocation of certs")
-    helpful.add_group("rollback", description="Options for reverting config changes")
-    helpful.add_group("plugins", description="Plugin options")
-    helpful.add_group("config_changes",
-                      description="Options for showing a history of config changes")
     helpful.add("config_changes", "--num", type=int,
                 help="How many past revisions you want to be displayed")
     helpful.add(
@@ -910,8 +924,6 @@ def _paths_parser(helpful):
     verb = helpful.verb
     if verb == "help":
         verb = helpful.help_arg
-    helpful.add_group(
-        "paths", description="Arguments changing execution paths & servers")
 
     cph = "Path to where cert is saved (with auth --csr), installed from or revoked."
     section = "paths"
@@ -950,19 +962,17 @@ def _paths_parser(helpful):
         help="Logs directory.")
     add("paths", "--server", default=flag_default("server"),
         help=config_help("server"))
-    # overwrites server, handled in HelpfulArgumentParser.parse_args()
-    add("testing", "--test-cert", "--staging", action='store_true', dest='staging',
-        help='Use the staging server to obtain test (invalid) certs; equivalent'
-             ' to --server ' + constants.STAGING_URI)
 
 
 def _plugins_parsing(helpful, plugins):
+    # It's nuts, but there are two "plugins" topics.  Somehow this works
     helpful.add_group(
-        "plugins", description="Certbot client supports an "
+        "plugins", description="Plugin Selection: Certbot client supports an "
         "extensible plugins architecture. See '%(prog)s plugins' for a "
         "list of all installed plugins and their names. You can force "
         "a particular plugin by setting options provided below. Running "
         "--help <plugin_name> will list flags specific to that plugin.")
+
     helpful.add(
         "plugins", "-a", "--authenticator", help="Authenticator plugin name.")
     helpful.add(
@@ -971,15 +981,17 @@ def _plugins_parsing(helpful, plugins):
         "plugins", "--configurator", help="Name of the plugin that is "
         "both an authenticator and an installer. Should not be used "
         "together with --authenticator or --installer.")
-    helpful.add("plugins", "--apache", action="store_true",
+    helpful.add(["plugins", "certonly", "run", "install"],
+                "--apache", action="store_true",
                 help="Obtain and install certs using Apache")
-    helpful.add("plugins", "--nginx", action="store_true",
+    helpful.add(["plugins", "certonly", "run", "install"],
+                "--nginx", action="store_true",
                 help="Obtain and install certs using Nginx")
-    helpful.add("plugins", "--standalone", action="store_true",
+    helpful.add(["plugins", "certonly"], "--standalone", action="store_true",
                 help='Obtain certs using a "standalone" webserver.')
-    helpful.add("plugins", "--manual", action="store_true",
+    helpful.add(["plugins", "certonly"], "--manual", action="store_true",
                 help='Provide laborious manual instructions for obtaining a cert')
-    helpful.add("plugins", "--webroot", action="store_true",
+    helpful.add(["plugins", "certonly"], "--webroot", action="store_true",
                 help='Obtain certs by placing files in a webroot directory.')
 
     # things should not be reorder past/pre this comment:

certbot/interfaces.py

@@ -227,7 +227,7 @@ class IConfig(zope.interface.Interface):
         "Location of renewal configuration file.")
 
     no_verify_ssl = zope.interface.Attribute(
-        "Disable SSL certificate verification.")
+        "Disable verification of the ACME server's certificate.")
     tls_sni_01_port = zope.interface.Attribute(
         "Port number to perform tls-sni-01 challenge. "
         "Boulder in testing mode defaults to 5001.")

certbot/plugins/disco_test.py

@@ -56,9 +56,7 @@ class PluginEntryPointTest(unittest.TestCase):
                 name, PluginEntryPoint.entry_point_to_plugin_name(entry_point))
 
     def test_description(self):
-        self.assertEqual(
-            "Automatically use a temporary webserver",
-            self.plugin_ep.description)
+        self.assertTrue("temporary webserver" in self.plugin_ep.description)
 
     def test_description_with_name(self):
         self.plugin_ep.plugin_cls = mock.MagicMock(description="Desc")

certbot/plugins/standalone.py

@@ -154,7 +154,7 @@ class Authenticator(common.Plugin):
     rely on any existing server program.
     """
 
-    description = "Automatically use a temporary webserver"
+    description = "Spin up a temporary webserver"
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)

certbot/tests/cli_test.py

@@ -113,7 +113,15 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         out = self._help_output(['--help', 'plugins'])
         self.assertTrue("--manual-test-mode" not in out)
         self.assertTrue("--prepare" in out)
-        self.assertTrue("Plugin options" in out)
+        self.assertTrue('"plugins" subcommand' in out)
+
+        # test multiple topics
+        out = self._help_output(['-h', 'renew'])
+        self.assertTrue("--keep" in out)
+        out = self._help_output(['-h', 'automation'])
+        self.assertTrue("--keep" in out)
+        out = self._help_output(['-h', 'revoke'])
+        self.assertTrue("--keep" not in out)
 
         out = self._help_output(['--help', 'install'])
         self.assertTrue("--cert-path" in out)