From 31fef196c0ed57a5cc6b1c4d409ff8097afbc716 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 27 Sep 2015 01:15:35 +0000
Subject: [PATCH 01/19] --help is effectively a verb for CLI purposes...

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 7cb4a0458..711ac0048 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -679,7 +679,7 @@ def create_parser(plugins, args):
 # For now unfortunately this constant just needs to match the code below;
 # there isn't an elegant way to autogenerate it in time.
 VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",
-         "plugins"]
+         "plugins", "--help"]
 
 
 def _create_subparsers(helpful):

From 001d37f9650d0c5f7521673f9fd075a7bd662b0c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 27 Sep 2015 02:41:55 +0000
Subject: [PATCH 02/19] "-h" is also a ver.

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index a5968ec9c..2c996cd3e 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -679,7 +679,7 @@ def create_parser(plugins, args):
 # For now unfortunately this constant just needs to match the code below;
 # there isn't an elegant way to autogenerate it in time.
 VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",
-         "plugins", "--help"]
+         "plugins", "--help", "-h"]
 
 
 def _create_subparsers(helpful):

From f3c2a096b54950e5368907b698c488a458180961 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 27 Sep 2015 02:48:44 +0000
Subject: [PATCH 03/19] Move the verb/subcommand to the end of the argparse
 line

---
 letsencrypt/cli.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 2c996cd3e..8afbf023f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -495,13 +495,14 @@ class HelpfulArgumentParser(object):
     def preprocess_args(self, args):
         """Work around some limitations in argparse.
 
-        Currently, add the default verb "run" as a default.
+	Currently: add the default verb "run" as a default, and ensure that the
+        subcommand / verb comes last.
         """
-
-        for token in args:
+        for i,token in enumerate(args):
             if token in VERBS:
-                return args
-        return ["run"] + args
+		reordered = args[:i] + args[i+1:] + [args[i]]
+                return reordered
+        return args + ["run"]
 
     def prescan_for_flag(self, flag, possible_arguments):
         """Checks cli input for flags.

From ddc04c755bf6a3e9da956ecf8782ee32b6464cc0 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 27 Sep 2015 07:56:38 +0000
Subject: [PATCH 04/19] work in progress

---
 letsencrypt/cli.py | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6d3aa9d2c..53609009b 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -730,53 +730,55 @@ def _create_subparsers(helpful):
     # the order of add_subparser() calls is important: it defines the
     # order in which subparser names will be displayed in --help
     add_subparser("run", run)
+
     parser_auth = add_subparser("auth", auth)
+    helpful.add_group("auth", "Options for modifying how a cert is obtained")
     parser_install = add_subparser("install", install)
+    helpful.add_group("install", "Options for modifying how a cert is deployed")
     parser_revoke = add_subparser("revoke", revoke)
+    helpful.add_group("revoke", "Options for revocation of certs")
     parser_rollback = add_subparser("rollback", rollback)
+    helpful.add_group("rollback", "Options for reverting config changes")
     add_subparser("config_changes", config_changes)
     parser_plugins = add_subparser("plugins", plugins_cmd)
+    helpful.add_group("plugins", "Plugin options")
 
-    parser_auth.add_argument(
-        "--csr", type=read_file, help="Path to a Certificate Signing "
-        "Request (CSR) in DER format.")
-    parser_auth.add_argument(
+    helpful.add("auth", 
+        "--csr", type=read_file, help="Path to a Certificate Signing Request (CSR) in DER format.")
+    helpful.add("auth", 
         "--cert-path", default=flag_default("auth_cert_path"),
         help="When using --csr this is where certificate is saved.")
-    parser_auth.add_argument(
+    helpful.add("auth",
         "--chain-path", default=flag_default("auth_chain_path"),
         help="When using --csr this is where certificate chain is saved.")
 
-    parser_install.add_argument(
-        "--cert-path", required=True, help="Path to a certificate that "
-        "is going to be installed.")
-    parser_install.add_argument(
+    helpful.add("install", 
+        "--cert-path", required=True, help="Path to a certificate that is going to be installed.")
+    helpful.add("install", 
         "--key-path", required=True, help="Accompanying private key")
-    parser_install.add_argument(
+    helpful.add("install", 
         "--chain-path", help="Accompanying path to a certificate chain.")
-    parser_revoke.add_argument(
-        "--cert-path", type=read_file, help="Revoke a specific certificate.",
-        required=True)
-    parser_revoke.add_argument(
+    helpful.add("revoke", 
+        "--cert-path", type=read_file, help="Revoke a specific certificate.", required=True)
+    helpful.add("revoke", 
         "--key-path", type=read_file,
-        help="Revoke certificate using its accompanying key. Useful if "
-        "Account Key is lost.")
+        help="Revoke certificate using its accompanying key. Useful if Account Key is lost.")
 
-    parser_rollback.add_argument(
+    helpful.add("rollback", 
         "--checkpoints", type=int, metavar="N",
         default=flag_default("rollback_checkpoints"),
         help="Revert configuration N number of checkpoints.")
 
-    parser_plugins.add_argument(
+    helpful.add("plugins", 
         "--init", action="store_true", help="Initialize plugins.")
-    parser_plugins.add_argument(
+    helpful.add("plugins", 
         "--prepare", action="store_true",
         help="Initialize and prepare plugins.")
-    parser_plugins.add_argument(
+    helpful.add("plugins", 
         "--authenticators", action="append_const", dest="ifaces",
         const=interfaces.IAuthenticator,
         help="Limit to authenticator plugins only.")
-    parser_plugins.add_argument(
+    helpful.add("plugins", 
         "--installers", action="append_const", dest="ifaces",
         const=interfaces.IInstaller, help="Limit to installer plugins only.")
 

From 2e0fd36c2831db4fcdaefdd5c43fac41ee7fbac6 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 21:01:02 +0000
Subject: [PATCH 05/19] Improve flag and help processing

 * letsencrypt --help $SUBCOMMAND now works.
   Fixes #787 #819
 * subcommand arguments are now actually argument groups, so that all flags
   can be placed before or after subcommand verbs as the user wishes
   Fixes: #820

A limitation:

 * args like --cert-path were previously present for multiple verbs
   (auth/install/revoke) with separate docs; they are now in the "paths"
   topic.  That's fine, though it would be good to *also* list them when the
   user types letsencrypt --help install.
---
 letsencrypt/cli.py | 64 ++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 53609009b..ac2c55551 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -489,8 +489,6 @@ class SilentParser(object):  # pylint: disable=too-few-public-methods
         self.parser.add_argument(*args, **kwargs)
 
 
-HELP_TOPICS = ["all", "security", "paths", "automation", "testing", "plugins"]
-
 
 class HelpfulArgumentParser(object):
     """Argparse Wrapper.
@@ -529,12 +527,17 @@ class HelpfulArgumentParser(object):
     def preprocess_args(self, args):
         """Work around some limitations in argparse.
 
-	Currently: add the default verb "run" as a default, and ensure that the
+        Currently: add the default verb "run" as a default, and ensure that the
         subcommand / verb comes last.
         """
+
+        if "-h" in args or "--help" in args:
+            # all verbs double as help arguments; don't get them confused
+            return args
+
         for i,token in enumerate(args):
             if token in VERBS:
-		reordered = args[:i] + args[i+1:] + [args[i]]
+                reordered = args[:i] + args[i+1:] + [args[i]]
                 return reordered
         return args + ["run"]
 
@@ -717,6 +720,9 @@ def create_parser(plugins, args):
 VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",
          "plugins", "--help", "-h"]
 
+HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"] +
+               [v for v in VERBS if "-" not in v])
+
 
 def _create_subparsers(helpful):
     subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND")
@@ -732,53 +738,34 @@ def _create_subparsers(helpful):
     add_subparser("run", run)
 
     parser_auth = add_subparser("auth", auth)
-    helpful.add_group("auth", "Options for modifying how a cert is obtained")
+    helpful.add_group("auth", description="Options for modifying how a cert is obtained")
     parser_install = add_subparser("install", install)
-    helpful.add_group("install", "Options for modifying how a cert is deployed")
+    helpful.add_group("install", description="Options for modifying how a cert is deployed")
     parser_revoke = add_subparser("revoke", revoke)
-    helpful.add_group("revoke", "Options for revocation of certs")
+    helpful.add_group("revoke", description="Options for revocation of certs")
     parser_rollback = add_subparser("rollback", rollback)
-    helpful.add_group("rollback", "Options for reverting config changes")
+    helpful.add_group("rollback", description="Options for reverting config changes")
     add_subparser("config_changes", config_changes)
     parser_plugins = add_subparser("plugins", plugins_cmd)
-    helpful.add_group("plugins", "Plugin options")
+    helpful.add_group("plugins", description="Plugin options")
 
-    helpful.add("auth", 
-        "--csr", type=read_file, help="Path to a Certificate Signing Request (CSR) in DER format.")
-    helpful.add("auth", 
-        "--cert-path", default=flag_default("auth_cert_path"),
-        help="When using --csr this is where certificate is saved.")
     helpful.add("auth",
-        "--chain-path", default=flag_default("auth_chain_path"),
-        help="When using --csr this is where certificate chain is saved.")
-
-    helpful.add("install", 
-        "--cert-path", required=True, help="Path to a certificate that is going to be installed.")
-    helpful.add("install", 
-        "--key-path", required=True, help="Accompanying private key")
-    helpful.add("install", 
-        "--chain-path", help="Accompanying path to a certificate chain.")
-    helpful.add("revoke", 
-        "--cert-path", type=read_file, help="Revoke a specific certificate.", required=True)
-    helpful.add("revoke", 
-        "--key-path", type=read_file,
-        help="Revoke certificate using its accompanying key. Useful if Account Key is lost.")
-
-    helpful.add("rollback", 
+        "--csr", type=read_file, help="Path to a Certificate Signing Request (CSR) in DER format.")
+    helpful.add("rollback",
         "--checkpoints", type=int, metavar="N",
         default=flag_default("rollback_checkpoints"),
         help="Revert configuration N number of checkpoints.")
 
-    helpful.add("plugins", 
+    helpful.add("plugins",
         "--init", action="store_true", help="Initialize plugins.")
-    helpful.add("plugins", 
+    helpful.add("plugins",
         "--prepare", action="store_true",
         help="Initialize and prepare plugins.")
-    helpful.add("plugins", 
+    helpful.add("plugins",
         "--authenticators", action="append_const", dest="ifaces",
         const=interfaces.IAuthenticator,
         help="Limit to authenticator plugins only.")
-    helpful.add("plugins", 
+    helpful.add("plugins",
         "--installers", action="append_const", dest="ifaces",
         const=interfaces.IInstaller, help="Limit to installer plugins only.")
 
@@ -787,6 +774,15 @@ def _paths_parser(helpful):
     add = helpful.add
     helpful.add_group(
         "paths", description="Arguments changing execution paths & servers")
+    helpful.add("paths",
+        "--cert-path", default=flag_default("auth_cert_path"),
+        help="Path to where certificate is saved (with auth), "
+             "installed (with install --csr) or revoked.")
+    helpful.add("paths",
+        "--key-path", required=True,
+        help="Path to private key for cert creation or revocation (if account key is missing)")
+    helpful.add("paths",
+        "--chain-path", help="Accompanying path to a certificate chain.")
     add("paths", "--config-dir", default=flag_default("config_dir"),
         help=config_help("config_dir"))
     add("paths", "--work-dir", default=flag_default("work_dir"),

From a0af023b1436e27f5c1a7626aeeab374d927cf3b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 14:48:26 -0700
Subject: [PATCH 06/19] --key-path is mandatory for install, optional for
 revoke

---
 letsencrypt/cli.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ac2c55551..8042173e8 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -530,7 +530,6 @@ class HelpfulArgumentParser(object):
         Currently: add the default verb "run" as a default, and ensure that the
         subcommand / verb comes last.
         """
-
         if "-h" in args or "--help" in args:
             # all verbs double as help arguments; don't get them confused
             return args
@@ -779,7 +778,7 @@ def _paths_parser(helpful):
         help="Path to where certificate is saved (with auth), "
              "installed (with install --csr) or revoked.")
     helpful.add("paths",
-        "--key-path", required=True,
+        "--key-path", required=("install" in helpful.args),
         help="Path to private key for cert creation or revocation (if account key is missing)")
     helpful.add("paths",
         "--chain-path", help="Accompanying path to a certificate chain.")

From 05d439a33937c4c46d2ee949dc70c9126f463efd Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 14:48:40 -0700
Subject: [PATCH 07/19] Update cli tests

  We don't expect to error out if called with no args
---
 letsencrypt/tests/cli_test.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 2e9f3330c..992b254a7 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -40,7 +40,9 @@ class CLITest(unittest.TestCase):
         return ret, stdout, stderr, client
 
     def test_no_flags(self):
-        self.assertRaises(SystemExit, self._call, [])
+        with mock.patch('letsencrypt.cli.run') as mock_run:
+          self._call([])
+          self.assertEqual(1, mock_run.call_count)
 
     def test_help(self):
         self.assertRaises(SystemExit, self._call, ['--help'])

From 2297349b95f1451d10caae24297ba3b84dd7d6ce Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 16:56:36 -0700
Subject: [PATCH 08/19] lintian

---
 letsencrypt/cli.py            | 24 ++++++++++++------------
 letsencrypt/tests/cli_test.py |  4 ++--
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 0d72a3eb5..b7efa041a 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -500,7 +500,6 @@ class SilentParser(object):  # pylint: disable=too-few-public-methods
         self.parser.add_argument(*args, **kwargs)
 
 
-
 class HelpfulArgumentParser(object):
     """Argparse Wrapper.
 
@@ -545,7 +544,7 @@ class HelpfulArgumentParser(object):
             # all verbs double as help arguments; don't get them confused
             return args
 
-        for i,token in enumerate(args):
+        for i, token in enumerate(args):
             if token in VERBS:
                 reordered = args[:i] + args[i+1:] + [args[i]]
                 return reordered
@@ -745,18 +744,21 @@ def _create_subparsers(helpful):
 
     # the order of add_subparser() calls is important: it defines the
     # order in which subparser names will be displayed in --help
-    add_subparser("run", run)
+    # these add_subparser objects return objects to which arguments could be
+    # attached, but they have annoying arg ordering constrains so we use
+    # groups instead: https://github.com/letsencrypt/letsencrypt/issues/820
 
-    parser_auth = add_subparser("auth", auth)
+    add_subparser("run", run)
+    add_subparser("auth", auth)
     helpful.add_group("auth", description="Options for modifying how a cert is obtained")
-    parser_install = add_subparser("install", install)
+    add_subparser("install", install)
     helpful.add_group("install", description="Options for modifying how a cert is deployed")
-    parser_revoke = add_subparser("revoke", revoke)
+    add_subparser("revoke", revoke)
     helpful.add_group("revoke", description="Options for revocation of certs")
-    parser_rollback = add_subparser("rollback", rollback)
+    add_subparser("rollback", rollback)
     helpful.add_group("rollback", description="Options for reverting config changes")
     add_subparser("config_changes", config_changes)
-    parser_plugins = add_subparser("plugins", plugins_cmd)
+    add_subparser("plugins", plugins_cmd)
     helpful.add_group("plugins", description="Plugin options")
 
     helpful.add("auth",
@@ -769,12 +771,10 @@ def _create_subparsers(helpful):
     helpful.add("plugins",
         "--init", action="store_true", help="Initialize plugins.")
     helpful.add("plugins",
-        "--prepare", action="store_true",
-        help="Initialize and prepare plugins.")
+        "--prepare", action="store_true", help="Initialize and prepare plugins.")
     helpful.add("plugins",
         "--authenticators", action="append_const", dest="ifaces",
-        const=interfaces.IAuthenticator,
-        help="Limit to authenticator plugins only.")
+        const=interfaces.IAuthenticator, help="Limit to authenticator plugins only.")
     helpful.add("plugins",
         "--installers", action="append_const", dest="ifaces",
         const=interfaces.IInstaller, help="Limit to installer plugins only.")
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index ce32b8f78..9a99a74cc 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -44,8 +44,8 @@ class CLITest(unittest.TestCase):
 
     def test_no_flags(self):
         with mock.patch('letsencrypt.cli.run') as mock_run:
-          self._call([])
-          self.assertEqual(1, mock_run.call_count)
+            self._call([])
+            self.assertEqual(1, mock_run.call_count)
 
     def test_help(self):
         self.assertRaises(SystemExit, self._call, ['--help'])

From 6b6bc038827e359173039a5cb229104ef257e127 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 17:12:38 -0700
Subject: [PATCH 09/19] --cert-path was required for install and revoke

Oops
---
 letsencrypt/cli.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index b7efa041a..82bd57ec8 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -786,6 +786,7 @@ def _paths_parser(helpful):
         "paths", description="Arguments changing execution paths & servers")
     helpful.add("paths",
         "--cert-path", default=flag_default("auth_cert_path"),
+        required=("install" in helpful.args or "revoke" in helpful.args),
         help="Path to where certificate is saved (with auth), "
              "installed (with install --csr) or revoked.")
     helpful.add("paths",

From 18dacc528df67e703336bd778518a25f4850b345 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 18:08:58 -0700
Subject: [PATCH 10/19] Preserve all argparse parameters

Try to restore all variants that applied to the different subcomannds
---
 letsencrypt/cli.py | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 82bd57ec8..ac79ab93c 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -521,6 +521,7 @@ class HelpfulArgumentParser(object):
         self.parser._add_config_file_help = False  # pylint: disable=protected-access
         self.silent_parser = SilentParser(self.parser)
 
+        self.verb = None
         self.args = self.preprocess_args(args)
         help1 = self.prescan_for_flag("-h", self.help_topics)
         help2 = self.prescan_for_flag("--help", self.help_topics)
@@ -542,12 +543,16 @@ class HelpfulArgumentParser(object):
         """
         if "-h" in args or "--help" in args:
             # all verbs double as help arguments; don't get them confused
+            self.verb = "help"
             return args
 
         for i, token in enumerate(args):
             if token in VERBS:
                 reordered = args[:i] + args[i+1:] + [args[i]]
+                self.verb = token
                 return reordered
+
+        self.verb = "run"
         return args + ["run"]
 
     def prescan_for_flag(self, flag, possible_arguments):
@@ -782,18 +787,28 @@ def _create_subparsers(helpful):
 
 def _paths_parser(helpful):
     add = helpful.add
+    verb = helpful.verb
     helpful.add_group(
         "paths", description="Arguments changing execution paths & servers")
-    helpful.add("paths",
-        "--cert-path", default=flag_default("auth_cert_path"),
-        required=("install" in helpful.args or "revoke" in helpful.args),
-        help="Path to where certificate is saved (with auth), "
-             "installed (with install --csr) or revoked.")
-    helpful.add("paths",
-        "--key-path", required=("install" in helpful.args),
+
+    cph = "Path to where cert is saved (with auth), installed (with install --csr) or revoked."
+    if verb == "auth":
+        add("paths", "--cert-path", default=flag_default("auth_cert_path"), help=cph)
+    elif verb == "revoke":
+        add("paths", "--cert-path", type=read_file, required=True, help=cph)
+    else:
+        add("paths", "--cert-path", help=cph, required=(verb == "install"))
+
+    # revoke --key-path reads a file, install --key-path takes a string
+    add("paths", "--key-path", type=((verb == "revoke" and read_file) or str),
+        required=(verb == "install"),
         help="Path to private key for cert creation or revocation (if account key is missing)")
-    helpful.add("paths",
-        "--chain-path", help="Accompanying path to a certificate chain.")
+
+    default_cp = None
+    if verb == "auth":
+        default_cp = flag_default("auth_chain_path")
+    add("paths", "--chain-path", default=default_cp,
+        help="Accompanying path to a certificate chain.")
     add("paths", "--config-dir", default=flag_default("config_dir"),
         help=config_help("config_dir"))
     add("paths", "--work-dir", default=flag_default("work_dir"),

From 627fca37b4e45d300fdb3a023943b11c6bbae593 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Sep 2015 18:18:18 -0700
Subject: [PATCH 11/19] We didn't actually need to define --help as a verb

---
 letsencrypt/cli.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ac79ab93c..efc0f9f70 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -732,10 +732,10 @@ def create_parser(plugins, args):
 # For now unfortunately this constant just needs to match the code below;
 # there isn't an elegant way to autogenerate it in time.
 VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",
-         "plugins", "--help", "-h"]
+         "plugins"]
 
 HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"] +
-               [v for v in VERBS if "-" not in v])
+               [v for v in VERBS])
 
 
 def _create_subparsers(helpful):

From 1e3c92c714bb382298343d5c14f14aa896e765ab Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 11:49:46 -0700
Subject: [PATCH 12/19] Cleanup the verb -> subparser mapping

---
 letsencrypt/cli.py | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index efc0f9f70..fd9d8cbb6 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -9,6 +9,7 @@ import os
 import pkg_resources
 import sys
 import time
+import types
 import traceback
 
 import configargparse
@@ -731,17 +732,16 @@ def create_parser(plugins, args):
 
 # For now unfortunately this constant just needs to match the code below;
 # there isn't an elegant way to autogenerate it in time.
-VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",
-         "plugins"]
-
-HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"] +
-               [v for v in VERBS])
-
+VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", "plugins"]
+HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"] + VERBS)
 
 def _create_subparsers(helpful):
     subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND")
 
-    def add_subparser(name, func):  # pylint: disable=missing-docstring
+    def add_subparser(name):  # pylint: disable=missing-docstring
+        # Each subcommand is implemented by a function of the same name
+        func = eval(name) # pylint: disable=eval-used
+        assert isinstance(func, types.FunctionType), "squirrels in namespace"
         subparser = subparsers.add_parser(
             name, help=func.__doc__.splitlines()[0], description=func.__doc__)
         subparser.set_defaults(func=func)
@@ -752,18 +752,13 @@ def _create_subparsers(helpful):
     # these add_subparser objects return objects to which arguments could be
     # attached, but they have annoying arg ordering constrains so we use
     # groups instead: https://github.com/letsencrypt/letsencrypt/issues/820
+    for v in VERBS:
+        add_subparser(v)
 
-    add_subparser("run", run)
-    add_subparser("auth", auth)
     helpful.add_group("auth", description="Options for modifying how a cert is obtained")
-    add_subparser("install", install)
     helpful.add_group("install", description="Options for modifying how a cert is deployed")
-    add_subparser("revoke", revoke)
     helpful.add_group("revoke", description="Options for revocation of certs")
-    add_subparser("rollback", rollback)
     helpful.add_group("rollback", description="Options for reverting config changes")
-    add_subparser("config_changes", config_changes)
-    add_subparser("plugins", plugins_cmd)
     helpful.add_group("plugins", description="Plugin options")
 
     helpful.add("auth",

From 2a3a111d628711e131ea202511a1383c10dfe378 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 12:09:38 -0700
Subject: [PATCH 13/19] Disable pylint invalid-name

It's clearly making our code harder to read and write
---
 .pylintrc                              | 2 +-
 acme/acme/challenges.py                | 4 ++--
 acme/acme/challenges_test.py           | 2 +-
 acme/acme/jose/interfaces_test.py      | 4 ++--
 acme/acme/jose/json_util_test.py       | 6 +++---
 acme/acme/jose/jwa_test.py             | 2 +-
 acme/acme/jose/jwk.py                  | 2 +-
 acme/acme/jose/util.py                 | 4 ++--
 acme/acme/jose/util_test.py            | 4 ++--
 acme/acme/messages_test.py             | 2 +-
 letsencrypt/account.py                 | 2 +-
 letsencrypt/display/enhancements.py    | 2 +-
 letsencrypt/display/ops.py             | 2 +-
 letsencrypt/log.py                     | 2 +-
 letsencrypt/plugins/common.py          | 6 +++---
 letsencrypt/tests/auth_handler_test.py | 2 +-
 letsencrypt/tests/log_test.py          | 2 +-
 letsencrypt/tests/reverter_test.py     | 8 ++++----
 18 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/.pylintrc b/.pylintrc
index bf318704a..268d61ec6 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -38,7 +38,7 @@ load-plugins=linter_plugin
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=fixme,locally-disabled,abstract-class-not-used,bad-continuation,too-few-public-methods,no-self-use
+disable=fixme,locally-disabled,abstract-class-not-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name
 # abstract-class-not-used cannot be disabled locally (at least in pylint 1.4.1)
 
 
diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py
index d81e77f83..f5763adc4 100644
--- a/acme/acme/challenges.py
+++ b/acme/acme/challenges.py
@@ -315,7 +315,7 @@ class DVSNIResponse(ChallengeResponse):
     validation = jose.Field("validation", decoder=jose.JWS.from_json)
 
     @property
-    def z(self):  # pylint: disable=invalid-name
+    def z(self):
         """The ``z``  parameter.
 
         :rtype: bytes
@@ -333,7 +333,7 @@ class DVSNIResponse(ChallengeResponse):
         :rtype: bytes
 
         """
-        z = self.z  # pylint: disable=invalid-name
+        z = self.z
         return z[:32] + b'.' + z[32:] + self.DOMAIN_SUFFIX
 
     @property
diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py
index ed44d4c45..b3f48cdf2 100644
--- a/acme/acme/challenges_test.py
+++ b/acme/acme/challenges_test.py
@@ -269,7 +269,7 @@ class DVSNIResponseTest(unittest.TestCase):
             'validation': self.validation.to_json(),
         }
 
-        # pylint: disable=invalid-name
+
         label1 = b'e2df3498860637c667fedadc5a8494ec'
         label2 = b'09dcc75553c9b3bd73662b50e71b1e42'
         self.z = label1 + label2
diff --git a/acme/acme/jose/interfaces_test.py b/acme/acme/jose/interfaces_test.py
index 380c3a2a5..a3ee124ff 100644
--- a/acme/acme/jose/interfaces_test.py
+++ b/acme/acme/jose/interfaces_test.py
@@ -8,7 +8,7 @@ class JSONDeSerializableTest(unittest.TestCase):
     def setUp(self):
         from acme.jose.interfaces import JSONDeSerializable
 
-        # pylint: disable=missing-docstring,invalid-name
+        # pylint: disable=missing-docstring
 
         class Basic(JSONDeSerializable):
             def __init__(self, v):
@@ -53,7 +53,7 @@ class JSONDeSerializableTest(unittest.TestCase):
         self.nested = Basic([[self.basic1]])
         self.tuple = Basic(('foo',))
 
-        # pylint: disable=invalid-name
+
         self.Basic = Basic
         self.Sequence = Sequence
         self.Mapping = Mapping
diff --git a/acme/acme/jose/json_util_test.py b/acme/acme/jose/json_util_test.py
index a055f3bf7..f751382e0 100644
--- a/acme/acme/jose/json_util_test.py
+++ b/acme/acme/jose/json_util_test.py
@@ -92,7 +92,7 @@ class JSONObjectWithFieldsMetaTest(unittest.TestCase):
         from acme.jose.json_util import JSONObjectWithFieldsMeta
         self.field = Field('Baz')
         self.field2 = Field('Baz2')
-        # pylint: disable=invalid-name,missing-docstring,too-few-public-methods
+        # pylint: disable=missing-docstring,too-few-public-methods
         # pylint: disable=blacklisted-name
 
         @six.add_metaclass(JSONObjectWithFieldsMeta)
@@ -138,7 +138,7 @@ class JSONObjectWithFieldsTest(unittest.TestCase):
         from acme.jose.json_util import Field
 
         class MockJSONObjectWithFields(JSONObjectWithFields):
-            # pylint: disable=invalid-name,missing-docstring,no-self-argument
+            # pylint: disable=missing-docstring,no-self-argument
             # pylint: disable=too-few-public-methods
             x = Field('x', omitempty=True,
                       encoder=(lambda x: x * 2),
@@ -158,7 +158,7 @@ class JSONObjectWithFieldsTest(unittest.TestCase):
                     raise errors.DeserializationError()
                 return value
 
-        # pylint: disable=invalid-name
+
         self.MockJSONObjectWithFields = MockJSONObjectWithFields
         self.mock = MockJSONObjectWithFields(x=None, y=2, z=3)
 
diff --git a/acme/acme/jose/jwa_test.py b/acme/acme/jose/jwa_test.py
index 3328d083a..8ca512043 100644
--- a/acme/acme/jose/jwa_test.py
+++ b/acme/acme/jose/jwa_test.py
@@ -26,7 +26,7 @@ class JWASignatureTest(unittest.TestCase):
             def verify(self, key, msg, sig):
                 raise NotImplementedError()  # pragma: no cover
 
-        # pylint: disable=invalid-name
+
         self.Sig1 = MockSig('Sig1')
         self.Sig2 = MockSig('Sig2')
 
diff --git a/acme/acme/jose/jwk.py b/acme/acme/jose/jwk.py
index 7a976f189..67f243347 100644
--- a/acme/acme/jose/jwk.py
+++ b/acme/acme/jose/jwk.py
@@ -186,7 +186,7 @@ class JWKRSA(JWK):
 
     @classmethod
     def fields_from_json(cls, jobj):
-        # pylint: disable=invalid-name
+
         n, e = (cls._decode_param(jobj[x]) for x in ('n', 'e'))
         public_numbers = rsa.RSAPublicNumbers(e=e, n=n)
         if 'd' not in jobj:  # public key
diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py
index ab3606efc..46c43bf35 100644
--- a/acme/acme/jose/util.py
+++ b/acme/acme/jose/util.py
@@ -7,7 +7,7 @@ import six
 
 
 class abstractclassmethod(classmethod):
-    # pylint: disable=invalid-name,too-few-public-methods
+    # pylint: disable=too-few-public-methods
     """Descriptor for an abstract classmethod.
 
     It augments the :mod:`abc` framework with an abstract
@@ -172,7 +172,7 @@ class ImmutableMap(collections.Mapping, collections.Hashable):
 
 
 class frozendict(collections.Mapping, collections.Hashable):
-    # pylint: disable=invalid-name,too-few-public-methods
+    # pylint: disable=too-few-public-methods
     """Frozen dictionary."""
     __slots__ = ('_items', '_keys')
 
diff --git a/acme/acme/jose/util_test.py b/acme/acme/jose/util_test.py
index 4cdd9127f..295c70fee 100644
--- a/acme/acme/jose/util_test.py
+++ b/acme/acme/jose/util_test.py
@@ -92,7 +92,7 @@ class ImmutableMapTest(unittest.TestCase):
     """Tests for acme.jose.util.ImmutableMap."""
 
     def setUp(self):
-        # pylint: disable=invalid-name,too-few-public-methods
+        # pylint: disable=too-few-public-methods
         # pylint: disable=missing-docstring
         from acme.jose.util import ImmutableMap
 
@@ -156,7 +156,7 @@ class ImmutableMapTest(unittest.TestCase):
         self.assertEqual("B(x='foo', y='bar')", repr(self.B(x='foo', y='bar')))
 
 
-class frozendictTest(unittest.TestCase):  # pylint: disable=invalid-name
+class frozendictTest(unittest.TestCase):
     """Tests for acme.jose.util.frozendict."""
 
     def setUp(self):
diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py
index d2d859bc5..718a936dd 100644
--- a/acme/acme/messages_test.py
+++ b/acme/acme/messages_test.py
@@ -64,7 +64,7 @@ class ConstantTest(unittest.TestCase):
         class MockConstant(_Constant):  # pylint: disable=missing-docstring
             POSSIBLE_NAMES = {}
 
-        self.MockConstant = MockConstant  # pylint: disable=invalid-name
+        self.MockConstant = MockConstant
         self.const_a = MockConstant('a')
         self.const_b = MockConstant('b')
 
diff --git a/letsencrypt/account.py b/letsencrypt/account.py
index c97e4f6fe..81d31b831 100644
--- a/letsencrypt/account.py
+++ b/letsencrypt/account.py
@@ -54,7 +54,7 @@ class Account(object):  # pylint: disable=too-few-public-methods
                 tz=pytz.UTC).replace(microsecond=0),
             creation_host=socket.getfqdn()) if meta is None else meta
 
-        self.id = hashlib.md5(  # pylint: disable=invalid-name
+        self.id = hashlib.md5(
             self.key.key.public_key().public_bytes(
                 encoding=serialization.Encoding.DER,
                 format=serialization.PublicFormat.SubjectPublicKeyInfo)
diff --git a/letsencrypt/display/enhancements.py b/letsencrypt/display/enhancements.py
index 8edc72ba0..c56198161 100644
--- a/letsencrypt/display/enhancements.py
+++ b/letsencrypt/display/enhancements.py
@@ -11,7 +11,7 @@ from letsencrypt.display import util as display_util
 logger = logging.getLogger(__name__)
 
 # Define a helper function to avoid verbose code
-util = zope.component.getUtility  # pylint: disable=invalid-name
+util = zope.component.getUtility
 
 
 def ask(enhancement):
diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py
index 43705e309..cb424a81b 100644
--- a/letsencrypt/display/ops.py
+++ b/letsencrypt/display/ops.py
@@ -12,7 +12,7 @@ from letsencrypt.display import util as display_util
 logger = logging.getLogger(__name__)
 
 # Define a helper function to avoid verbose code
-util = zope.component.getUtility  # pylint: disable=invalid-name
+util = zope.component.getUtility
 
 
 def choose_plugin(prepared, question):
diff --git a/letsencrypt/log.py b/letsencrypt/log.py
index e800d37c9..6436f6fc2 100644
--- a/letsencrypt/log.py
+++ b/letsencrypt/log.py
@@ -25,7 +25,7 @@ class DialogHandler(logging.Handler):  # pylint: disable=too-few-public-methods
         logging.Handler.__init__(self, level)
         self.height = height
         self.width = width
-        # "dialog" collides with module name... pylint: disable=invalid-name
+        # "dialog" collides with module name...
         self.d = dialog.Dialog() if d is None else d
         self.lines = []
 
diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py
index 59598a35e..95ad56a0a 100644
--- a/letsencrypt/plugins/common.py
+++ b/letsencrypt/plugins/common.py
@@ -23,10 +23,10 @@ def dest_namespace(name):
     """ArgumentParser dest namespace (prefix of all destinations)."""
     return name.replace("-", "_") + "_"
 
-private_ips_regex = re.compile(  # pylint: disable=invalid-name
+private_ips_regex = re.compile(
     r"(^127\.0\.0\.1)|(^10\.)|(^172\.1[6-9]\.)|"
     r"(^172\.2[0-9]\.)|(^172\.3[0-1]\.)|(^192\.168\.)")
-hostname_regex = re.compile(  # pylint: disable=invalid-name
+hostname_regex = re.compile(
     r"^(([a-z0-9]|[a-z0-9][a-z0-9\-]*[a-z0-9])\.)*[a-z]+$", re.IGNORECASE)
 
 
@@ -173,7 +173,7 @@ class Dvsni(object):
                             achall.chall.encode("token") + '.pem')
 
     def _setup_challenge_cert(self, achall, s=None):
-        # pylint: disable=invalid-name
+
         """Generate and write out challenge certificate."""
         cert_path = self.get_cert_path(achall)
         key_path = self.get_key_path(achall)
diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py
index ed29ead25..18ee56081 100644
--- a/letsencrypt/tests/auth_handler_test.py
+++ b/letsencrypt/tests/auth_handler_test.py
@@ -355,7 +355,7 @@ class GenChallengePathTest(unittest.TestCase):
 class MutuallyExclusiveTest(unittest.TestCase):
     """Tests for letsencrypt.auth_handler.mutually_exclusive."""
 
-    # pylint: disable=invalid-name,missing-docstring,too-few-public-methods
+    # pylint: disable=missing-docstring,too-few-public-methods
     class A(object):
         pass
 
diff --git a/letsencrypt/tests/log_test.py b/letsencrypt/tests/log_test.py
index 50d0712e7..c1afd2c8a 100644
--- a/letsencrypt/tests/log_test.py
+++ b/letsencrypt/tests/log_test.py
@@ -8,7 +8,7 @@ import mock
 class DialogHandlerTest(unittest.TestCase):
 
     def setUp(self):
-        self.d = mock.MagicMock()  # pylint: disable=invalid-name
+        self.d = mock.MagicMock()
 
         from letsencrypt.log import DialogHandler
         self.handler = DialogHandler(height=2, width=6, d=self.d)
diff --git a/letsencrypt/tests/reverter_test.py b/letsencrypt/tests/reverter_test.py
index 62c47f8d6..d31b6f2cc 100644
--- a/letsencrypt/tests/reverter_test.py
+++ b/letsencrypt/tests/reverter_test.py
@@ -85,7 +85,7 @@ class ReverterCheckpointLocalTest(unittest.TestCase):
         self.assertEqual(read_in(self.config1), "directive-dir1")
 
     def test_multiple_registration_fail_and_revert(self):
-        # pylint: disable=invalid-name
+
         config3 = os.path.join(self.dir1, "config3.txt")
         update_file(config3, "Config3")
         config4 = os.path.join(self.dir2, "config4.txt")
@@ -173,7 +173,7 @@ class ReverterCheckpointLocalTest(unittest.TestCase):
         self.assertRaises(errors.ReverterError, self.reverter.recovery_routine)
 
     def test_recover_checkpoint_revert_temp_failures(self):
-        # pylint: disable=invalid-name
+
         mock_recover = mock.MagicMock(
             side_effect=errors.ReverterError("e"))
 
@@ -291,7 +291,7 @@ class TestFullCheckpointsReverter(unittest.TestCase):
             errors.ReverterError, self.reverter.rollback_checkpoints, "one")
 
     def test_rollback_finalize_checkpoint_valid_inputs(self):
-        # pylint: disable=invalid-name
+
         config3 = self._setup_three_checkpoints()
 
         # Check resulting backup directory
@@ -334,7 +334,7 @@ class TestFullCheckpointsReverter(unittest.TestCase):
 
     @mock.patch("letsencrypt.reverter.os.rename")
     def test_finalize_checkpoint_no_rename_directory(self, mock_rename):
-        # pylint: disable=invalid-name
+
         self.reverter.add_to_checkpoint(self.sets[0], "perm save")
         mock_rename.side_effect = OSError
 

From 2d578468bde4cbba2138216633de44bfdd46cf04 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 12:32:44 -0700
Subject: [PATCH 14/19] Use a verb -> function table instead of eval()

 - plugins_cmd() not plugins() broke the more minimalist eval() approach
 - more wrangling was required to mock out calls via the VERBS table
---
 letsencrypt/cli.py            | 25 +++++++++++++++----------
 letsencrypt/tests/cli_test.py |  2 ++
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index fd9d8cbb6..66f991063 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -9,7 +9,6 @@ import os
 import pkg_resources
 import sys
 import time
-import types
 import traceback
 
 import configargparse
@@ -731,17 +730,23 @@ def create_parser(plugins, args):
     return helpful.parser, helpful.args
 
 # For now unfortunately this constant just needs to match the code below;
-# there isn't an elegant way to autogenerate it in time.
-VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", "plugins"]
-HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"] + VERBS)
+# there isn't an elegant way to autogenerate it in time. pylint: disable=bad-whitespace
+VERBS = {
+    "run"            : run,
+    "auth"           : auth,
+    "install"        : install,
+    "revoke"         : revoke,
+    "rollback"       : rollback,
+    "config_changes" : config_changes,
+    "plugins"        : plugins_cmd
+}
+HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"]
+               + VERBS.keys())
 
 def _create_subparsers(helpful):
     subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND")
 
-    def add_subparser(name):  # pylint: disable=missing-docstring
-        # Each subcommand is implemented by a function of the same name
-        func = eval(name) # pylint: disable=eval-used
-        assert isinstance(func, types.FunctionType), "squirrels in namespace"
+    def add_subparser(name, func):  # pylint: disable=missing-docstring
         subparser = subparsers.add_parser(
             name, help=func.__doc__.splitlines()[0], description=func.__doc__)
         subparser.set_defaults(func=func)
@@ -752,8 +757,8 @@ def _create_subparsers(helpful):
     # these add_subparser objects return objects to which arguments could be
     # attached, but they have annoying arg ordering constrains so we use
     # groups instead: https://github.com/letsencrypt/letsencrypt/issues/820
-    for v in VERBS:
-        add_subparser(v)
+    for v, func in VERBS.items():
+        add_subparser(v, func)
 
     helpful.add_group("auth", description="Options for modifying how a cert is obtained")
     helpful.add_group("install", description="Options for modifying how a cert is deployed")
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 9a99a74cc..f5613ee58 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -44,6 +44,8 @@ class CLITest(unittest.TestCase):
 
     def test_no_flags(self):
         with mock.patch('letsencrypt.cli.run') as mock_run:
+            from letsencrypt import cli
+            cli.VERBS["run"] = mock_run
             self._call([])
             self.assertEqual(1, mock_run.call_count)
 

From d85f42d71f5aba91cb96af6f9959c778fb047b9f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 15:29:29 -0700
Subject: [PATCH 15/19] Plugins don't need to be in HELP_TOPICS

They're already added as topics automatically, though they do need to be in
the hand-written top level help.
---
 letsencrypt/cli.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 66f991063..1ad57b738 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -80,8 +80,8 @@ More detailed help:
   -h, --help [topic]    print this message, or detailed help on a topic;
                         the available topics are:
 
-   all, apache, automation, nginx, paths, security, testing, or any of the
-   subcommands
+   all, apache, automation, manual, nginx, paths, security, testing, or any of
+   the subcommands
 """
 
 
@@ -740,8 +740,7 @@ VERBS = {
     "config_changes" : config_changes,
     "plugins"        : plugins_cmd
 }
-HELP_TOPICS = (["all", "security", "paths", "automation", "testing", "apache", "nginx"]
-               + VERBS.keys())
+HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + VERBS.keys()
 
 def _create_subparsers(helpful):
     subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND")

From 5ca1a27200fb17ac04104ba65c05d810bb20b906 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 15:31:32 -0700
Subject: [PATCH 16/19] Keep the acme/ subtree compatible with strict pylinting

---
 acme/acme/challenges.py           | 4 ++--
 acme/acme/challenges_test.py      | 2 +-
 acme/acme/jose/interfaces_test.py | 4 ++--
 acme/acme/jose/json_util_test.py  | 6 +++---
 acme/acme/jose/jwa_test.py        | 2 +-
 acme/acme/jose/jwk.py             | 2 +-
 acme/acme/jose/util.py            | 4 ++--
 acme/acme/jose/util_test.py       | 4 ++--
 acme/acme/messages_test.py        | 2 +-
 9 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py
index f5763adc4..d81e77f83 100644
--- a/acme/acme/challenges.py
+++ b/acme/acme/challenges.py
@@ -315,7 +315,7 @@ class DVSNIResponse(ChallengeResponse):
     validation = jose.Field("validation", decoder=jose.JWS.from_json)
 
     @property
-    def z(self):
+    def z(self):  # pylint: disable=invalid-name
         """The ``z``  parameter.
 
         :rtype: bytes
@@ -333,7 +333,7 @@ class DVSNIResponse(ChallengeResponse):
         :rtype: bytes
 
         """
-        z = self.z
+        z = self.z  # pylint: disable=invalid-name
         return z[:32] + b'.' + z[32:] + self.DOMAIN_SUFFIX
 
     @property
diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py
index b3f48cdf2..ed44d4c45 100644
--- a/acme/acme/challenges_test.py
+++ b/acme/acme/challenges_test.py
@@ -269,7 +269,7 @@ class DVSNIResponseTest(unittest.TestCase):
             'validation': self.validation.to_json(),
         }
 
-
+        # pylint: disable=invalid-name
         label1 = b'e2df3498860637c667fedadc5a8494ec'
         label2 = b'09dcc75553c9b3bd73662b50e71b1e42'
         self.z = label1 + label2
diff --git a/acme/acme/jose/interfaces_test.py b/acme/acme/jose/interfaces_test.py
index a3ee124ff..380c3a2a5 100644
--- a/acme/acme/jose/interfaces_test.py
+++ b/acme/acme/jose/interfaces_test.py
@@ -8,7 +8,7 @@ class JSONDeSerializableTest(unittest.TestCase):
     def setUp(self):
         from acme.jose.interfaces import JSONDeSerializable
 
-        # pylint: disable=missing-docstring
+        # pylint: disable=missing-docstring,invalid-name
 
         class Basic(JSONDeSerializable):
             def __init__(self, v):
@@ -53,7 +53,7 @@ class JSONDeSerializableTest(unittest.TestCase):
         self.nested = Basic([[self.basic1]])
         self.tuple = Basic(('foo',))
 
-
+        # pylint: disable=invalid-name
         self.Basic = Basic
         self.Sequence = Sequence
         self.Mapping = Mapping
diff --git a/acme/acme/jose/json_util_test.py b/acme/acme/jose/json_util_test.py
index f751382e0..a055f3bf7 100644
--- a/acme/acme/jose/json_util_test.py
+++ b/acme/acme/jose/json_util_test.py
@@ -92,7 +92,7 @@ class JSONObjectWithFieldsMetaTest(unittest.TestCase):
         from acme.jose.json_util import JSONObjectWithFieldsMeta
         self.field = Field('Baz')
         self.field2 = Field('Baz2')
-        # pylint: disable=missing-docstring,too-few-public-methods
+        # pylint: disable=invalid-name,missing-docstring,too-few-public-methods
         # pylint: disable=blacklisted-name
 
         @six.add_metaclass(JSONObjectWithFieldsMeta)
@@ -138,7 +138,7 @@ class JSONObjectWithFieldsTest(unittest.TestCase):
         from acme.jose.json_util import Field
 
         class MockJSONObjectWithFields(JSONObjectWithFields):
-            # pylint: disable=missing-docstring,no-self-argument
+            # pylint: disable=invalid-name,missing-docstring,no-self-argument
             # pylint: disable=too-few-public-methods
             x = Field('x', omitempty=True,
                       encoder=(lambda x: x * 2),
@@ -158,7 +158,7 @@ class JSONObjectWithFieldsTest(unittest.TestCase):
                     raise errors.DeserializationError()
                 return value
 
-
+        # pylint: disable=invalid-name
         self.MockJSONObjectWithFields = MockJSONObjectWithFields
         self.mock = MockJSONObjectWithFields(x=None, y=2, z=3)
 
diff --git a/acme/acme/jose/jwa_test.py b/acme/acme/jose/jwa_test.py
index 8ca512043..3328d083a 100644
--- a/acme/acme/jose/jwa_test.py
+++ b/acme/acme/jose/jwa_test.py
@@ -26,7 +26,7 @@ class JWASignatureTest(unittest.TestCase):
             def verify(self, key, msg, sig):
                 raise NotImplementedError()  # pragma: no cover
 
-
+        # pylint: disable=invalid-name
         self.Sig1 = MockSig('Sig1')
         self.Sig2 = MockSig('Sig2')
 
diff --git a/acme/acme/jose/jwk.py b/acme/acme/jose/jwk.py
index 67f243347..7a976f189 100644
--- a/acme/acme/jose/jwk.py
+++ b/acme/acme/jose/jwk.py
@@ -186,7 +186,7 @@ class JWKRSA(JWK):
 
     @classmethod
     def fields_from_json(cls, jobj):
-
+        # pylint: disable=invalid-name
         n, e = (cls._decode_param(jobj[x]) for x in ('n', 'e'))
         public_numbers = rsa.RSAPublicNumbers(e=e, n=n)
         if 'd' not in jobj:  # public key
diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py
index 46c43bf35..ab3606efc 100644
--- a/acme/acme/jose/util.py
+++ b/acme/acme/jose/util.py
@@ -7,7 +7,7 @@ import six
 
 
 class abstractclassmethod(classmethod):
-    # pylint: disable=too-few-public-methods
+    # pylint: disable=invalid-name,too-few-public-methods
     """Descriptor for an abstract classmethod.
 
     It augments the :mod:`abc` framework with an abstract
@@ -172,7 +172,7 @@ class ImmutableMap(collections.Mapping, collections.Hashable):
 
 
 class frozendict(collections.Mapping, collections.Hashable):
-    # pylint: disable=too-few-public-methods
+    # pylint: disable=invalid-name,too-few-public-methods
     """Frozen dictionary."""
     __slots__ = ('_items', '_keys')
 
diff --git a/acme/acme/jose/util_test.py b/acme/acme/jose/util_test.py
index 295c70fee..4cdd9127f 100644
--- a/acme/acme/jose/util_test.py
+++ b/acme/acme/jose/util_test.py
@@ -92,7 +92,7 @@ class ImmutableMapTest(unittest.TestCase):
     """Tests for acme.jose.util.ImmutableMap."""
 
     def setUp(self):
-        # pylint: disable=too-few-public-methods
+        # pylint: disable=invalid-name,too-few-public-methods
         # pylint: disable=missing-docstring
         from acme.jose.util import ImmutableMap
 
@@ -156,7 +156,7 @@ class ImmutableMapTest(unittest.TestCase):
         self.assertEqual("B(x='foo', y='bar')", repr(self.B(x='foo', y='bar')))
 
 
-class frozendictTest(unittest.TestCase):
+class frozendictTest(unittest.TestCase):  # pylint: disable=invalid-name
     """Tests for acme.jose.util.frozendict."""
 
     def setUp(self):
diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py
index 718a936dd..d2d859bc5 100644
--- a/acme/acme/messages_test.py
+++ b/acme/acme/messages_test.py
@@ -64,7 +64,7 @@ class ConstantTest(unittest.TestCase):
         class MockConstant(_Constant):  # pylint: disable=missing-docstring
             POSSIBLE_NAMES = {}
 
-        self.MockConstant = MockConstant
+        self.MockConstant = MockConstant  # pylint: disable=invalid-name
         self.const_a = MockConstant('a')
         self.const_b = MockConstant('b')
 

From 2406fc0486d8f74ad9979b1973e9e24d9d453df7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 15:56:58 -0700
Subject: [PATCH 17/19] Go back to VERBS as a list

The dictionary was destroying the ordering, which was important.
---
 letsencrypt/cli.py            | 28 ++++++++++++----------------
 letsencrypt/tests/cli_test.py |  2 --
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 1ad57b738..73dd24bdb 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -730,24 +730,20 @@ def create_parser(plugins, args):
     return helpful.parser, helpful.args
 
 # For now unfortunately this constant just needs to match the code below;
-# there isn't an elegant way to autogenerate it in time. pylint: disable=bad-whitespace
-VERBS = {
-    "run"            : run,
-    "auth"           : auth,
-    "install"        : install,
-    "revoke"         : revoke,
-    "rollback"       : rollback,
-    "config_changes" : config_changes,
-    "plugins"        : plugins_cmd
-}
-HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + VERBS.keys()
+# there isn't an elegant way to autogenerate it in time.
+VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", "plugins"]
+HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + VERBS
 
 def _create_subparsers(helpful):
     subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND")
 
-    def add_subparser(name, func):  # pylint: disable=missing-docstring
-        subparser = subparsers.add_parser(
-            name, help=func.__doc__.splitlines()[0], description=func.__doc__)
+    def add_subparser(name):  # pylint: disable=missing-docstring
+        if name == "plugins":
+            func = plugins_cmd
+        else:
+            func = eval(name) # pylint: disable=eval-used
+        h = func.__doc__.splitlines()[0]
+        subparser = subparsers.add_parser(name, help=h, description=func.__doc__)
         subparser.set_defaults(func=func)
         return subparser
 
@@ -756,8 +752,8 @@ def _create_subparsers(helpful):
     # these add_subparser objects return objects to which arguments could be
     # attached, but they have annoying arg ordering constrains so we use
     # groups instead: https://github.com/letsencrypt/letsencrypt/issues/820
-    for v, func in VERBS.items():
-        add_subparser(v, func)
+    for v in VERBS:
+        add_subparser(v)
 
     helpful.add_group("auth", description="Options for modifying how a cert is obtained")
     helpful.add_group("install", description="Options for modifying how a cert is deployed")
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index f5613ee58..9a99a74cc 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -44,8 +44,6 @@ class CLITest(unittest.TestCase):
 
     def test_no_flags(self):
         with mock.patch('letsencrypt.cli.run') as mock_run:
-            from letsencrypt import cli
-            cli.VERBS["run"] = mock_run
             self._call([])
             self.assertEqual(1, mock_run.call_count)
 

From 11ca1108c2536adb0d735e76829f178f06a08715 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 16:53:08 -0700
Subject: [PATCH 18/19] Test cases for command line help

---
 letsencrypt/tests/cli_test.py | 37 ++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 9a99a74cc..75eec1978 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -2,6 +2,7 @@
 import itertools
 import os
 import shutil
+import StringIO
 import traceback
 import tempfile
 import unittest
@@ -42,6 +43,21 @@ class CLITest(unittest.TestCase):
                     ret = cli.main(args)
         return ret, stdout, stderr, client
 
+    def _call_stdout(self, args):
+        """
+        Variant of _call that preserves stdout so that it can be mocked by the
+        caller.
+        """
+        from letsencrypt import cli
+        args = ['--text', '--config-dir', self.config_dir,
+                '--work-dir', self.work_dir, '--logs-dir', self.logs_dir,
+                '--agree-eula'] + args
+        with mock.patch('letsencrypt.cli.sys.stderr') as stderr:
+            with mock.patch('letsencrypt.cli.client') as client:
+                ret = cli.main(args)
+        return ret, None, stderr, client
+
+
     def test_no_flags(self):
         with mock.patch('letsencrypt.cli.run') as mock_run:
             self._call([])
@@ -49,7 +65,26 @@ class CLITest(unittest.TestCase):
 
     def test_help(self):
         self.assertRaises(SystemExit, self._call, ['--help'])
-        self.assertRaises(SystemExit, self._call, ['--help all'])
+        self.assertRaises(SystemExit, self._call, ['--help', 'all'])
+        output = StringIO.StringIO()
+        with mock.patch('letsencrypt.cli.sys.stdout', new=output):
+            self.assertRaises(SystemExit, self._call_stdout, ['--help', 'all'])
+            out = output.getvalue()
+            self.assertTrue("--configurator" in out)
+            self.assertTrue("how a cert is deployed" in out)
+            self.assertTrue("--manual-test-mode" in out)
+            output.truncate(0)
+            self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx'])
+            out = output.getvalue()
+            self.assertTrue("--nginx-ctl" in out)
+            self.assertTrue("--manual-test-mode" not in out)
+            self.assertTrue("--checkpoints" not in out)
+            output.truncate(0)
+            self.assertRaises(SystemExit, self._call_stdout, ['--help', 'plugins'])
+            out = output.getvalue()
+            self.assertTrue("--manual-test-mode" not in out)
+            self.assertTrue("--prepare" in out)
+            self.assertTrue("Plugin options" in out)
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])

From 43cb36807a001562726bceaaaae00d708fcc5ed2 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Sep 2015 17:00:09 -0700
Subject: [PATCH 19/19] Also test top level help

---
 letsencrypt/tests/cli_test.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 75eec1978..0a92aba62 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -85,6 +85,12 @@ class CLITest(unittest.TestCase):
             self.assertTrue("--manual-test-mode" not in out)
             self.assertTrue("--prepare" in out)
             self.assertTrue("Plugin options" in out)
+            output.truncate(0)
+            self.assertRaises(SystemExit, self._call_stdout, ['-h'])
+            out = output.getvalue()
+            from letsencrypt import cli
+            self.assertTrue(cli.USAGE in out)
+
 
     def test_rollback(self):
         _, _, _, client = self._call(['rollback'])