diff options
author | Lucas Hoffmann <lucc@users.noreply.github.com> | 2017-01-27 10:27:57 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-01-27 10:27:57 +0100 |
commit | 9d0a67842e5a9c85f08d619d60360cb90255b6a5 (patch) | |
tree | 7f1709304d33c7e4db6c568a265515a1909fd902 | |
parent | b7497be293683eb5fe6b761939dd245de2b292e8 (diff) | |
parent | 83d706d86142c693cdc92c4824b38c96ad0e4988 (diff) |
Merge pull request #1000 from dcbaker/pr/argument-validator
argparse validators
-rw-r--r-- | alot/__main__.py | 42 | ||||
-rw-r--r-- | alot/commands/envelope.py | 6 | ||||
-rw-r--r-- | alot/commands/globals.py | 12 | ||||
-rw-r--r-- | alot/commands/thread.py | 10 | ||||
-rw-r--r-- | alot/completion.py | 6 | ||||
-rw-r--r-- | alot/settings/manager.py | 19 | ||||
-rw-r--r-- | alot/settings/theme.py | 13 | ||||
-rw-r--r-- | alot/utils/argparse.py | 136 | ||||
-rw-r--r-- | alot/utils/booleanaction.py | 33 | ||||
-rw-r--r-- | alot/utils/configobj.py (renamed from alot/settings/checks.py) | 3 | ||||
-rwxr-xr-x | docs/source/generate_commands.py | 2 | ||||
-rw-r--r-- | tests/utils/__init__.py | 0 | ||||
-rw-r--r-- | tests/utils/argparse_test.py | 154 | ||||
-rw-r--r-- | tests/utils/configobj_test.py (renamed from tests/settings/checks_test.py) | 2 |
14 files changed, 353 insertions, 85 deletions
diff --git a/alot/__main__.py b/alot/__main__.py index 3bb71c47..48826f53 100644 --- a/alot/__main__.py +++ b/alot/__main__.py @@ -15,55 +15,71 @@ from alot.db.manager import DBManager from alot.ui import UI from alot.commands import * from alot.commands import CommandParseError, COMMANDS +from alot.utils import argparse as cargparse -def main(): - """The main entry point to alot. It parses the command line and prepares - for the user interface main loop to run.""" - # set up the parser to parse the command line options. +_SUBCOMMANDS = ['search', 'compose', 'bufferlist', 'taglist', 'pyshell'] + + +def parser(): + """Parse command line arguments, validate them, and return them.""" parser = argparse.ArgumentParser() parser.add_argument('-v', '--version', action='version', version=alot.__version__) parser.add_argument('-r', '--read-only', action='store_true', help='open db in read only mode') - parser.add_argument('-c', '--config', help='config file', - type=lambda x: argparse.FileType('r')(x).name) + parser.add_argument('-c', '--config', + action=cargparse.ValidatedStoreAction, + validator=cargparse.require_file, + help='config file') parser.add_argument('-n', '--notmuch-config', default=os.environ.get( 'NOTMUCH_CONFIG', os.path.expanduser('~/.notmuch-config')), - type=lambda x: argparse.FileType('r')(x).name, + action=cargparse.ValidatedStoreAction, + validator=cargparse.require_file, help='notmuch config') parser.add_argument('-C', '--colour-mode', choices=(1, 16, 256), type=int, default=256, help='terminal colour mode [default: %(default)s].') - parser.add_argument('-p', '--mailindex-path', #type=directory, + parser.add_argument('-p', '--mailindex-path', + action=cargparse.ValidatedStoreAction, + validator=cargparse.require_dir, help='path to notmuch index') parser.add_argument('-d', '--debug-level', default='info', choices=('debug', 'info', 'warning', 'error'), help='debug log [default: %(default)s]') parser.add_argument('-l', '--logfile', default='/dev/null', - type=lambda x: argparse.FileType('w')(x).name, + action=cargparse.ValidatedStoreAction, + validator=cargparse.optional_file_like, help='logfile [default: %(default)s]') # We will handle the subcommands in a seperate run of argparse as argparse # does not support optional subcommands until now. - subcommands = ('search', 'compose', 'bufferlist', 'taglist', 'pyshell') parser.add_argument('command', nargs=argparse.REMAINDER, help='possible subcommands are {}'.format( - ', '.join(subcommands))) + ', '.join(_SUBCOMMANDS))) options = parser.parse_args() + if options.command: # We have a command after the initial options so we also parse that. # But we just use the parser that is already defined for the internal # command that will back this subcommand. parser = argparse.ArgumentParser() subparsers = parser.add_subparsers(dest='subcommand') - for subcommand in subcommands: + for subcommand in _SUBCOMMANDS: subparsers.add_parser(subcommand, parents=[COMMANDS['global'][subcommand][1]]) command = parser.parse_args(options.command) else: command = None + return options, command + + +def main(): + """The main entry point to alot. It parses the command line and prepares + for the user interface main loop to run.""" + options, command = parser() + # logging root_logger = logging.getLogger() for log_handler in root_logger.handlers: @@ -105,7 +121,7 @@ def main(): cmdstring = settings.get('initial_command') except CommandParseError as err: sys.exit(err) - elif command.subcommand in subcommands: + elif command.subcommand in _SUBCOMMANDS: cmdstring = ' '.join(options.command) # set up and start interface diff --git a/alot/commands/envelope.py b/alot/commands/envelope.py index 5082cc8f..d0dce4be 100644 --- a/alot/commands/envelope.py +++ b/alot/commands/envelope.py @@ -26,7 +26,7 @@ from ..errors import GPGProblem from ..helper import email_as_string from ..helper import string_decode from ..settings import settings -from ..utils.booleanaction import BooleanAction +from ..utils import argparse as cargparse MODE = 'envelope' @@ -283,9 +283,9 @@ class SendCommand(Command): @registerCommand(MODE, 'edit', arguments=[ - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'spawn editor in new terminal'}), - (['--refocus'], {'action': BooleanAction, 'default': True, + (['--refocus'], {'action': cargparse.BooleanAction, 'default': True, 'help': 'refocus envelope after editing'})]) class EditCommand(Command): """edit mail""" diff --git a/alot/commands/globals.py b/alot/commands/globals.py index ef085259..9c056cae 100644 --- a/alot/commands/globals.py +++ b/alot/commands/globals.py @@ -34,7 +34,7 @@ from ..widgets.utils import DialogBox from ..db.errors import DatabaseLockedError from ..db.envelope import Envelope from ..settings import settings -from ..utils.booleanaction import BooleanAction +from ..utils import argparse as cargparse MODE = 'global' @@ -173,11 +173,11 @@ class RefreshCommand(Command): @registerCommand( MODE, 'shellescape', arguments=[ - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'run in terminal window'}), - (['--thread'], {'action': BooleanAction, 'default': None, + (['--thread'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'run in separate thread'}), - (['--refocus'], {'action': BooleanAction, + (['--refocus'], {'action': cargparse.BooleanAction, 'help': 'refocus current buffer after command ' 'has finished'}), (['cmd'], {'help': 'command line to execute'})], @@ -404,7 +404,7 @@ class CallCommand(Command): @registerCommand(MODE, 'bclose', arguments=[ (['--redraw'], - {'action': BooleanAction, + {'action': cargparse.BooleanAction, 'help': 'redraw current buffer after command has finished'}), (['--force'], {'action': 'store_true', 'help': 'never ask for confirmation'})]) @@ -686,7 +686,7 @@ class HelpCommand(Command): (['--attach'], {'nargs': '+', 'help': 'attach files'}), (['--omit_signature'], {'action': 'store_true', 'help': 'do not add signature'}), - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'spawn editor in new terminal'}), (['rest'], {'nargs': '*'}), ]) diff --git a/alot/commands/thread.py b/alot/commands/thread.py index 499e06c6..df22e0f1 100644 --- a/alot/commands/thread.py +++ b/alot/commands/thread.py @@ -35,7 +35,7 @@ from ..settings import settings from ..helper import parse_mailcap_nametemplate from ..helper import split_commandstring from ..helper import email_as_string -from ..utils.booleanaction import BooleanAction +from ..utils import argparse as cargparse from ..widgets.globals import AttachmentWidget MODE = 'thread' @@ -108,9 +108,9 @@ def determine_sender(mail, action='reply'): @registerCommand(MODE, 'reply', arguments=[ (['--all'], {'action': 'store_true', 'help': 'reply to all'}), - (['--list'], {'action': BooleanAction, 'default': None, + (['--list'], {'action': cargparse.BooleanAction, 'default': None, 'dest': 'listreply', 'help': 'reply to list'}), - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'open editor in new window'})]) class ReplyCommand(Command): @@ -318,7 +318,7 @@ class ReplyCommand(Command): @registerCommand(MODE, 'forward', arguments=[ (['--attach'], {'action': 'store_true', 'help': 'attach original mail'}), - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'open editor in new window'})]) class ForwardCommand(Command): @@ -470,7 +470,7 @@ class BounceMailCommand(Command): @registerCommand(MODE, 'editnew', arguments=[ - (['--spawn'], {'action': BooleanAction, 'default': None, + (['--spawn'], {'action': cargparse.BooleanAction, 'default': None, 'help': 'open editor in new window'})]) class EditNewCommand(Command): diff --git a/alot/completion.py b/alot/completion.py index 882f10f1..08089930 100644 --- a/alot/completion.py +++ b/alot/completion.py @@ -15,7 +15,7 @@ from . import crypto from . import commands from .buffers import EnvelopeBuffer from .settings import settings -from .utils.booleanaction import BooleanAction +from .utils import argparse as cargparse from .helper import split_commandline from .addressbook import AddressbookError from .errors import CompletionError @@ -263,8 +263,8 @@ class ArgparseOptionCompleter(Completer): for optionstring in act.option_strings: if optionstring.startswith(pref): # append '=' for options that await a string value - if isinstance( - act, (argparse._StoreAction, BooleanAction)): + if isinstance(act, (argparse._StoreAction, + cargparse.BooleanAction)): optionstring += '=' res.append(optionstring) diff --git a/alot/settings/manager.py b/alot/settings/manager.py index 7ec79347..dcdd853a 100644 --- a/alot/settings/manager.py +++ b/alot/settings/manager.py @@ -14,15 +14,11 @@ from ..account import SendmailAccount from ..addressbook.abook import AbookAddressBook from ..addressbook.external import ExternalAddressbook from ..helper import pretty_datetime, string_decode +from ..utils import configobj as checks from .errors import ConfigError from .utils import read_config from .utils import resolve_att -from .checks import force_list -from .checks import mail_container -from .checks import gpg_key -from .checks import attr_triple -from .checks import align_mode from .theme import Theme @@ -60,12 +56,13 @@ class SettingsManager(object): def read_config(self, path): """parse alot's config file from path""" spec = os.path.join(DEFAULTSPATH, 'alot.rc.spec') - newconfig = read_config(path, spec, - checks={'mail_container': mail_container, - 'force_list': force_list, - 'align': align_mode, - 'attrtriple': attr_triple, - 'gpg_key_hint': gpg_key}) + newconfig = read_config( + path, spec, checks={ + 'mail_container': checks.mail_container, + 'force_list': checks.force_list, + 'align': checks.align_mode, + 'attrtriple': checks.attr_triple, + 'gpg_key_hint': checks.gpg_key}) self._config.merge(newconfig) hooks_path = os.path.expanduser(self._config.get('hooksfile')) diff --git a/alot/settings/theme.py b/alot/settings/theme.py index c05a4db0..f2a59763 100644 --- a/alot/settings/theme.py +++ b/alot/settings/theme.py @@ -5,11 +5,8 @@ from __future__ import absolute_import import os +from ..utils import configobj as checks from .utils import read_config -from .checks import align_mode -from .checks import attr_triple -from .checks import width_tuple -from .checks import force_list from .errors import ConfigError DEFAULTSPATH = os.path.join(os.path.dirname(__file__), '..', 'defaults') @@ -26,10 +23,10 @@ class Theme(object): """ self._spec = os.path.join(DEFAULTSPATH, 'theme.spec') self._config = read_config(path, self._spec, - checks={'align': align_mode, - 'widthtuple': width_tuple, - 'force_list': force_list, - 'attrtriple': attr_triple}) + checks={'align': checks.align_mode, + 'widthtuple': checks.width_tuple, + 'force_list': checks.force_list, + 'attrtriple': checks.attr_triple}) self._colours = [1, 16, 256] # make sure every entry in 'order' lists have their own subsections threadline = self._config['search']['threadline'] diff --git a/alot/utils/argparse.py b/alot/utils/argparse.py new file mode 100644 index 00000000..adacdd6e --- /dev/null +++ b/alot/utils/argparse.py @@ -0,0 +1,136 @@ +# encoding=utf-8 +# Copyright (C) 2011-2012 Patrick Totzke <patricktotzke@gmail.com> +# Copyright © 2017 Dylan Baker + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +"""Custom extensions of the argparse module.""" + +from __future__ import absolute_import + +import argparse +import collections +import functools +import itertools +import os +import stat + + +_TRUEISH = ['true', 'yes', 'on', '1', 't', 'y'] +_FALSISH = ['false', 'no', 'off', '0', 'f', 'n'] + + +class ValidationFailed(Exception): + """Exception raised when Validation fails in a ValidatedStoreAction.""" + pass + + +def _boolean(string): + string = string.lower() + if string in _FALSISH: + return False + elif string in _TRUEISH: + return True + else: + raise ValueError('Option must be one of: {}'.format( + ', '.join(itertools.chain(iter(_TRUEISH), iter(_FALSISH))))) + + +def _path_factory(check): + """Create a function that checks paths.""" + + @functools.wraps(check) + def validator(paths): + if isinstance(paths, basestring): + check(paths) + elif isinstance(paths, collections.Sequence): + for path in paths: + check(path) + else: + raise Exception('expected either basestr or sequenc of basstr') + + return validator + + +@_path_factory +def require_file(path): + """Validator that asserts that a file exists. + + This fails if there is nothing at the given path. + """ + if not os.path.isfile(path): + raise ValidationFailed('{} is not a valid file.'.format(path)) + + +@_path_factory +def optional_file_like(path): + """Validator that ensures that if a file exists it regular, a fifo, or a + character device. The file is not required to exist. + + This includes character special devices like /dev/null. + """ + if (os.path.exists(path) and not (os.path.isfile(path) or + stat.S_ISFIFO(os.stat(path).st_mode) or + stat.S_ISCHR(os.stat(path).st_mode))): + raise ValidationFailed( + '{} is not a valid file, character device, or fifo.'.format(path)) + + +@_path_factory +def require_dir(path): + """Validator that asserts that a directory exists. + + This fails if there is nothing at the given path. + """ + if not os.path.isdir(path): + raise ValidationFailed('{} is not a valid directory.'.format(path)) + + +class BooleanAction(argparse.Action): + """Argparse action that can be used to store boolean values.""" + def __init__(self, *args, **kwargs): + kwargs['type'] = _boolean + kwargs['metavar'] = 'BOOL' + argparse.Action.__init__(self, *args, **kwargs) + + def __call__(self, parser, namespace, values, option_string=None): + setattr(namespace, self.dest, values) + + +class ValidatedStoreAction(argparse.Action): + """An action that allows a validation function to be specificied. + + The validator keyword must be a function taking exactly one argument, that + argument is a list of strings or the type specified by the type argument. + It must raise ValidationFailed with a message when validation fails. + """ + + def __init__(self, option_strings, dest=None, nargs=None, default=None, + required=False, type=None, metavar=None, help=None, + validator=None): + super(ValidatedStoreAction, self).__init__( + option_strings=option_strings, dest=dest, nargs=nargs, + default=default, required=required, metavar=metavar, type=type, + help=help) + + self.validator = validator + + def __call__(self, parser, namespace, values, option_string=None): + if self.validator: + try: + self.validator(values) + except ValidationFailed as e: + raise argparse.ArgumentError(self, str(e)) + + setattr(namespace, self.dest, values) diff --git a/alot/utils/booleanaction.py b/alot/utils/booleanaction.py deleted file mode 100644 index 4c9ce1ed..00000000 --- a/alot/utils/booleanaction.py +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright (C) 2011-2012 Patrick Totzke <patricktotzke@gmail.com> -# This file is released under the GNU GPL, version 3 or a later revision. -# For further details see the COPYING file -from __future__ import absolute_import - -import argparse - - -TRUEISH = ['true', 'yes', 'on', '1', 't', 'y'] -FALSISH = ['false', 'no', 'off', '0', 'f', 'n'] - - -def boolean(string): - string = string.lower() - if string in FALSISH: - return False - elif string in TRUEISH: - return True - else: - raise ValueError() - - -class BooleanAction(argparse.Action): - """ - argparse action that can be used to store boolean values - """ - def __init__(self, *args, **kwargs): - kwargs['type'] = boolean - kwargs['metavar'] = 'BOOL' - argparse.Action.__init__(self, *args, **kwargs) - - def __call__(self, parser, namespace, values, option_string=None): - setattr(namespace, self.dest, values) diff --git a/alot/settings/checks.py b/alot/utils/configobj.py index 125d1860..aba61c3d 100644 --- a/alot/settings/checks.py +++ b/alot/utils/configobj.py @@ -5,11 +5,12 @@ from __future__ import absolute_import import mailbox import re -from urwid import AttrSpec, AttrSpecError from urlparse import urlparse + from validate import VdtTypeError from validate import is_list from validate import ValidateError, VdtValueTooLongError, VdtValueError +from urwid import AttrSpec, AttrSpecError from .. import crypto from ..errors import GPGProblem diff --git a/docs/source/generate_commands.py b/docs/source/generate_commands.py index 19719544..99607fbd 100755 --- a/docs/source/generate_commands.py +++ b/docs/source/generate_commands.py @@ -7,7 +7,7 @@ from alot.commands import * from alot.commands import COMMANDS import alot.buffers from argparse import HelpFormatter, SUPPRESS, OPTIONAL, ZERO_OR_MORE, ONE_OR_MORE, PARSER, REMAINDER -from alot.utils.booleanaction import BooleanAction +from alot.utils.argparse import BooleanAction from gettext import gettext as _ import collections as _collections import copy as _copy diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py new file mode 100644 index 00000000..e69de29b --- /dev/null +++ b/tests/utils/__init__.py diff --git a/tests/utils/argparse_test.py b/tests/utils/argparse_test.py new file mode 100644 index 00000000..b76e96ec --- /dev/null +++ b/tests/utils/argparse_test.py @@ -0,0 +1,154 @@ +# encoding=utf-8 +# Copyright © 2017 Dylan Baker + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +"""Tests for alot.utils.argparse""" + +from __future__ import absolute_import + +import argparse +import contextlib +import os +import shutil +import tempfile +import unittest + +from alot.utils import argparse as cargparse + + +class TestValidatedStore(unittest.TestCase): + """Tests for the ValidatedStore action class.""" + + def _argparse(self, args): + """Create an argparse instance with a validator.""" + + def validator(args): + if args == 'fail': + raise cargparse.ValidationFailed + + parser = argparse.ArgumentParser() + parser.add_argument( + 'foo', + action=cargparse.ValidatedStoreAction, + validator=validator) + return parser.parse_args(args) + + def test_validates(self): + # Arparse will raise a SystemExit (calls sys.exit) rather than letting + # the exception cause the program to close. + with self.assertRaises(SystemExit): + self._argparse(['fail']) + + +@contextlib.contextmanager +def temporary_directory(suffix='', prefix='', dir=None): + """Python3 interface implementation. + + Python3 provides a class that can be used as a context manager, which + creates a temporary directory and removes it when the context manager + exits. This function emulates enough of the interface of + TemporaryDirectory, for this module to use, and is designed as a drop in + replacement that can be replaced after the python3 port. + + The only user visible difference is that this does not implement the + cleanup method that TemporaryDirectory does. + """ + directory = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir) + yield directory + shutil.rmtree(directory) + + +class TestRequireFile(unittest.TestCase): + """Tests for the require_file validator.""" + + def test_doesnt_exist(self): + with temporary_directory() as d: + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_file(os.path.join(d, 'doesnt-exist')) + + def test_dir(self): + with temporary_directory() as d: + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_file(d) + + def test_file(self): + with tempfile.NamedTemporaryFile() as f: + cargparse.require_file(f.name) + + def test_char_special(self): + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_file('/dev/null') + + def test_fifo(self): + with temporary_directory() as d: + path = os.path.join(d, 'fifo') + os.mkfifo(path) + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_file(path) + + +class TestRequireDir(unittest.TestCase): + """Tests for the require_dir validator.""" + + def test_doesnt_exist(self): + with temporary_directory() as d: + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_dir(os.path.join(d, 'doesnt-exist')) + + def test_dir(self): + with temporary_directory() as d: + cargparse.require_dir(d) + + def test_file(self): + with tempfile.NamedTemporaryFile() as f: + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_dir(f.name) + + def test_char_special(self): + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_dir('/dev/null') + + def test_fifo(self): + with temporary_directory() as d: + path = os.path.join(d, 'fifo') + os.mkfifo(path) + with self.assertRaises(cargparse.ValidationFailed): + cargparse.require_dir(path) + + +class TestOptionalFileLike(unittest.TestCase): + """Tests for the optional_file_like validator.""" + + def test_doesnt_exist(self): + with temporary_directory() as d: + cargparse.optional_file_like(os.path.join(d, 'doesnt-exist')) + + def test_dir(self): + with temporary_directory() as d: + with self.assertRaises(cargparse.ValidationFailed): + cargparse.optional_file_like(d) + + def test_file(self): + with tempfile.NamedTemporaryFile() as f: + cargparse.optional_file_like(f.name) + + def test_char_special(self): + cargparse.optional_file_like('/dev/null') + + def test_fifo(self): + with temporary_directory() as d: + path = os.path.join(d, 'fifo') + os.mkfifo(path) + cargparse.optional_file_like(path) diff --git a/tests/settings/checks_test.py b/tests/utils/configobj_test.py index 86269c29..9b212f29 100644 --- a/tests/settings/checks_test.py +++ b/tests/utils/configobj_test.py @@ -3,7 +3,7 @@ from __future__ import absolute_import import unittest -from alot.settings import checks +from alot.utils import configobj as checks class TestForceList(unittest.TestCase): |