From 82567b093b6926b096564a3905ec9fffa098f687 Mon Sep 17 00:00:00 2001 From: Duncan Lock Date: Tue, 9 May 2017 18:18:42 -0700 Subject: [PATCH 1/5] Add extra output options Add the following options, both on the command line and in the config: --output_info: Where to output INFO messages: stdout|stderr --output_warn: Where to output WARN messages: stdout|stderr --output_error: Where to output ERROR messages: stdout|stderr --output_fatal: Where to output FATAL messages: stdout|stderr --wrap_long_lines: Wrap long lines of output, or output each message on a single line. --print_options: Print out effective config options, before starting. --- lib/ansiblereview/__main__.py | 17 +++++++ lib/ansiblereview/utils/__init__.py | 72 +++++++++++++++++++++-------- lib/ansiblereview/version.py | 2 +- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/lib/ansiblereview/__main__.py b/lib/ansiblereview/__main__.py index c04f077..8028ec7 100755 --- a/lib/ansiblereview/__main__.py +++ b/lib/ansiblereview/__main__.py @@ -50,6 +50,20 @@ def main(): parser.add_option('-v', dest='log_level', action="store_const", default=logging.WARN, const=logging.INFO, help="Show more verbose output") + parser.add_option('--output_info', dest='output_info', + help="Where to output INFO messages: stdout|stderr") + parser.add_option('--output_warn', dest='output_warn', + help="Where to output WARN messages: stdout|stderr") + parser.add_option('--output_error', dest='output_error', + help="Where to output ERROR messages: stdout|stderr") + parser.add_option('--output_fatal', dest='output_fatal', + help="Where to output FATAL messages: stdout|stderr") + parser.add_option('--wrap_long_lines', dest='wrap_long_lines', + help="Wrap long lines of output, or output each message on a single line.") + parser.add_option('--print_options', dest='print_options', default=False, + help="Print out effective config options, before starting.") + + options, args = parser.parse_args(sys.argv[1:]) settings = read_config(options.configfile) @@ -72,6 +86,9 @@ def main(): warn("Using example lint-rules found at %s" % lint_dir, options, file=sys.stderr) options.lintdir = lint_dir + if options.print_options: + print(str(options)) + if len(args) == 0: candidates = get_candidates_from_diff(sys.stdin) else: diff --git a/lib/ansiblereview/utils/__init__.py b/lib/ansiblereview/utils/__init__.py index 541744a..0159f38 100644 --- a/lib/ansiblereview/utils/__init__.py +++ b/lib/ansiblereview/utils/__init__.py @@ -21,21 +21,29 @@ import configparser -def abort(message, file=sys.stderr): +def abort(message, settings, file=None): + if file is None: + file = getattr(sys, settings.output_abort) print(stringc("FATAL: %s" % message, 'red'), file=file) sys.exit(1) -def error(message, file=sys.stderr): +def error(message, settings, file=None): + if file is None: + file = getattr(sys, settings.output_error) print(stringc("ERROR: %s" % message, 'red'), file=file) -def warn(message, settings, file=sys.stdout): +def warn(message, settings, file=None): + if file is None: + file = getattr(sys, settings.output_warn) if settings.log_level <= logging.WARNING: print(stringc("WARN: %s" % message, 'yellow'), file=file) -def info(message, settings, file=sys.stdout): +def info(message, settings, file=None): + if file is None: + file = getattr(sys, settings.output_info) if settings.log_level <= logging.INFO: print(stringc("INFO: %s" % message, 'green'), file=file) @@ -61,12 +69,12 @@ def is_line_in_ranges(line, ranges): def read_standards(settings): if not settings.rulesdir: - abort("Standards directory is not set on command line or in configuration file - aborting") + abort("Standards directory is not set on command line or in configuration file - aborting", settings) sys.path.append(os.path.abspath(os.path.expanduser(settings.rulesdir))) try: standards = importlib.import_module('standards') except ImportError as e: - abort("Could not import standards from directory %s: %s" % (settings.rulesdir, str(e))) + abort("Could not import standards from directory %s: %s" % (settings.rulesdir, str(e)), settings) return standards @@ -112,6 +120,11 @@ def review(candidate, settings, lines=None): (type(candidate).__name__, candidate.path, candidate.version), settings) + if settings.wrap_long_lines: + br = '\n' + else: + br = ' ' + for standard in standards.standards: if type(candidate).__name__.lower() not in standard.types: continue @@ -122,14 +135,14 @@ def review(candidate, settings, lines=None): if not err.lineno or is_line_in_ranges(err.lineno, lines_ranges(lines))]: if not standard.version: - warn("Best practice \"%s\" not met:\n%s:%s" % - (standard.name, candidate.path, err), settings) + warn("Best practice \"%s\" not met:%s%s:%s" % + (standard.name, br, candidate.path, err), settings) elif LooseVersion(standard.version) > LooseVersion(candidate.version): - warn("Future standard \"%s\" not met:\n%s:%s" % - (standard.name, candidate.path, err), settings) + warn("Future standard \"%s\" not met:%s%s:%s" % + (standard.name, br, candidate.path, err), settings) else: - error("Standard \"%s\" not met:\n%s:%s" % - (standard.name, candidate.path, err)) + error("Standard \"%s\" not met:%s%s:%s" % + (standard.name, br, candidate.path, err), settings) errors = errors + 1 if not result.errors: if not standard.version: @@ -142,23 +155,44 @@ def review(candidate, settings, lines=None): return errors +# TODO: Settings & read_config should probably just use: +# +# config.read(...) +# config._sections - which is a dict of the config file contents class Settings(object): def __init__(self, values): - self.rulesdir = values.get('rulesdir') - self.lintdir = values.get('lintdir') + self.rulesdir = values.get('rulesdir', None) + self.lintdir = values.get('lintdir', None) + self.configfile = values.get('configfile') + self.output_info = values.get('output_info', 'stdout') + self.output_warn = values.get('output_warn', 'stdout') + self.output_error = values.get('output_error', 'stderr') + self.output_fatal = values.get('output_fatal', 'stderr') + + self.wrap_long_lines = values.get('wrap_long_lines', False) + def read_config(config_file): config = configparser.RawConfigParser({'standards': None, 'lint': None}) config.read(config_file) + tmp_settings = dict(configfile=config_file) + if config.has_section('rules'): - return Settings(dict(rulesdir=config.get('rules', 'standards'), - lintdir=config.get('rules', 'lint'), - configfile=config_file)) - else: - return Settings(dict(rulesdir=None, lintdir=None, configfile=config_file)) + tmp_settings['rulesdir'] = config.get('rules', 'standards') + tmp_settings['lintdir'] = config.get('rules', 'lint') + + if config.has_section('output'): + tmp_settings['output_info'] = config.get('output', 'info') + tmp_settings['output_warn'] = config.get('output', 'warn') + tmp_settings['output_error'] = config.get('output', 'error') + tmp_settings['output_fatal'] = config.get('output', 'fatal') + + tmp_settings['wrap_long_lines'] = config.get('output', 'wrap_long_lines') + + return Settings(tmp_settings) class ExecuteResult(object): diff --git a/lib/ansiblereview/version.py b/lib/ansiblereview/version.py index 1139861..df6b1d9 100644 --- a/lib/ansiblereview/version.py +++ b/lib/ansiblereview/version.py @@ -1 +1 @@ -__version__ = '0.13.2' +__version__ = '0.13.3' From f1721e504613ebdf12127c40db3fd603a7ddf5c7 Mon Sep 17 00:00:00 2001 From: Duncan Lock Date: Tue, 9 May 2017 18:29:28 -0700 Subject: [PATCH 2/5] Default for wrap_long_lines was the wronf way around. --- lib/ansiblereview/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansiblereview/utils/__init__.py b/lib/ansiblereview/utils/__init__.py index 0159f38..7d06f32 100644 --- a/lib/ansiblereview/utils/__init__.py +++ b/lib/ansiblereview/utils/__init__.py @@ -171,7 +171,7 @@ def __init__(self, values): self.output_error = values.get('output_error', 'stderr') self.output_fatal = values.get('output_fatal', 'stderr') - self.wrap_long_lines = values.get('wrap_long_lines', False) + self.wrap_long_lines = values.get('wrap_long_lines', True) def read_config(config_file): From bd9c323aacc66cc711e15c59166b983f98cb76ee Mon Sep 17 00:00:00 2001 From: Duncan Lock Date: Tue, 9 May 2017 19:14:14 -0700 Subject: [PATCH 3/5] Convert wrap_long_lines config option to bool, so it actually works. --- lib/ansiblereview/__main__.py | 23 ++++++++++++++++++++--- lib/ansiblereview/utils/__init__.py | 2 +- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/ansiblereview/__main__.py b/lib/ansiblereview/__main__.py index 8028ec7..2e469a8 100755 --- a/lib/ansiblereview/__main__.py +++ b/lib/ansiblereview/__main__.py @@ -11,6 +11,8 @@ from appdirs import AppDirs from pkg_resources import resource_filename +from copy import copy +from optparse import Option, OptionValueError def get_candidates_from_diff(difftext): try: @@ -31,12 +33,25 @@ def get_candidates_from_diff(difftext): return candidates +def check_boolean(option, opt, value): + try: + return value.lower() in ("yes", "true", "t", "1") + except ValueError: + raise OptionValueError( + "option %s: invalid boolean value: %r" % (opt, value)) + +class MyOption (Option): + TYPES = Option.TYPES + ("boolean",) + TYPE_CHECKER = copy(Option.TYPE_CHECKER) + TYPE_CHECKER["boolean"] = check_boolean + + def main(): config_dir = AppDirs("ansible-review", "com.github.willthames").user_config_dir default_config_file = os.path.join(config_dir, "config.ini") parser = optparse.OptionParser("%prog playbook_file|role_file|inventory_file", - version="%prog " + __version__) + version="%prog " + __version__, option_class=MyOption) parser.add_option('-c', dest='configfile', default=default_config_file, help="Location of configuration file: [%s]" % default_config_file) parser.add_option('-d', dest='rulesdir', @@ -58,9 +73,11 @@ def main(): help="Where to output ERROR messages: stdout|stderr") parser.add_option('--output_fatal', dest='output_fatal', help="Where to output FATAL messages: stdout|stderr") - parser.add_option('--wrap_long_lines', dest='wrap_long_lines', + + parser.add_option('--wrap_long_lines', dest='wrap_long_lines', type="boolean", help="Wrap long lines of output, or output each message on a single line.") - parser.add_option('--print_options', dest='print_options', default=False, + + parser.add_option('--print_options', dest='print_options', type="boolean", default=False, help="Print out effective config options, before starting.") diff --git a/lib/ansiblereview/utils/__init__.py b/lib/ansiblereview/utils/__init__.py index 7d06f32..48e9076 100644 --- a/lib/ansiblereview/utils/__init__.py +++ b/lib/ansiblereview/utils/__init__.py @@ -171,7 +171,7 @@ def __init__(self, values): self.output_error = values.get('output_error', 'stderr') self.output_fatal = values.get('output_fatal', 'stderr') - self.wrap_long_lines = values.get('wrap_long_lines', True) + self.wrap_long_lines = values.get('wrap_long_lines', True) == 'True' def read_config(config_file): From 33bc909bc709400c5b622a7c65a08faa624606d8 Mon Sep 17 00:00:00 2001 From: Duncan Lock Date: Tue, 9 May 2017 20:44:52 -0700 Subject: [PATCH 4/5] pep8 extra blank line --- lib/ansiblereview/__main__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ansiblereview/__main__.py b/lib/ansiblereview/__main__.py index 2e469a8..55da7f7 100755 --- a/lib/ansiblereview/__main__.py +++ b/lib/ansiblereview/__main__.py @@ -14,6 +14,7 @@ from copy import copy from optparse import Option, OptionValueError + def get_candidates_from_diff(difftext): try: import unidiff From 1f804cd351186276261270d7af34970adc8e98d4 Mon Sep 17 00:00:00 2001 From: Duncan Lock Date: Tue, 1 Aug 2017 20:28:39 -0700 Subject: [PATCH 5/5] Attempt to ignore folders --- lib/ansiblereview/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/ansiblereview/__init__.py b/lib/ansiblereview/__init__.py index cf24da1..e00ed4e 100644 --- a/lib/ansiblereview/__init__.py +++ b/lib/ansiblereview/__init__.py @@ -170,7 +170,13 @@ class Rolesfile(Unversioned): pass +class Folder(Unversioned): + pass + + def classify(filename): + if (os.path.isdir(filename)): + return Folder(filename) parentdir = os.path.basename(os.path.dirname(filename)) if parentdir in ['tasks']: return Task(filename)