Fix nits and address review comments

letsencrypt/cli.py

@@ -10,7 +10,6 @@ import logging
 import logging.handlers
 import os
 import pkg_resources
-import string
 import sys
 import time
 import traceback
@@ -103,7 +102,7 @@ def usage_strings(plugins):
 
 
 def _find_domains(args, installer):
-    if args.domains is None:
+    if not args.domains:
         domains = display_ops.choose_names(installer)
     else:
         domains = args.domains
@@ -477,7 +476,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
 def obtain_cert(args, config, plugins):
     """Authenticate & obtain cert, but do not install it."""
 
-    if args.domains is not None and args.csr is not None:
+    if args.domains and args.csr is not None:
         # TODO: --csr could have a priority, when --domains is
         # supplied, check if CSR matches given domains?
         return "--domains and --csr are mutually exclusive"
@@ -840,7 +839,7 @@ def prepare_and_parse_args(plugins, args):
     #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=DomainFlagProcessor,
+                metavar="DOMAIN", action=DomainFlagProcessor, 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.")
@@ -1073,17 +1072,18 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
         Keep a record of --webroot-path / -w flags during processing, so that
         we know which apply to which -d flags
         """
-        if not config.webroot_path:
+        if config.webroot_path is None:      # first -w flag encountered
             config.webroot_path = []
             # if any --domain flags preceded the first --webroot-path flag,
             # apply that webroot path to those; subsequent entries in
             # config.webroot_map are filled in by cli.DomainFlagProcessor
             if config.domains:
-                config.webroot_map = dict([(d, webroot) for d in config.domains])
                 self.domain_before_webroot = True
-            else:
-                config.webroot_map = {}
+                for d in config.domains:
+                    config.webroot_map.setdefault(d, webroot)
         elif self.domain_before_webroot:
+            # FIXME if you set domains in a config file, you should get a different error
+            # here, pointing you to --webroot-map
             raise errors.Error("If you specify multiple webroot paths, one of "
                                "them must precede all domain flags")
         config.webroot_path.append(webroot)
@@ -1095,15 +1095,12 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
         Process a new -d flag, helping the webroot plugin construct a map of
         {domain : webrootpath} if -w / --webroot-path is in use
         """
-        if not config.domains:
-            config.domains = []
-
-        for d in map(string.strip, domain_arg.split(",")): # pylint: disable=bad-builtin
-            if d not in config.domains:
-                config.domains.append(d)
+        for domain in (d.strip() for d in domain_arg.split(",")):
+            if domain not in config.domains:
+                config.domains.append(domain)
                 # Each domain has a webroot_path of the most recent -w flag
                 if config.webroot_path:
-                    config.webroot_map[d] = config.webroot_path[-1]
+                    config.webroot_map[domain] = config.webroot_path[-1]
 
 
 def setup_log_file_handler(args, logfile, fmt):

letsencrypt/plugins/webroot.py

@@ -15,7 +15,6 @@ from letsencrypt.plugins import common
 logger = logging.getLogger(__name__)
 
 
-
 class Authenticator(common.Plugin):
     """Webroot Authenticator."""
     zope.interface.implements(interfaces.IAuthenticator)
@@ -72,9 +71,10 @@ to serve all files under specified web root ({0})."""
         return [self._perform_single(achall) for achall in achalls]
 
     def _path_for_achall(self, achall):
-        path = self.full_roots[achall.domain]
-        if not path:
-            raise errors.PluginError("Cannot find path {0} for domain: {1}"
+        try:
+            path = self.full_roots[achall.domain]
+        except IndexError:
+            raise errors.PluginError("Cannot find webroot path for domain: {1}"
                         .format(path, achall.domain))
         return os.path.join(path, achall.chall.encode("token"))