Address comments

docs/pkgs/letsencrypt_apache.rst

@@ -10,6 +10,12 @@
 .. automodule:: letsencrypt_apache.configurator
    :members:
 
+:mod:`letsencrypt_apache.display_ops`
+=====================================
+
+.. automodule:: letsencrypt_apache.display_ops
+   :members:
+
 :mod:`letsencrypt_apache.dvsni`
 ===============================
 

letsencrypt_apache/configurator.py

@@ -86,7 +86,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     zope.interface.implements(interfaces.IAuthenticator, interfaces.IInstaller)
     zope.interface.classProvides(interfaces.IPluginFactory)
 
-    description = "Apache Web Server"
+    description = "Apache Web Server - Alpha"
 
     @classmethod
     def add_parser_arguments(cls, add):

letsencrypt_apache/display_ops.py

@@ -1,5 +1,7 @@
 """Contains UI methods for Apache operations."""
+import logging
 import os
+
 import zope.component
 
 from letsencrypt import interfaces
@@ -7,14 +9,17 @@ from letsencrypt import interfaces
 import letsencrypt.display.util as display_util
 
 
+logger = logging.getLogger(__name__)
+
+
 def select_vhost(domain, vhosts):
     """Select an appropriate Apache Vhost.
 
     :param vhosts: Available Apache Virtual Hosts
     :type vhosts: :class:`list` of type `~obj.Vhost`
 
-    :returns: VirtualHost
-    :rtype: `~obj.Vhost`
+    :returns: VirtualHost or `None`
+    :rtype: `~obj.Vhost` or `None`
 
     """
     if not vhosts:
@@ -39,8 +44,20 @@ def _vhost_menu(domain, vhosts):
     :rtype: `tuple`
 
     """
-    filename_size = 24
-    disp_name_size = 17
+    # Free characters in the line of display text (9 is for ' | ' formatting)
+    free_chars = display_util.WIDTH - len("HTTPS") - len("Enabled") - 9
+
+    if free_chars < 2:
+        logger.debug("Display size is too small for "
+                     "letsencrypt_apache.display_ops._vhost_menu()")
+        # This runs the edge off the screen, but it doesn't cause an "error"
+        filename_size = 1
+        disp_name_size = 1
+    else:
+        # Filename is a bit more important and probably longer with 000-*
+        filename_size = int(free_chars * .6)
+        disp_name_size = free_chars - filename_size
+
     choices = []
     for vhost in vhosts:
         if len(vhost.names) == 1:
@@ -51,13 +68,14 @@ def _vhost_menu(domain, vhosts):
             disp_name = "Multiple Names"
 
         choices.append(
-            "{0:{4}s} | {1:{5}s} | {2:5s} | {3:7s}".format(
-                os.path.basename(vhost.filep)[:filename_size],
-                disp_name[:disp_name_size],
-                "HTTPS" if vhost.ssl else "",
-                "Enabled" if vhost.enabled else "",
-                filename_size,
-                disp_name_size)
+            "{fn:{fn_size}s} | {name:{name_size}s} | {https:5s} | "
+            "{active:7s}".format(
+                fn=os.path.basename(vhost.filep)[:filename_size],
+                name=disp_name[:disp_name_size],
+                https="HTTPS" if vhost.ssl else "",
+                active="Enabled" if vhost.enabled else "",
+                fn_size=filename_size,
+                name_size=disp_name_size)
         )
 
     code, tag = zope.component.getUtility(interfaces.IDisplay).menu(

letsencrypt_apache/obj.py

@@ -15,7 +15,6 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     :ivar bool enabled: Virtual host is enabled
 
     """
-
     def __init__(self, filep, path, addrs, ssl, enabled, names=None):
         # pylint: disable=too-many-arguments
         """Initialize a VH."""
@@ -32,18 +31,18 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
     def __str__(self):
         return (
-            "File: %s\n"
-            "Vhost path: %s\n"
-            "Addresses: %s\n"
-            "Names: %s\n"
-            "TLS Enabled: %s\n"
-            "Site Enabled: %s" % (
-                self.filep,
-                self.path,
-                ", ".join(str(addr) for addr in self.addrs),
-                ", ".join(name for name in self.names),
-                "Yes" if self.ssl else "No",
-                "Yes" if self.enabled else "No"))
+            "File: {filename}\n"
+            "Vhost path: {vhpath}\n"
+            "Addresses: {addrs}\n"
+            "Names: {names}\n"
+            "TLS Enabled: {tls}\n"
+            "Site Enabled: {active}".format(
+                filename=self.filep,
+                vhpath=self.path,
+                addrs=", ".join(str(addr) for addr in self.addrs),
+                names=", ".join(name for name in self.names),
+                tls="Yes" if self.ssl else "No",
+                active="Yes" if self.enabled else "No"))
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):

letsencrypt_apache/tests/display_ops_test.py

@@ -43,5 +43,16 @@ class SelectVhostTest(unittest.TestCase):
     def test_no_vhosts(self):
         self.assertEqual(self._call([]), None)
 
+    @mock.patch("letsencrypt_apache.display_ops.display_util")
+    @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")
+    @mock.patch("letsencrypt_apache.display_ops.logging")
+    def test_small_display(self, mock_logging, mock_util, mock_display_util):
+        mock_display_util.WIDTH = 20
+        mock_util().menu.return_value = (display_util.OK, 0)
+        self._call(self.vhosts)
+
+        self.assertTrue(mock_logging.is_called)
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover