diff --git a/.pylintrc b/.pylintrc index f31b77e9a..92dde98c0 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,abstract-class-little-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name +disable=fixme,locally-disabled,abstract-class-not-used,abstract-class-little-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name,too-many-instance-attributes # abstract-class-not-used cannot be disabled locally (at least in # pylint 1.4.1), same for abstract-class-little-used diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f8c0da85a..39c19346b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -652,12 +652,12 @@ class HelpfulArgumentParser(object): help1 = self.prescan_for_flag("-h", self.help_topics) help2 = self.prescan_for_flag("--help", self.help_topics) assert max(True, "a") == "a", "Gravity changed direction" - help_arg = max(help1, help2) - if help_arg is True: + self.help_arg = max(help1, help2) + if self.help_arg is True: # just --help with no topic; avoid argparse altogether print usage sys.exit(0) - self.visible_topics = self.determine_help_topics(help_arg) + self.visible_topics = self.determine_help_topics(self.help_arg) #print self.visible_topics self.groups = {} # elements are added by .add_group() @@ -875,12 +875,12 @@ def prepare_and_parse_args(plugins, args): help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") + _create_subparsers(helpful) _paths_parser(helpful) # _plugins_parsing should be the last thing to act upon the main # parser (--help should display plugin-specific options last) _plugins_parsing(helpful, plugins) - _create_subparsers(helpful) return helpful.parse_args() @@ -916,19 +916,28 @@ def _create_subparsers(helpful): def _paths_parser(helpful): add = helpful.add 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), installed (with install --csr) or revoked." + cph = "Path to where cert is saved (with auth --csr), installed from or revoked." + section = "paths" + if verb in ("install", "revoke", "certonly"): + section = verb if verb == "certonly": - add("paths", "--cert-path", default=flag_default("auth_cert_path"), help=cph) + add(section, "--cert-path", default=flag_default("auth_cert_path"), help=cph) elif verb == "revoke": - add("paths", "--cert-path", type=read_file, required=True, help=cph) + add(section, "--cert-path", type=read_file, required=True, help=cph) else: - add("paths", "--cert-path", help=cph, required=(verb == "install")) + add(section, "--cert-path", help=cph, required=(verb == "install")) + section = "paths" + if verb in ("install", "revoke"): + section = verb + print helpful.help_arg, helpful.help_arg == "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), + add(section, "--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)") diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 55946e7aa..d6d056777 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -64,47 +64,56 @@ class CLITest(unittest.TestCase): self._call([]) self.assertEqual(1, mock_run.call_count) + def _help_output(self, args): + "Run a help command, and return the help string for scrutiny" + output = StringIO.StringIO() + with mock.patch('letsencrypt.cli.sys.stdout', new=output): + self.assertRaises(SystemExit, self._call_stdout, args) + out = output.getvalue() + return out + def test_help(self): self.assertRaises(SystemExit, self._call, ['--help']) self.assertRaises(SystemExit, self._call, ['--help', 'all']) - output = StringIO.StringIO() - with mock.patch('letsencrypt.cli.sys.stdout', new=output): - plugins = disco.PluginsRegistry.find_all() - 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) + plugins = disco.PluginsRegistry.find_all() + out = self._help_output(['--help', 'all']) + self.assertTrue("--configurator" in out) + self.assertTrue("how a cert is deployed" in out) + self.assertTrue("--manual-test-mode" in out) - self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx']) - out = output.getvalue() - if "nginx" in plugins: - # may be false while building distributions without plugins - self.assertTrue("--nginx-ctl" in out) - self.assertTrue("--manual-test-mode" not in out) - self.assertTrue("--checkpoints" not in out) - output.truncate(0) + out = self._help_output(['-h', 'nginx']) + if "nginx" in plugins: + # may be false while building distributions without plugins + self.assertTrue("--nginx-ctl" in out) + self.assertTrue("--manual-test-mode" not in out) + self.assertTrue("--checkpoints" not in out) - self.assertRaises(SystemExit, self._call_stdout, ['-h']) - out = output.getvalue() - if "nginx" in plugins: - self.assertTrue("Use the Nginx plugin" in out) - else: - self.assertTrue("(nginx support is experimental" in out) - output.truncate(0) + out = self._help_output(['-h']) + if "nginx" in plugins: + self.assertTrue("Use the Nginx plugin" in out) + else: + self.assertTrue("(nginx support is experimental" in out) - 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) - output.truncate(0) + 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.assertRaises(SystemExit, self._call_stdout, ['-h']) - out = output.getvalue() - from letsencrypt import cli - self.assertTrue(cli.usage_strings(plugins)[0] in out) + out = self._help_output(['--help', 'install']) + self.assertTrue("--cert-path" in out) + self.assertTrue("--key-path" in out) + + out = self._help_output(['--help', 'revoke']) + self.assertTrue("--cert-path" in out) + self.assertTrue("--key-path" in out) + + out = self._help_output(['-h', 'config_changes']) + self.assertTrue("--cert-path" not in out) + self.assertTrue("--key-path" not in out) + + out = self._help_output(['-h']) + from letsencrypt import cli + self.assertTrue(cli.usage_strings(plugins)[0] in out) @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops):