Repository: pimutils/todoman
Pull request #97 (2017-02-15 21:29:40)

Validate colour flag and setting separately
* Validate colour setting when loading the configuration (with  ConfigObj).
* Have click pre-emptively validate the --color flag.


Patch:
diff --git a/todoman/cli.py b/todoman/cli.py
index b9d4c49..e738f63 100644
--- a/todoman/cli.py
+++ b/todoman/cli.py
@@ -74,6 +74,7 @@ _interactive_option = click.option(
               help=('Accept informal descriptions such as "tomorrow" instead '
                     'of a properly formatted date.'))
 @click.option('--colour', '--color', default=None,
+              type=click.Choice(['always', 'auto', 'never']),
               help=('By default todoman will disable colored output if stdout '
                     'is not a TTY (value `auto`). Set to `never` to disable '
                     'colored output entirely, or `always` to enable it '
@@ -87,8 +88,10 @@ def cli(ctx, human_time, color, porcelain):
         config = load_config()
     except ConfigurationException as e:
         raise click.ClickException(e.args[0])
-    ctx.obj = {}
-    ctx.obj['config'] = config
+
+    ctx.obj = {
+        'config': config,
+    }
 
     if porcelain:
         ctx.obj['formatter'] = PorcelainFormatter()
@@ -103,18 +106,13 @@ def cli(ctx, human_time, color, porcelain):
         ctx.color = True
     elif color == 'never':
         ctx.color = False
-    elif color != 'auto':
-        raise click.UsageError('Invalid color setting: Choose from always, '
-                               'never, auto.')
-
-    paths = []
-    for path in glob.iglob(expanduser(config["main"]["path"])):
-        if not isdir(path):
-            continue
-        paths.append(path)
-
-    cache_path = config['main']['cache_path']
-    ctx.obj['db'] = Database(paths, cache_path)
+
+    paths = [
+        path for path in glob.iglob(expanduser(config["main"]["path"]))
+        if isdir(path)
+    ]
+
+    ctx.obj['db'] = Database(paths, config['main']['cache_path'])
 
     if not ctx.invoked_subcommand:
         ctx.invoke(cli.commands["list"])
diff --git a/todoman/configuration.py b/todoman/configuration.py
index cbdb15c..aa394ff 100644
--- a/todoman/configuration.py
+++ b/todoman/configuration.py
@@ -3,7 +3,7 @@ from os.path import exists, join
 
 import xdg.BaseDirectory
 from configobj import ConfigObj, flatten_errors
-from validate import Validator
+from validate import is_option, Validator, VdtValueError
 
 from . import __documentation__
 
@@ -33,6 +33,15 @@ def validate_cache_path(path):
         )
 
 
+def validate_color(color):
+    color = is_option(color)
+    if color == 'always':
+        return True
+    elif color == 'never':
+        return False
+    return None
+
+
 def find_config():
     custom_path = os.environ.get('TODOMAN_CONFIG')
     if custom_path:
@@ -69,5 +78,9 @@ def load_config():
                 ('{} is missing from the {} section of the configuration ' +
                  'file').format(key, section)
             )
+        if isinstance(error, VdtValueError):
+            raise ConfigurationException(
+                'Bad {} setting, {}'.format(key, error.args[0])
+            )
 
     return config


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index c136a53..8e08b6e 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -61,10 +61,20 @@ def test_invalid_color(config, runner):
     )
     result = runner.invoke(cli, ['list'])
     assert result.exception
-    assert "Error: Invalid color setting: Choose from always, never, auto." \
+    assert 'Error: Bad color setting, the value "12" is unacceptable.' \
         in result.output
 
 
+def test_invalid_color_arg(config, runner):
+    config.write(
+        '[main]\n'
+        'path = "/"\n'
+    )
+    result = runner.invoke(cli, ['--color', '12', 'list'])
+    assert result.exception
+    assert 'Usage:' in result.output
+
+
 def test_missing_path(config, runner):
     config.write(
         '[main]\n'