From e11655e3b92df06a4b9a30d9bc2734720c6826c5 Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Thu, 4 May 2017 10:35:57 -0700 Subject: [PATCH] Refactor to use namedtuples because I like names. Changed some function names to better reflect what they do. --- git_lint/git_lint.py | 32 +++++++++-------- git_lint/option_handler.py | 39 ++++++++++++++------- git_lint/options.py | 70 ++++++++++++++++++++------------------ 3 files changed, 80 insertions(+), 61 deletions(-) diff --git a/git_lint/git_lint.py b/git_lint/git_lint.py index d3b62bd..884c81d 100644 --- a/git_lint/git_lint.py +++ b/git_lint/git_lint.py @@ -9,11 +9,18 @@ import re import subprocess import sys import pprint + try: import configparser except ImportError as e: import ConfigParser as configparser +try: # noqa: F401 + from typing import Dict, List, Text, Any, Optional, Union, Callable, Tuple # noqa: F401 +except: # noqa: F401 + pass # noqa: F401 + + _ = gettext.gettext @@ -167,25 +174,23 @@ class MatchFilter: # \___|_||_\___\__|_\_\ |_|_|_||_\__\___|_| /__/ # -def executable_exists(script, label): - if not len(script): - sys.exit( - _('Syntax error in command configuration for {} ').format(label)) +def linter_exists(linter, label): + if not len(linter): + sys.exit(_('Syntax error in linter configuration for {} ').format(label)) - scriptname = script.split(' ').pop(0) - if not len(scriptname): - sys.exit( - _('Syntax error in command configuration for {} ').format(label)) + lintername = linter.split(' ').pop(0) + if not len(lintername): + sys.exit(_('Syntax error in linter configuration for {} ').format(label)) def is_executable(path): return os.path.exists(path) and os.access(path, os.X_OK) - if scriptname.startswith('/'): - return (is_executable(scriptname) and scriptname) or None + if lintername.startswith('/'): + return (is_executable(lintername) and lintername) or None # shutil.which() doesn't appear until Python 3, darnit. possibles = [path for path in - [os.path.join(path, scriptname) + [os.path.join(path, lintername) for path in os.environ.get('PATH').split(':')] if is_executable(path)] @@ -193,10 +198,9 @@ def executable_exists(script, label): def get_linter_status(config): - def get_working_linter_names(config): return [i.name for i in config - if executable_exists(i.linter['command'], i.name)] + if linter_exists(i.linter['command'], i.name)] working_linter_names = get_working_linter_names(config) broken_linter_names = (set([i.name for i in config]) - set(working_linter_names)) @@ -440,7 +444,6 @@ def run_linters(options, config, extras=[]): all_filenames, unfindable_filenames = get_filelist(options, extras) is_lintable = MatchFilter(config) - lintable_filenames = set([filename for filename in all_filenames if is_lintable(filename)]) @@ -450,7 +453,6 @@ def run_linters(options, config, extras=[]): cant_lint_filter = MatchFilter(build_config_subset( broken_linter_names)) - cant_lint_filenames = [filename for filename in lintable_filenames if cant_lint_filter(filename)] diff --git a/git_lint/option_handler.py b/git_lint/option_handler.py index 523ea9c..996d665 100644 --- a/git_lint/option_handler.py +++ b/git_lint/option_handler.py @@ -2,8 +2,19 @@ # Author: Elf M. Sternberg from functools import reduce +from collections import namedtuple import getopt +try: # noqa: F401 + from typing import Dict, List, Text, Any, Optional, Union, Callable, Tuple # noqa: F401 +except: # noqa: F401 + pass # noqa: F401 + + +Option = namedtuple('Option', ['short', 'long', 'takes', 'help', 'conflicts']) # type: str, str, str, str, List[str] + +Arguments = namedtuple('Arguments', ['arguments', 'filenames', 'excluded']) # type: List[str], List[str], List[str] + # This was a lot shorter and smarter in Hy... # A lot of what you see here is separated from git_lint itself, since this will not be @@ -17,6 +28,7 @@ import getopt def cleanup_options(options, commandline): + # type: (List[Option], List[str]) -> Arguments """Takes a table of options and the commandline, and returns a dictionary of those options that appear on the commandline along with any extra arguments. @@ -29,36 +41,39 @@ def cleanup_options(options, commandline): """ def make_option_streamliner(options): + # type: (List[Option]) -> Callable[[Dict[str, str], Option], Dict[str, str]] """Takes a list of option tuples, and returns a function that takes the output of getopt and reduces it to the longopt key and associated values as a dictionary. """ - fullset = {} + fullset = {} # type: Dict[str, str] for option in options: - if option[1]: - fullset['--' + option[1]] = option[1] - if option[0]: - fullset['-' + option[0]] = option[1] + if option.long: + fullset['--' + option.long] = option.long + if option.short: + fullset['-' + option.short] = option.long def streamliner(acc, it): + # type: (Dict[str, str], Option) -> Dict[str, str] acc[fullset[it[0]]] = it[1] return acc return streamliner def remove_conflicted_options(options, request): + # type: (List[Option], Dict[str, str]) -> Tuple[List[str], List[str]] """Takes our list of option tuples, and a cleaned copy of what was requested from getopt, and returns a copy of the request without any options that are marked as superseded, along with the list of superseded options """ - def get_excluded_keys(memo, opt): - return memo + ((len(opt) > 4 and opt[4]) or []) + def get_excluded_keys(memo, option): + return memo + option.conflicts keys = request.keys() - marked = [option for option in options if option[1] in keys] + marked = [option for option in options if option.long in keys] exclude = reduce(get_excluded_keys, marked, []) excluded = [key for key in keys if key in exclude] cleaned = {key: request[key] for key in keys @@ -66,12 +81,12 @@ def cleanup_options(options, commandline): return (cleaned, excluded) def shortoptstogo(i): - return i[0] + ((i[2] and ':') or '') + return i.short + ((i.takes and ':') or '') def longoptstogo(i): - return i[1] + ((i[2] and '=') or '') + return i.long + ((i.takes and '=') or '') - optstringsshort = ''.join([shortoptstogo(opt) for opt in options if opt[0]]) + optstringsshort = ''.join([shortoptstogo(opt) for opt in options if opt.short]) optstringslong = [longoptstogo(opt) for opt in options] (chosen_options, filenames) = getopt.getopt(commandline[1:], optstringsshort, @@ -84,4 +99,4 @@ def cleanup_options(options, commandline): (ret, excluded) = remove_conflicted_options( options, reduce(streamline_options, chosen_options, {})) - return (ret, filenames, excluded) + return Arguments(ret, filenames, excluded) diff --git a/git_lint/options.py b/git_lint/options.py index d18452b..db11b9d 100644 --- a/git_lint/options.py +++ b/git_lint/options.py @@ -1,43 +1,45 @@ import gettext +from option_handler import Option + _ = gettext.gettext OPTIONS = [ - ('o', 'only', True, - _('A comma-separated list of only those linters to run'), ['exclude']), - ('x', 'exclude', True, - _('A comma-separated list of linters to skip'), []), - ('l', 'linters', False, - _('Show the list of configured linters'), []), - ('b', 'base', False, - _('Check all changed files in the repository, not just those in the current directory.'), []), - ('a', 'all', False, - _('Scan all files in the repository, not just those that have changed.'), ['revision']), - ('r', 'revision', True, - _('Scan all files changed between revisions'), []), - (None, 'pr', False, - _('Scan all files changed between head and previous check-in'), ['revision']), - ('e', 'every', False, - _('Short for -b -a: scan everything'), []), - ('w', 'workspace', False, - _('Scan the workspace'), ['staging']), - ('s', 'staging', False, - _('Scan the staging area (useful for pre-commit).'), []), + Option('o', 'only', True, + _('A comma-separated list of only those linters to run'), ['exclude']), + Option('x', 'exclude', True, + _('A comma-separated list of linters to skip'), []), + Option('l', 'linters', False, + _('Show the list of configured linters'), []), + Option('b', 'base', False, + _('Check all changed files in the repository, not just those in the current directory.'), []), + Option('a', 'all', False, + _('Scan all files in the repository, not just those that have changed.'), ['revision']), + Option('r', 'revision', True, + _('Scan all files changed between revisions'), []), + Option(None, 'pr', False, + _('Scan all files changed between head and previous check-in'), ['revision']), + Option('e', 'every', False, + _('Short for -b -a: scan everything'), []), + Option('w', 'workspace', False, + _('Scan the workspace'), ['staging']), + Option('s', 'staging', False, + _('Scan the staging area (useful for pre-commit).'), []), # ('g', 'changes', False, # _("Report lint failures only for diff'd sections"), ['complete']), # ('p', 'complete', False, # _('Report lint failures for all files'), []), - ('t', 'bylinter', False, - _('Group the reports by linter first as they appear in the config file [default]'), []), - ('f', 'byfile', False, - _('Group the reports by file first'), []), - ('d', 'dryrun', False, - _('Dry run - report what would be done, but do not run linters'), []), - ('c', 'config', True, - _('Path to config file'), []), - ('h', 'help', False, - _('This help message'), []), - ('V', 'verbose', False, - _('A slightly more verbose output'), []), - ('v', 'version', False, - _('Version information'), []) + Option('t', 'bylinter', False, + _('Group the reports by linter first as they appear in the config file [default]'), []), + Option('f', 'byfile', False, + _('Group the reports by file first'), []), + Option('d', 'dryrun', False, + _('Dry run - report what would be done, but do not run linters'), []), + Option('c', 'config', True, + _('Path to config file'), []), + Option('h', 'help', False, + _('This help message'), []), + Option('V', 'verbose', False, + _('A slightly more verbose output'), []), + Option('v', 'version', False, + _('Version information'), []) ]