diff --git a/certbot-apache/certbot_apache/display_ops.py b/certbot-apache/certbot_apache/display_ops.py index 6bcb64dd5..f9e0802f7 100644 --- a/certbot-apache/certbot_apache/display_ops.py +++ b/certbot-apache/certbot_apache/display_ops.py @@ -27,9 +27,7 @@ def select_vhost(domain, vhosts): return None while True: code, tag = _vhost_menu(domain, vhosts) - if code == display_util.HELP: - _more_info_vhost(vhosts[tag]) - elif code == display_util.OK: + if code == display_util.OK: return vhosts[tag] else: return None @@ -85,8 +83,7 @@ def _vhost_menu(domain, vhosts): "or Address of {0}.{1}Which virtual host would you " "like to choose?\n(note: conf files with multiple " "vhosts are not yet supported)".format(domain, os.linesep), - choices, help_label="More Info", - ok_label="Select", force_interactive=True) + choices, force_interactive=True) except errors.MissingCommandlineFlag: msg = ("Encountered vhost ambiguity but unable to ask for user guidance in " "non-interactive mode. Currently Certbot needs each vhost to be " @@ -96,10 +93,3 @@ def _vhost_menu(domain, vhosts): raise errors.MissingCommandlineFlag(msg) return code, tag - - -def _more_info_vhost(vhost): - zope.component.getUtility(interfaces.IDisplay).notification( - "Virtual Host Information:{0}{1}{0}{2}".format( - os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)), - force_interactive=True) diff --git a/certbot-apache/certbot_apache/tests/display_ops_test.py b/certbot-apache/certbot_apache/tests/display_ops_test.py index f8b75022e..e59d411bd 100644 --- a/certbot-apache/certbot_apache/tests/display_ops_test.py +++ b/certbot-apache/certbot_apache/tests/display_ops_test.py @@ -43,13 +43,10 @@ class SelectVhostTest(unittest.TestCase): @certbot_util.patch_get_utility() def test_more_info_cancel(self, mock_util): mock_util().menu.side_effect = [ - (display_util.HELP, 1), - (display_util.HELP, 0), (display_util.CANCEL, -1), ] self.assertEqual(None, self._call(self.vhosts)) - self.assertEqual(mock_util().notification.call_count, 2) def test_no_vhosts(self): self.assertEqual(self._call([]), None) diff --git a/certbot/cert_manager.py b/certbot/cert_manager.py index 6519e7068..62cbfa695 100644 --- a/certbot/cert_manager.py +++ b/certbot/cert_manager.py @@ -157,7 +157,7 @@ def _get_certname(config, verb): if not choices: raise errors.Error("No existing certificates found.") code, index = disp.menu("Which certificate would you like to {0}?".format(verb), - choices, ok_label="Select", flag="--cert-name", + choices, flag="--cert-name", force_interactive=True) if code != display_util.OK or not index in range(0, len(choices)): raise errors.Error("User ended interaction.") diff --git a/certbot/display/util.py b/certbot/display/util.py index f1922ebb9..194b46a24 100644 --- a/certbot/display/util.py +++ b/certbot/display/util.py @@ -24,10 +24,10 @@ CANCEL = "cancel" """Display exit code for a user canceling the display.""" HELP = "help" -"""Display exit code when for when the user requests more help.""" +"""Display exit code when for when the user requests more help. (UNUSED)""" ESC = "esc" -"""Display exit code when the user hits Escape""" +"""Display exit code when the user hits Escape (UNUSED)""" def _wrap_lines(msg): @@ -123,8 +123,8 @@ class FileDisplay(object): else: logger.debug("Not pausing for user confirmation") - def menu(self, message, choices, ok_label="", cancel_label="", - help_label="", default=None, + def menu(self, message, choices, ok_label=None, cancel_label=None, + help_label=None, default=None, cli_flag=None, force_interactive=False, **unused_kwargs): # pylint: disable=unused-argument """Display a menu. @@ -228,14 +228,13 @@ class FileDisplay(object): ans.startswith(no_label[0].upper())): return False - def checklist(self, message, tags, default_status=True, default=None, + def checklist(self, message, tags, default=None, cli_flag=None, force_interactive=False, **unused_kwargs): # pylint: disable=unused-argument """Display a checklist. :param str message: Message to display to user :param list tags: `str` tags to select, len(tags) > 0 - :param bool default_status: Not used for FileDisplay :param default: default value to return (if one exists) :param str cli_flag: option used to set this value with the CLI :param bool force_interactive: True if it's safe to prompt the user diff --git a/certbot/interfaces.py b/certbot/interfaces.py index f5c32f131..501a5c57e 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -396,8 +396,8 @@ class IDisplay(zope.interface.Interface): """ - def menu(message, choices, ok_label="OK", - cancel_label="Cancel", help_label="", + def menu(message, choices, ok_label=None, + cancel_label=None, help_label=None, default=None, cli_flag=None, force_interactive=False): """Displays a generic menu. @@ -409,9 +409,9 @@ class IDisplay(zope.interface.Interface): :param choices: choices :type choices: :class:`list` of :func:`tuple` or :class:`str` - :param str ok_label: label for OK button - :param str cancel_label: label for Cancel button - :param str help_label: label for Help button + :param str ok_label: label for OK button (UNUSED) + :param str cancel_label: label for Cancel button (UNUSED) + :param str help_label: label for Help button (UNUSED) :param int default: default (non-interactive) choice from the menu :param str cli_flag: to automate choice from the menu, eg "--keep" :param bool force_interactive: True if it's safe to prompt the user @@ -470,8 +470,7 @@ class IDisplay(zope.interface.Interface): """ - def checklist(message, tags, default_state, - default=None, cli_args=None, force_interactive=False): + def checklist(message, tags, default=None, cli_args=None, force_interactive=False): """Allow for multiple selections from a menu. When not setting force_interactive=True, you must provide a @@ -479,7 +478,6 @@ class IDisplay(zope.interface.Interface): :param str message: message to display to the user :param list tags: where each is of type :class:`str` len(tags) > 0 - :param bool default_status: If True, items are in a selected state by default. :param str default: default (non-interactive) state of the checklist :param str cli_flag: to automate choice from the menu, eg "--domains" :param bool force_interactive: True if it's safe to prompt the user diff --git a/certbot/main.py b/certbot/main.py index d3f6eaa09..cd87706b4 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -161,7 +161,7 @@ def _handle_identical_cert_request(config, lineage): "Renew & replace the cert (limit ~5 per 7 days)"] display = zope.component.getUtility(interfaces.IDisplay) - response = display.menu(question, choices, "OK", "Cancel", + response = display.menu(question, choices, default=0, force_interactive=True) if response[0] == display_util.CANCEL: # TODO: Add notification related to command-line options for diff --git a/certbot/plugins/selection.py b/certbot/plugins/selection.py index 0d216a0c3..a8ebc47dd 100644 --- a/certbot/plugins/selection.py +++ b/certbot/plugins/selection.py @@ -112,7 +112,7 @@ def choose_plugin(prepared, question): while True: disp = z_util(interfaces.IDisplay) code, index = disp.menu( - question, opts, help_label="More Info", force_interactive=True) + question, opts, force_interactive=True) if code == display_util.OK: plugin_ep = prepared[index] @@ -123,13 +123,6 @@ def choose_plugin(prepared, question): "was:\n\n{0}".format(plugin_ep.prepare()), pause=False) else: return plugin_ep - elif code == display_util.HELP: - if prepared[index].misconfigured: - msg = "Reported Error: %s" % prepared[index].prepare() - else: - msg = prepared[index].init().more_info() - z_util(interfaces.IDisplay).notification(msg, - force_interactive=True) else: return None diff --git a/certbot/plugins/selection_test.py b/certbot/plugins/selection_test.py index 41c2b55c9..9f0716905 100644 --- a/certbot/plugins/selection_test.py +++ b/certbot/plugins/selection_test.py @@ -137,13 +137,10 @@ class ChoosePluginTest(unittest.TestCase): @test_util.patch_get_utility("certbot.plugins.selection.z_util") def test_more_info(self, mock_util): mock_util().menu.side_effect = [ - (display_util.HELP, 0), - (display_util.HELP, 1), (display_util.OK, 1), ] self.assertEqual(self.mock_stand, self._call()) - self.assertEqual(mock_util().notification.call_count, 2) @test_util.patch_get_utility("certbot.plugins.selection.z_util") def test_no_choice(self, mock_util): diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index aad6ffc82..4662b2aa6 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -117,22 +117,11 @@ to serve all files under specified web root ({0}).""" code, index = display.menu( "Select the webroot for {0}:".format(domain), ["Enter a new webroot"] + known_webroots, - help_label="Help", cli_flag=path_flag, force_interactive=True) + cli_flag=path_flag, force_interactive=True) if code == display_util.CANCEL: raise errors.PluginError( "Every requested domain must have a " "webroot when using the webroot plugin.") - elif code == display_util.HELP: - display.notification( - "To use the webroot plugin, you need to have an " - "HTTP server running on this system serving files " - "for the requested domain. Additionally, this " - "server should be serving all files contained in a " - "public_html or webroot directory. The webroot " - "plugin works by temporarily saving necessary " - "resources in the HTTP server's webroot directory " - "to pass domain validation challenges.", - force_interactive=True) else: # code == display_util.OK return None if index == 0 else known_webroots[index - 1] @@ -141,10 +130,7 @@ to serve all files under specified web root ({0}).""" _validate_webroot, "Input the webroot for {0}:".format(domain), force_interactive=True) - if code == display_util.HELP: - # Displaying help is not currently implemented - return None - elif code == display_util.CANCEL or code == display_util.ESC: + if code == display_util.CANCEL: return None else: # code == display_util.OK return _validate_webroot(webroot) diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index 53809764b..e202a0a6d 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -84,10 +84,8 @@ class AuthenticatorTest(unittest.TestCase): self.config.webroot_map = {"otherthing.com": self.path} mock_display = mock_get_utility() - mock_display.menu.side_effect = ((display_util.HELP, -1), - (display_util.CANCEL, -1),) + mock_display.menu.side_effect = ((display_util.CANCEL, -1),) self.assertRaises(errors.PluginError, self.auth.perform, [self.achall]) - self.assertTrue(mock_display.notification.called) self.assertTrue(mock_display.menu.called) for call in mock_display.menu.call_args_list: self.assertTrue(self.achall.domain in call[0][0]) @@ -103,8 +101,7 @@ class AuthenticatorTest(unittest.TestCase): mock_display = mock_get_utility() mock_display.menu.return_value = (display_util.OK, 0,) with mock.patch('certbot.display.ops.validated_directory') as m: - m.side_effect = ((display_util.HELP, -1), - (display_util.CANCEL, -1), + m.side_effect = ((display_util.CANCEL, -1), (display_util.OK, self.path,)) self.auth.perform([self.achall])