bugfixes and error handling

ansibledoctor/Annotation.py

@@ -57,7 +57,7 @@ class Annotation:
                 if not line:
                     break
 
-                if re.match(regex, line):
+                if re.match(regex, line.strip()):
                     item = self._get_annotation_data(
                         line, self._annotation_definition["name"])
                     if item:

ansibledoctor/Cli.py

@@ -10,6 +10,7 @@ from ansibledoctor.Config import SingleConfig
 from ansibledoctor.DocumentationGenerator import Generator
 from ansibledoctor.DocumentationParser import Parser
 from ansibledoctor.Utils import SingleLog
+import ansibledoctor.Exception
 
 
 class AnsibleDoctor:
@@ -33,7 +34,7 @@ class AnsibleDoctor:
         parser = argparse.ArgumentParser(
             description="Generate documentation from annotated Ansible roles using templates")
         parser.add_argument("base_dir", nargs="?", help="role directory, (default: current working dir)")
-        parser.add_argument("-c", "--config", nargs="?", help="location of configuration file")
+        parser.add_argument("-c", "--config", nargs="?", dest="config_file", help="location of configuration file")
         parser.add_argument("-o", "--output", action="store", dest="output_dir", type=str,
                             help="output base dir")
         parser.add_argument("-f", "--force", action="store_true", dest="force_overwrite",
@@ -50,12 +51,17 @@ class AnsibleDoctor:
         return parser.parse_args().__dict__
 
     def _get_config(self):
+        try:
             config = SingleConfig(args=self.args)
+        except ansibledoctor.Exception.ConfigError as e:
+            self.log.sysexit_with_message(e)
+
         if config.is_role:
             self.logger.info("Ansible role detected")
         else:
-            self.log.error("No Ansible role detected")
-            sys.exit(1)
+            self.log.sysexit_with_message("No Ansible role detected")
+
+        self.log.set_level(config.config["logging"]["level"])
+        self.logger.info("Using config file {}".format(config.config_file))
 
-        # TODO: user wrapper method to catch config exceptions
         return config

ansibledoctor/Config.py

@@ -5,11 +5,13 @@ import sys
 
 import anyconfig
 import yaml
+import jsonschema.exceptions
 from appdirs import AppDirs
 from jsonschema._utils import format_as_index
 from pkg_resources import resource_filename
 
 from ansibledoctor.Utils import Singleton
+import ansibledoctor.Exception
 
 config_dir = AppDirs("ansible-doctor").user_config_dir
 default_config_file = os.path.join(config_dir, "config.yml")
@@ -36,15 +38,19 @@ class Config():
         """
         self.config_file = None
         self.schema = None
-        self.dry_run = False
         self.args = self._set_args(args)
-        self.config = self._get_config()
+        self.base_dir = self._set_base_dir()
         self.is_role = self._set_is_role() or False
+        self.dry_run = self._set_dry_run() or False
+        self.config = self._get_config()
         self._annotations = self._set_annotations()
 
     def _set_args(self, args):
         defaults = self._get_defaults()
-        self.config_file = args.get("config_file") or default_config_file
+        if args.get("config_file"):
+            self.config_file = os.path.abspath(os.path.expanduser(os.path.expandvars(args.get("config_file"))))
+        else:
+            self.config_file = default_config_file
 
         args.pop("config_file", None)
         tmp_args = dict(filter(lambda item: item[1] is not None, args.items()))
@@ -64,14 +70,13 @@ class Config():
         return tmp_dict
 
     def _get_defaults(self):
-        default_output = os.getcwd()
         default_template = os.path.join(os.path.dirname(os.path.realpath(__file__)), "templates")
         defaults = {
             "logging": {
                 "level": "WARNING",
                 "json": False
             },
-            "output_dir": default_output,
+            "output_dir": os.getcwd(),
             "template_dir": default_template,
             "template": "readme",
             "force_overwrite": False,
@@ -84,18 +89,23 @@ class Config():
     def _get_config(self):
         defaults = self._get_defaults()
         source_files = []
+
         source_files.append(self.config_file)
-        # TODO: support multipel filename formats e.g. .yaml or .ansibledoctor
-        source_files.append(os.path.relpath(
-            os.path.normpath(os.path.join(os.getcwd(), ".ansibledoctor.yml"))))
+        source_files.append(os.path.join(self.base_dir, ".ansibledoctor"))
+        source_files.append(os.path.join(self.base_dir, ".ansibledoctor.yml"))
+        source_files.append(os.path.join(self.base_dir, ".ansibledoctor.yaml"))
         cli_options = self.args
 
         for config in source_files:
             if config and os.path.exists(config):
                 with open(config, "r", encoding="utf8") as stream:
                     s = stream.read()
-                    # TODO: catch malformed files
+                    try:
                         sdict = yaml.safe_load(s)
+                    except yaml.parser.ParserError as e:
+                        message = "{}\n{}".format(e.problem, str(e.problem_mark))
+                        raise ansibledoctor.Exception.ConfigError("Unable to read file", message)
+
                     if self._validate(sdict):
                         anyconfig.merge(defaults, sdict, ac_merge=anyconfig.MS_DICTS)
                         defaults["logging"]["level"] = defaults["logging"]["level"].upper()
@@ -130,23 +140,33 @@ class Config():
         }
         return annotations
 
+    def _set_base_dir(self):
+        if self.args.get("base_dir"):
+            real = os.path.abspath(os.path.expanduser(os.path.expandvars(self.args.get("base_dir"))))
+        else:
+            real = os.getcwd()
+        return real
+
     def _set_is_role(self):
-        if os.path.isdir(os.path.join(os.getcwd(), "tasks")):
+        if os.path.isdir(os.path.join(self.base_dir, "tasks")):
+            return True
+
+    def _set_dry_run(self):
+        if self.args.get("dry_run"):
             return True
 
     def _validate(self, config):
         try:
             anyconfig.validate(config, self.schema, ac_schema_safe=False)
-            return True
-        except Exception as e:
-            schema_error = "Failed validating '{validator}' in schema{schema}".format(
+        except jsonschema.exceptions.ValidationError as e:
+            schema_error = "Failed validating '{validator}' in schema{schema}\n{message}".format(
                 validator=e.validator,
-                schema=format_as_index(list(e.relative_schema_path)[:-1])
+                schema=format_as_index(list(e.relative_schema_path)[:-1]),
+                message=e.message
             )
+            raise ansibledoctor.Exception.ConfigError("Configuration error", schema_error)
 
-            # TODO: raise exception
-            print("{schema}: {msg}".format(schema=schema_error, msg=e.message))
-            sys.exit(999)
+        return True
 
     def _add_dict_branch(self, tree, vector, value):
         key = vector[0]

ansibledoctor/DocumentationGenerator.py

@@ -8,12 +8,14 @@ import os
 import pprint
 import sys
 
+from functools import reduce
 import jinja2.exceptions
 import ruamel.yaml
 from jinja2 import Environment
 from jinja2 import FileSystemLoader
 from six import binary_type
 from six import text_type
+import ansibledoctor.Exception
 
 from ansibledoctor.Config import SingleConfig
 from ansibledoctor.Utils import FileUtils
@@ -26,8 +28,9 @@ class Generator:
         self.extension = "j2"
         self._parser = None
         self.config = SingleConfig()
-        self.log = SingleLog().logger
-        self.log.info("Using template dir: " + self.config.get_template())
+        self.log = SingleLog()
+        self.logger = self.log.logger
+        self.logger.info("Using template dir: " + self.config.get_template())
         self._parser = doc_parser
         self._scan_template()
 
@@ -43,16 +46,16 @@ class Generator:
 
             relative_file = file[len(base_dir) + 1:]
             if ntpath.basename(file)[:1] != "_":
-                self.log.debug("Found template file: " + relative_file)
+                self.logger.debug("Found template file: " + relative_file)
                 self.template_files.append(relative_file)
             else:
-                self.log.debug("Ignoring template file: " + relative_file)
+                self.logger.debug("Ignoring template file: " + relative_file)
 
     def _create_dir(self, directory):
         if not self.config.dry_run:
             os.makedirs(directory, exist_ok=True)
         else:
-            self.log.info("Creating dir: " + directory)
+            self.logger.info("Creating dir: " + directory)
 
     def _write_doc(self):
         files_to_overwite = []
@@ -64,17 +67,19 @@ class Generator:
 
         if len(files_to_overwite) > 0 and self.config.config.get("force_overwrite") is False:
             if not self.config.dry_run:
-                self.log.warn("This files will be overwritten:")
+                self.logger.warn("This files will be overwritten:")
                 print(*files_to_overwite, sep="\n")
-                resulst = FileUtils.query_yes_no("Do you want to continue?")
-                if resulst != "yes":
-                    sys.exit()
+
+                try:
+                    FileUtils.query_yes_no("Do you want to continue?")
+                except ansibledoctor.Exception.InputError:
+                    self.log.sysexit_with_message("Aborted...")
 
         for file in self.template_files:
-            doc_file = self.config.config.get("output_dir") + "/" + file[:-len(self.extension) - 1]
+            doc_file = os.path.join(self.config.config.get("output_dir"), os.path.splitext(file)[0])
             source_file = self.config.get_template() + "/" + file
 
-            self.log.debug("Writing doc output to: " + doc_file + " from: " + source_file)
+            self.logger.debug("Writing doc output to: " + doc_file + " from: " + source_file)
 
             # make sure the directory exists
             self._create_dir(os.path.dirname(os.path.realpath(doc_file)))
@@ -87,19 +92,20 @@ class Generator:
                             # print(json.dumps(self._parser.get_data(), indent=4, sort_keys=True))
                             jenv = Environment(loader=FileSystemLoader(self.config.get_template()), lstrip_blocks=True, trim_blocks=True)
                             jenv.filters["to_nice_yaml"] = self._to_nice_yaml
+                            jenv.filters["deep_get"] = self._deep_get
                             data = jenv.from_string(data).render(self._parser.get_data(), role=self._parser.get_data())
                             if not self.config.dry_run:
                                 with open(doc_file, "wb") as outfile:
                                     outfile.write(data.encode("utf-8"))
-                                    self.log.info("Writing to: " + doc_file)
+                                    self.logger.info("Writing to: " + doc_file)
                             else:
-                                self.log.info("Writing to: " + doc_file)
-                        except jinja2.exceptions.UndefinedError as e:
-                            self.log.error("Jinja2 templating error: <" + str(e) + "> when loading file: '" + file + "', run in debug mode to see full except")
-                            sys.exit(1)
+                                self.logger.info("Writing to: " + doc_file)
+                        except (jinja2.exceptions.UndefinedError, jinja2.exceptions.TemplateSyntaxError)as e:
+                            self.log.sysexit_with_message(
+                                "Jinja2 templating error while loading file: '{}'\n{}".format(file, str(e)))
                         except UnicodeEncodeError as e:
-                            self.log.error("Unable to print special chars: <" + str(e) + ">, run in debug mode to see full except")
-                            sys.exit(1)
+                            self.log.sysexit_with_message(
+                                "Unable to print special characters\n{}".format(str(e)))
 
     def _to_nice_yaml(self, a, indent=4, *args, **kw):
         """Make verbose, human readable yaml."""
@@ -109,6 +115,10 @@ class Generator:
         yaml.dump(a, stream, **kw)
         return stream.getvalue().rstrip()
 
+    def _deep_get(self, _, dictionary, keys, *args, **kw):
+        default = None
+        return reduce(lambda d, key: d.get(key, default) if isinstance(d, dict) else default, keys.split("."), dictionary)
+
     def render(self):
-        self.log.info("Using output dir: " + self.config.config.get("output_dir"))
+        self.logger.info("Using output dir: " + self.config.config.get("output_dir"))
         self._write_doc()

ansibledoctor/DocumentationParser.py

@@ -46,10 +46,13 @@ class Parser:
             if any("meta/main." + ext in rfile for ext in extensions):
                 with open(rfile, "r", encoding="utf8") as yaml_file:
                     try:
-                        data = defaultdict(dict, yaml.load(yaml_file, Loader=yaml.SafeLoader))
+                        data = defaultdict(dict, yaml.safe_load(yaml_file))
                         if data.get("galaxy_info"):
                             for key, value in data.get("galaxy_info").items():
                                 self._data["meta"][key] = {"value": value}
+
+                        if data.get("dependencies") is not None:
+                            self._data["meta"]["dependencies"] = {"value": data.get("dependencies")}
                     except yaml.YAMLError as exc:
                         print(exc)
 
@@ -60,7 +63,6 @@ class Parser:
             self.log.info("Finding annotations for: @" + annotaion)
             self._annotation_objs[annotaion] = Annotation(name=annotaion, files_registry=self._files_registry)
             tags[annotaion] = self._annotation_objs[annotaion].get_details()
-        # print(json.dumps(tags, indent=4, sort_keys=True))
         anyconfig.merge(self._data, tags, ac_merge=anyconfig.MS_DICTS)
 
     def get_data(self):

ansibledoctor/Exception.py

@@ -0,0 +1,22 @@
+#!/usr/bin/env python3
+"""Custom exception definition."""
+
+
+class DoctorError(Exception):
+    """Generic exception class for ansible-doctor."""
+
+    def __init__(self, msg, original_exception=""):
+        super(DoctorError, self).__init__(msg + ("\n%s" % original_exception))
+        self.original_exception = original_exception
+
+
+class ConfigError(DoctorError):
+    """Errors related to config file handling."""
+
+    pass
+
+
+class InputError(DoctorError):
+    """Errors related to config file handling."""
+
+    pass

ansibledoctor/FileRegistry.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 import glob
 import os
+import pathspec
 import sys
 
 from ansibledoctor.Config import SingleConfig
@@ -31,32 +32,18 @@ class Registry:
         :return: None
         """
         extensions = YAML_EXTENSIONS
-        base_dir = os.getcwd()
+        base_dir = self.config.base_dir
+        role_name = os.path.basename(base_dir)
+        excludes = self.config.config.get("exclude_files")
+        excludespec = pathspec.PathSpec.from_lines("gitwildmatch", excludes)
 
         self.log.debug("Scan for files: " + base_dir)
 
         for extension in extensions:
-            for filename in glob.iglob(base_dir + "/**/*." + extension, recursive=True):
-                if self._is_excluded_yaml_file(filename, base_dir):
-                    self.log.debug("Excluding: " + filename)
-                else:
-                    self.log.debug("Adding to role:" + base_dir + " => " + filename)
+            pattern = os.path.join(base_dir, "**/*." + extension)
+            for filename in glob.iglob(pattern, recursive=True):
+                if not excludespec.match_file(filename):
+                    self.log.debug("Adding file to '{}': {}".format(role_name, os.path.relpath(filename, base_dir)))
                     self._doc.append(filename)
-
-    # TODO: not working...
-    def _is_excluded_yaml_file(self, file, base_dir):
-        """
-        Sub method for handling file exclusions based on the full path starts with.
-
-        :param file:
-        :param role_base_dir:
-        :return:
-        """
-        excluded = self.config.config.get("exclude_files")
-
-        is_filtered = False
-        for excluded_dir in excluded:
-            if file.startswith(base_dir + "/" + excluded_dir):
-                is_filtered = True
-
-        return is_filtered
+                else:
+                    self.log.debug("Excluding file: {}".format(os.path.relpath(filename, base_dir)))

ansibledoctor/Utils.py

@@ -8,6 +8,7 @@ from distutils.util import strtobool
 import colorama
 import yaml
 from pythonjsonlogger import jsonlogger
+import ansibledoctor.Exception
 
 CONSOLE_FORMAT = "{}[%(levelname)s]{} %(message)s"
 JSON_FORMAT = "(asctime) (levelname) (message)"
@@ -75,6 +76,7 @@ class Log:
         self.logger.addHandler(self._get_warn_handler(json=json))
         self.logger.addHandler(self._get_info_handler(json=json))
         self.logger.addHandler(self._get_critical_handler(json=json))
+        self.logger.addHandler(self._get_debug_handler(json=json))
         self.logger.propagate = False
 
     def _get_error_handler(self, json=False):
@@ -106,7 +108,7 @@ class Log:
         handler.setLevel(logging.INFO)
         handler.addFilter(LogFilter(logging.INFO))
         handler.setFormatter(MultilineFormatter(
-            self.info(CONSOLE_FORMAT.format(colorama.Fore.BLUE, colorama.Style.RESET_ALL))))
+            self.info(CONSOLE_FORMAT.format(colorama.Fore.CYAN, colorama.Style.RESET_ALL))))
 
         if json:
             handler.setFormatter(MultilineJsonFormatter(JSON_FORMAT))
@@ -125,6 +127,18 @@ class Log:
 
         return handler
 
+    def _get_debug_handler(self, json=False):
+        handler = logging.StreamHandler(sys.stderr)
+        handler.setLevel(logging.DEBUG)
+        handler.addFilter(LogFilter(logging.DEBUG))
+        handler.setFormatter(MultilineFormatter(
+            self.critical(CONSOLE_FORMAT.format(colorama.Fore.BLUE, colorama.Style.RESET_ALL))))
+
+        if json:
+            handler.setFormatter(MultilineJsonFormatter(JSON_FORMAT))
+
+        return handler
+
     def set_level(self, s):
         self.logger.setLevel(s)
 
@@ -159,6 +173,13 @@ class Log:
         """
         return "{}{}{}".format(color, msg, colorama.Style.RESET_ALL)
 
+    def sysexit(self, code=1):
+        sys.exit(code)
+
+    def sysexit_with_message(self, msg, code=1):
+        self.logger.critical(str(msg))
+        self.sysexit(code)
+
 
 class SingleLog(Log, metaclass=Singleton):
     pass
@@ -169,9 +190,8 @@ class FileUtils:
     def create_path(path):
         os.makedirs(path, exist_ok=True)
 
-    # http://code.activestate.com/recipes/577058/
     @staticmethod
-    def query_yes_no(question, default="yes"):
+    def query_yes_no(question, default=True):
         """Ask a yes/no question via input() and return their answer.
 
         "question" is a string that is presented to the user.
@@ -181,25 +201,16 @@ class FileUtils:
 
         The "answer" return value is one of "yes" or "no".
         """
-        valid = {"yes": "yes", "y": "yes", "ye": "yes",
-                 "no": "no", "n": "no"}
-        if default is None:
-            prompt = " [y/n] "
-        elif default == "yes":
+        if default:
             prompt = "[Y/n]"
-        elif default == "no":
-            prompt = " [y/N] "
         else:
-            raise ValueError("Invalid default answer: '%s'" % default)
+            prompt = "[N/y]"
 
-        while 1:
-            choice = input(question + prompt).lower()
-            if default is not None and choice == "":
-                return default
-            elif choice in valid.keys():
-                return valid[choice]
-            else:
-                sys.stdout.write("Please respond with 'yes' or 'no' (or 'y' or 'n').\n")
+        try:
+            choice = input("{} {} ".format(question, prompt)) or default
+            to_bool(choice)
+        except (KeyboardInterrupt, ValueError) as e:
+            raise ansibledoctor.Exception.InputError("Error while reading input", e)
 
 
 def to_bool(string):

ansibledoctor/templates/readme/README.md.j2

@@ -1,9 +1,12 @@
 {% set meta = role.meta | default({}) %}
-# {{ (meta.name | default({"value": "_undefined_"})).value }}
+# {{ name | deep_get(meta, "name.value") | default("_undefined_") }}
+{% if description | deep_get(meta, "description.value") %}
 
-{% if meta.description is defined %}
-{{ meta.description.value }}
+{{ description | deep_get(meta, "description.value") }}
 {% endif %}
 
 {#      Vars      #}
 {% include '_vars.j2'  %}
+
+{#      Meta      #}
+{% include '_meta.j2'  %}

ansibledoctor/templates/readme/_dev_var_dump.txt.j2

@@ -1,4 +0,0 @@
-#============================================================================================================
-# This is a dump of the documentation variable : tags
-#============================================================================================================
-{{ tag | pprint }}

ansibledoctor/templates/readme/_meta.j2

@@ -0,0 +1,25 @@
+{% set meta = role.meta | default({}) %}
+{% if meta %}
+## Dependencies
+
+{% if meta | deep_get(meta, "dependencies.value") %}
+{% for item in meta.dependencies.value %}
+* {{ item }}
+{% endfor %}
+{% else %}
+None.
+{% endif %}
+{% if license | deep_get(meta, "license.value") %}
+
+## License
+
+{{ meta.license.value }}
+{% endif %}
+{% if author | deep_get(meta, "author.value") %}
+
+## Author
+
+{{ meta.author.value }}
+
+{% endif %}
+{% endif %}

ansibledoctor/templates/readme/_vars.j2

@@ -11,13 +11,14 @@
 {{ desc_line }}
 {% endfor %}
 {% endif %}
+{% if item.value is defined and item.value %}
 
 #### Default value
 
 ```YAML
 {{ item.value | to_nice_yaml(indent=2) }}
 ```
-
+{% endif %}
 {% if item.example is defined and item.example %}
 
 #### Example usage

setup.py

@@ -60,6 +60,7 @@ setup(
         "appdirs",
         "colorama",
         "anyconfig",
+        "pathspec",
         "python-json-logger",
         "jsonschema",
         "jinja2"