Merge pull request #1181 from three-comrades/compose_err

Give better feedback on invalid user input.

6 files changed, 50 insertions, 19 deletions

diff --git a/alot/__main__.py b/alot/__main__.py
index fc34164f..ee5e442a 100644
--- a/alot/__main__.py
+++ b/alot/__main__.py
@@ -106,6 +106,8 @@ def main():
         settings.read_config()
         settings.read_notmuch_config()
     except (ConfigError, OSError, IOError) as e:
+        print('Error when parsing a config file. '
+              'See log for potential details.')
         sys.exit(e)
 
     # store options given by config swiches to the settingsManager:
diff --git a/alot/account.py b/alot/account.py
index a4ce439a..c672686d 100644
--- a/alot/account.py
+++ b/alot/account.py
@@ -359,11 +359,11 @@ class SendmailAccount(Account):
         def errb(failure):
             """The callback used on error."""
             termobj = failure.value
-            errmsg = '%s failed with code %s:\n%s' % \
-                (self.cmd, termobj.exitCode, str(failure.value))
+            errmsg = '%s failed with code %s:\n%s\nProcess stderr:\n%s' % \
+                     (self.cmd, termobj.exitCode, str(failure.value),
+                      failure.value.stderr)
             logging.error(errmsg)
             logging.error(failure.getTraceback())
-            logging.error(failure.value.stderr)
             raise SendingMailFailed(errmsg)
 
         # make sure self.mail is a string
diff --git a/alot/commands/globals.py b/alot/commands/globals.py
index 57ddfcb6..bf1cad01 100644
--- a/alot/commands/globals.py
+++ b/alot/commands/globals.py
@@ -35,6 +35,7 @@ from ..widgets.utils import DialogBox
 from ..db.errors import DatabaseLockedError
 from ..db.envelope import Envelope
 from ..settings.const import settings
+from ..settings.errors import ConfigError, NoMatchingAccount
 from ..utils import argparse as cargparse
 
 MODE = 'global'
@@ -816,7 +817,14 @@ class ComposeCommand(Command):
         # find out the right account
         sender = self.envelope.get('From')
         name, addr = email.utils.parseaddr(sender)
-        account = settings.get_account_by_address(addr)
+        try:
+            account = settings.get_account_by_address(addr)
+        except NoMatchingAccount:
+            msg = 'Cannot compose mail - no account found for `%s`' % addr
+            logging.error(msg)
+            ui.notify(msg, priority='error')
+            raise CommandCanceled()
+
         if account is None:
             accounts = settings.get_accounts()
             if not accounts:
@@ -965,4 +973,8 @@ class ReloadCommand(Command):
     """Reload configuration."""
 
     def apply(self, ui):
-        settings.reload()
+        try:
+            settings.reload()
+        except ConfigError as e:
+            ui.notify('Error when reloading config files:\n {}'.format(e),
+                      priority='error')
diff --git a/alot/settings/manager.py b/alot/settings/manager.py
index 98d534fb..dffe1b32 100644
--- a/alot/settings/manager.py
+++ b/alot/settings/manager.py
@@ -79,7 +79,7 @@ class SettingsManager(object):
         """parse alot's config file from path"""
         spec = os.path.join(DEFAULTSPATH, 'alot.rc.spec')
         newconfig = read_config(
-            self.alot_rc_path, spec, checks={
+            self.alot_rc_path, spec, report_extra=True, checks={
                 'mail_container': checks.mail_container,
                 'force_list': checks.force_list,
                 'align': checks.align_mode,
@@ -116,22 +116,19 @@ class SettingsManager(object):
             # tl/dr; If the loop loads a theme it breaks. If it doesn't break,
             # then it raises a ConfigError.
             for dir_ in itertools.chain([themes_dir], data_dirs):
-                if not os.path.isdir(dir_):
-                    logging.warning(
-                        'cannot find theme %s: themes_dir %s is missing',
-                        themestring, dir_)
+                theme_path = os.path.join(dir_, themestring)
+                if not os.path.exists(os.path.expanduser(theme_path)):
+                    logging.warning('Theme `%s` does not exist.', theme_path)
                 else:
-                    theme_path = os.path.join(dir_, themestring)
                     try:
                         self._theme = Theme(theme_path)
                     except ConfigError as e:
-                        logging.warning(
-                            'Theme file %s failed validation: %s',
-                            themestring, e)
+                        raise ConfigError('Theme file `%s` failed '
+                                          'validation:\n%s' % (theme_path, e))
                     else:
                         break
             else:
-                raise ConfigError('Cannot load theme {}, see log for more '
+                raise ConfigError('Could not find theme {}, see log for more '
                                   'information'.format(themestring))
 
         # if still no theme is set, resort to default
diff --git a/alot/settings/theme.py b/alot/settings/theme.py
index f2a59763..d74cb0e7 100644
--- a/alot/settings/theme.py
+++ b/alot/settings/theme.py
@@ -22,7 +22,7 @@ class Theme(object):
         :raises: :class:`~alot.settings.errors.ConfigError`
         """
         self._spec = os.path.join(DEFAULTSPATH, 'theme.spec')
-        self._config = read_config(path, self._spec,
+        self._config = read_config(path, self._spec, report_extra=True,
                                    checks={'align': checks.align_mode,
                                            'widthtuple': checks.width_tuple,
                                            'force_list': checks.force_list,
diff --git a/alot/settings/utils.py b/alot/settings/utils.py
index d87157c3..8d656b79 100644
--- a/alot/settings/utils.py
+++ b/alot/settings/utils.py
@@ -3,14 +3,18 @@
 # For further details see the COPYING file
 from __future__ import absolute_import
 
-from configobj import ConfigObj, ConfigObjError, flatten_errors
+import logging
+
+from configobj import (ConfigObj, ConfigObjError, flatten_errors,
+                       get_extra_values)
 from validate import Validator
 from urwid import AttrSpec
 
 from .errors import ConfigError
 
 
-def read_config(configpath=None, specpath=None, checks=None):
+def read_config(configpath=None, specpath=None, checks=None,
+                report_extra=False):
     """
     get a (validated) config object for given config file path.
 
@@ -21,6 +25,8 @@ def read_config(configpath=None, specpath=None, checks=None):
     :param checks: custom checks to use for validator.
         see `validate docs <http://www.voidspace.org.uk/python/validate.html>`_
     :type checks: dict str->callable,
+    :param report_extra: log if a setting is not present in the spec file
+    :type report_extra: boolean
     :raises: :class:`~alot.settings.errors.ConfigError`
     :rtype: `configobj.ConfigObj`
     """
@@ -30,7 +36,9 @@ def read_config(configpath=None, specpath=None, checks=None):
         config = ConfigObj(infile=configpath, configspec=specpath,
                            file_error=True, encoding='UTF8')
     except ConfigObjError as e:
-        raise ConfigError(e)
+        msg = 'Error when parsing `%s`:\n%s' % (configpath, e)
+        logging.error(msg)
+        raise ConfigError(msg)
     except IOError:
         raise ConfigError('Could not read %s and/or %s'
                           % (configpath, specpath))
@@ -61,6 +69,18 @@ def read_config(configpath=None, specpath=None, checks=None):
                     msg = 'section "%s" is missing' % '.'.join(section_list)
                 error_msg += msg + '\n'
             raise ConfigError(error_msg)
+
+        extra_values = get_extra_values(config) if report_extra else None
+        if extra_values:
+            msg = ['Unknown values were found in `%s`. Please check for '
+                   'typos if a specified setting does not seem to work:'
+                   % configpath]
+            for sections, val in extra_values:
+                if sections:
+                    msg.append('%s: %s' % ('->'.join(sections), val))
+                else:
+                    msg.append(str(val))
+            logging.info('\n'.join(msg))
     return config