Merge branch 'no-color' into 'master'

Respect NO_COLOR environment variable

See merge request ungleich-public/cdist!887

cdist/argparse.py

@@ -89,13 +89,6 @@ def check_lower_bounded_int(value, lower_bound, name):
     return val
 
 
-def colored_output_type(val):
-    boolean_states = cdist.configuration.ColoredOutputOption.BOOLEAN_STATES
-    if val not in boolean_states.keys():
-        raise argparse.ArgumentError()
-    return boolean_states[val]
-
-
 def get_parsers():
     global parser
 
@@ -135,12 +128,11 @@ def get_parsers():
 
     parser['colored_output'] = argparse.ArgumentParser(add_help=False)
     parser['colored_output'].add_argument(
-            '--colors',
+            '--colors', metavar='WHEN',
-            help='Use a colored output for different log levels.'
+            help="Colorize cdist's output based on log level; "
-                 'It can be a boolean or "auto" (default) which enables this '
+                 "WHEN is 'always', 'never', or 'auto'.",
-                 'feature if stdout is a tty and disables it otherwise.',
             action='store', dest='colored_output', required=False,
-            type=colored_output_type)
+            choices=cdist.configuration.ColoredOutputOption.CHOICES)
 
     parser['beta'] = argparse.ArgumentParser(add_help=False)
     parser['beta'].add_argument(
@@ -501,7 +493,12 @@ def handle_loglevel(args):
     if hasattr(args, 'quiet') and args.quiet:
         args.verbose = _verbosity_level_off
 
-    logging.root.setLevel(_verbosity_level[args.verbose])
+    logging.getLogger().setLevel(_verbosity_level[args.verbose])
+
+
+def handle_log_colors(args):
+    if cdist.configuration.ColoredOutputOption.translate(args.colored_output):
+        cdist.log.DefaultLog.USE_COLORS = True
 
 
 def parse_and_configure(argv, singleton=True):
@@ -515,16 +512,13 @@ def parse_and_configure(argv, singleton=True):
         raise cdist.Error(str(e))
     # Loglevels are handled globally in here
     handle_loglevel(args)
+    handle_log_colors(args)
 
     log = logging.getLogger("cdist")
 
-    config = cfg.get_config()
-    if config.get('GLOBAL', {}).get('colored_output', False):
-        cdist.log.ColorFormatter.USE_COLORS = True
-
     log.verbose("version %s" % cdist.VERSION)
     log.trace('command line args: {}'.format(cfg.command_line_args))
-    log.trace('configuration: {}'.format(config))
+    log.trace('configuration: {}'.format(cfg.get_config()))
     log.trace('configured args: {}'.format(args))
 
     check_beta(vars(args))

cdist/config.py

@@ -203,6 +203,7 @@ class Config(object):
                 cdist.log.setupParallelLogging()
         elif args.timestamp:
             cdist.log.setupTimestampingLogging()
+
         log = logging.getLogger("config")
 
         # No new child process if only one host at a time.

cdist/configuration.py

@@ -48,7 +48,6 @@ _VERBOSITY_VALUES = (
 _ARCHIVING_VALUES = (
     'tar', 'tgz', 'tbz2', 'txz', 'none',
 )
-_COLORED_OUTPUT_DEFAULT = sys.stdout.isatty()
 
 
 class OptionBase:
@@ -249,8 +248,26 @@ class LogLevelOption(OptionBase):
 
 
 class ColoredOutputOption(BooleanOption):
-    BOOLEAN_STATES = dict(configparser.ConfigParser.BOOLEAN_STATES,
+    CHOICES = ('always', 'never', 'auto')
-                          auto=_COLORED_OUTPUT_DEFAULT)
+    DEFAULT = 'auto'
+
+    def get_converter(self):
+        return self.translate
+
+    @staticmethod
+    def translate(val):
+        if isinstance(val, bool):
+            return val
+        elif val == 'always':
+            return True
+        elif val == 'never':
+            return False
+        elif val == 'auto':
+            return 'NO_COLOR' not in os.environ and sys.stdout.isatty()
+
+
+ColoredOutputOption.DEFAULT = ColoredOutputOption.translate(
+    ColoredOutputOption.DEFAULT)
 
 
 _ARG_OPTION_MAPPING = {
@@ -337,12 +354,10 @@ class Configuration(metaclass=Singleton):
     }
 
     ARG_OPTION_MAPPING = _ARG_OPTION_MAPPING
-    ADJUST_ARG_OPTION_MAPPING = {
+    ADJUST_ARG_OPTION_MAPPING = {v: k for k, v in _ARG_OPTION_MAPPING.items()}
-       _ARG_OPTION_MAPPING[key]: key for key in _ARG_OPTION_MAPPING
-    }
     REQUIRED_DEFAULT_CONFIG_VALUES = {
         'GLOBAL': {
-            'colored_output': _COLORED_OUTPUT_DEFAULT,
+            'colored_output': 'auto',
             'verbosity': 0,
         },
     }
@@ -495,8 +510,7 @@ class Configuration(metaclass=Singleton):
             newconfig = self._read_config_file(config_file)
             self._update_config_dict(config, newconfig)
         # command line config file
-        if (self.args and 'config_file' in self.args and
+        if (self.args and self.args.get('config_file', None)):
-                self.args['config_file']):
             newconfig = self._read_config_file(self.args['config_file'])
             self._update_config_dict(config, newconfig)
         # command line

cdist/core/manifest.py

@@ -119,7 +119,7 @@ class Manifest(object):
             '__cdist_log_level': util.log_level_env_var_val(self.log),
             '__cdist_log_level_name': util.log_level_name_env_var_val(
                 self.log),
-            '__cdist_colored_log': str(cdist.log.ColorFormatter.USE_COLORS),
+            '__cdist_colored_log': str(self.log.USE_COLORS).lower(),
         }
 
         if dry_run:

cdist/emulator.py

@@ -129,8 +129,8 @@ class Emulator(object):
             # if invalid __cdist_log_level value
             logging.root.setLevel(logging.WARNING)
 
-        colored_log = self.env.get('__cdist_colored_log', 'False')
+        colored_log = self.env.get('__cdist_colored_log', 'false')
-        cdist.log.ColorFormatter.USE_COLORS = colored_log == 'True'
+        cdist.log.ColorFormatter.USE_COLORS = colored_log == 'true'
 
         self.log = logging.getLogger(self.target_host[0])
 

cdist/log.py

@@ -51,7 +51,6 @@ logging.trace = _trace
 
 
 class ColorFormatter(logging.Formatter):
-    USE_COLORS = False
     RESET = '\033[0m'
     COLOR_MAP = {
         'ERROR': '\033[0;31m',
@@ -62,12 +61,11 @@ class ColorFormatter(logging.Formatter):
         'TRACE': '\033[0;37m',
     }
 
-    def __init__(self, msg):
+    def __init__(self, fmt):
-        super().__init__(msg)
+        super().__init__(fmt=fmt)
 
     def format(self, record):
         msg = super().format(record)
-        if self.USE_COLORS:
         color = self.COLOR_MAP.get(record.levelname)
         if color:
             msg = color + msg + self.RESET
@@ -75,7 +73,7 @@ class ColorFormatter(logging.Formatter):
 
 
 class DefaultLog(logging.Logger):
-
+    USE_COLORS = False
     FORMAT = '%(levelname)s: %(message)s'
 
     class StdoutFilter(logging.Filter):
@@ -90,7 +88,10 @@ class DefaultLog(logging.Logger):
         super().__init__(name)
         self.propagate = False
 
+        if self.USE_COLORS:
             formatter = ColorFormatter(self.FORMAT)
+        else:
+            formatter = logging.Formatter(self.FORMAT)
 
         self.addFilter(self)
 

cdist/test/configuration/__init__.py

@@ -33,7 +33,7 @@ import sys
 my_dir = op.abspath(op.dirname(__file__))
 fixtures = op.join(my_dir, 'fixtures')
 interpolation_config_file = op.join(fixtures, "interpolation-test.cfg")
-colored_output_default = sys.stdout.isatty()
+colored_output_default = 'auto'
 
 
 def newConfigParser():
@@ -187,7 +187,8 @@ class ConfigurationTestCase(test.CdistTestCase):
                 'remote_shell': '/bin/sh',
                 'inventory_dir': None,
                 'cache_path_pattern': None,
-                'colored_output': colored_output_default,
+                'colored_output': cc.ColoredOutputOption.translate(
+                    colored_output_default),
                 'conf_dir': None,
                 'init_manifest': None,
                 'out_path': None,
@@ -587,7 +588,8 @@ class ConfigurationTestCase(test.CdistTestCase):
                 'remote_shell': '/usr/bin/sh',
                 'inventory_dir': None,
                 'cache_path_pattern': None,
-                'colored_output': colored_output_default,
+                'colored_output': cc.ColoredOutputOption.translate(
+                    colored_output_default),
                 'conf_dir': [
                     '/opt/cdist/conf',
                     '/usr/local/share/cdist/conf',
@@ -674,7 +676,8 @@ class ConfigurationTestCase(test.CdistTestCase):
                 'remote_shell': '/usr/bin/sh',
                 'inventory_dir': '/var/db/cdist/inventory',
                 'cache_path_pattern': None,
-                'colored_output': colored_output_default,
+                'colored_output': cc.ColoredOutputOption.translate(
+                    colored_output_default),
                 'conf_dir': [
                     '/opt/cdist/conf',
                     '/usr/local/share/cdist/conf',

configuration/cdist.cfg.skeleton

@@ -14,9 +14,11 @@
 # cache_path_pattern = %h
 #
 # colored_output
-#     Use a colored output for different log levels.
+#     Colorize cdist's output. If enabled, cdist will use different colors for
-#     It can be a boolean or 'auto' (default) which enables this feature if
+#     different log levels.
-#     stdout is a tty and disables it otherwise.
+#     Recognized values are 'always', 'never', and 'auto'.
+#     If the value is 'auto', colors are enabled if stdout is a TTY unless
+#     the NO_COLOR (https://no-color.org/) environment variable is defined.
 # colored_output = auto
 #
 # conf_dir

docs/src/cdist-reference.rst.sh

@@ -222,65 +222,89 @@ __cdist_log_level, __cdist_log_level_name
     | TRACE          | 5               |
     +----------------+-----------------+
 
+    Available for: initial manifest, explorer, type manifest, type explorer,
+    type gencode.
+__cdist_colored_log
+    whether or not cdist's log has colors enabled.
+    Is set to the string ``true`` if cdist's output is using colors,
+    otherwise the variable contains the string ``false``.
+
     Available for: initial manifest, explorer, type manifest, type explorer,
     type gencode.
 __cdist_dry_run
     Is set only when doing dry run (``-n`` flag).
+
     Available for: initial manifest, explorer, type manifest, type explorer,
     type gencode.
 __explorer
     Directory that contains all global explorers.
+
     Available for: initial manifest, explorer, type explorer, shell.
 __files
     Directory that contains content from the "files" subdirectories
     from the configuration directories.
+
     Available for: initial manifest, type manifest, type gencode, shell.
 __manifest
     Directory that contains the initial manifest.
+
     Available for: initial manifest, type manifest, shell.
 __global
     Directory that contains generic output like explorer.
+
     Available for: initial manifest, type manifest, type gencode, shell.
 __messages_in
     File to read messages from.
+
     Available for: initial manifest, type manifest, type gencode.
 __messages_out
     File to write messages.
+
     Available for: initial manifest, type manifest, type gencode.
 __object
     Directory that contains the current object.
+
     Available for: type manifest, type explorer, type gencode and code scripts.
 __object_id
     The type unique object id.
+
     Available for: type manifest, type explorer, type gencode and code scripts.
-    Note: The leading and the trailing "/" will always be stripped (caused by
+
+    | Note: The leading and the trailing "/" will always be stripped (caused by
       the filesystem database and ensured by the core).
-    Note: Double slashes ("//") will not be fixed and result in an error.
+    | Note: Double slashes ("//") will not be fixed and result in an error.
 __object_name
     The full qualified name of the current object.
+
     Available for: type manifest, type explorer, type gencode.
 __target_host
     The host we are deploying to. This is primary variable. It's content is
     literally the one user passed in.
+
     Available for: explorer, initial manifest, type explorer, type manifest, type gencode, shell.
 __target_hostname
     The hostname of host we are deploying to. This variable is derived from
     **__target_host** (using **socket.getaddrinfo(__target_host)** and then
     **socket.gethostbyaddr()**).
+
     Available for: explorer, initial manifest, type explorer, type manifest, type gencode, shell.
 __target_fqdn
     The fully qualified domain name of the host we are deploying to.
     This variable is derived from **__target_host**
     (using **socket.getfqdn()**).
+
     Available for: explorer, initial manifest, type explorer, type manifest, type gencode, shell.
 __target_host_tags
     Comma separated list of target host tags.
+
     Available for: explorer, initial manifest, type explorer, type manifest, type gencode, shell.
 __type
     Path to the current type.
+
     Available for: type manifest, type gencode.
 __type_explorer
     Directory that contains the type explorers.
+
     Available for: type explorer.
 
 Environment variables (for writing)
@@ -345,9 +369,9 @@ CDIST_BETA
     Enable beta functionalities.
 
 CDIST_COLORED_OUTPUT
-    Use a colored output for different log levels.
+    Colorize cdist's output. If enabled, cdist will use different colors for
-    It can be a boolean or 'auto' (default) which enables this feature if
+    different log levels.
-    stdout is a tty and disables it otherwise.
+    Recognized values are 'always', 'never', and 'auto' (the default).
 
 CDIST_CACHE_PATH_PATTERN
     Custom cache path pattern.

docs/src/man1/cdist.rst

@@ -15,21 +15,19 @@ SYNOPSIS
 
     cdist banner [-h] [-l LOGLEVEL] [-q] [-v]
 
-    cdist config [-h] [-l LOGLEVEL] [-q] [-v] [-b]
+    cdist config [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                 [--colors COLORED_OUTPUT] [-g CONFIG_FILE] [-4] [-6]
+                 [-g CONFIG_FILE] [-4] [-6] [-C CACHE_PATH_PATTERN]
-                 [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST]
+                 [-c CONF_DIR] [-i MANIFEST] [-j [JOBS]] [-n] [-o OUT_PATH] [-P]
-                 [-j [JOBS]] [-n] [-o OUT_PATH] [-P]
                  [-R [{tar,tgz,tbz2,txz}]] [-r REMOTE_OUT_PATH]
                  [--remote-copy REMOTE_COPY] [--remote-exec REMOTE_EXEC]
                  [-S] [-I INVENTORY_DIR] [-A] [-a] [-f HOSTFILE]
                  [-p [HOST_MAX]] [-s] [-t]
                  [host [host ...]]
 
-    cdist install [-h] [-l LOGLEVEL] [-q] [-v] [-b]
+    cdist install [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                  [--colors COLORED_OUTPUT] [-g CONFIG_FILE] [-4] [-6]
+                  [-g CONFIG_FILE] [-4] [-6] [-C CACHE_PATH_PATTERN]
-                  [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST]
+                  [-c CONF_DIR] [-i MANIFEST] [-j [JOBS]] [-n] [-o OUT_PATH]
-                  [-j [JOBS]] [-n] [-o OUT_PATH] [-P]
+                  [-P] [-R [{tar,tgz,tbz2,txz}]] [-r REMOTE_OUT_PATH]
-                  [-R [{tar,tgz,tbz2,txz}]] [-r REMOTE_OUT_PATH]
                   [--remote-copy REMOTE_COPY] [--remote-exec REMOTE_EXEC]
                   [-S] [-I INVENTORY_DIR] [-A] [-a] [-f HOSTFILE]
                   [-p [HOST_MAX]] [-s] [-t]
@@ -37,35 +35,31 @@ SYNOPSIS
 
     cdist inventory [-h] {add-host,add-tag,del-host,del-tag,list} ...
 
-    cdist inventory add-host [-h] [-l LOGLEVEL] [-q] [-v] [-b]
+    cdist inventory add-host [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                             [--colors COLORED_OUTPUT] [-g CONFIG_FILE]
+                             [-g CONFIG_FILE] [-I INVENTORY_DIR] [-f HOSTFILE]
-                             [-I INVENTORY_DIR] [-f HOSTFILE]
                              [host [host ...]]
 
-    cdist inventory add-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b]
+    cdist inventory add-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                            [--colors COLORED_OUTPUT] [-g CONFIG_FILE]
+                            [-g CONFIG_FILE] [-I INVENTORY_DIR] [-f HOSTFILE]
-                            [-I INVENTORY_DIR] [-f HOSTFILE] [-T TAGFILE]
-                            [-t TAGLIST]
-                            [host [host ...]]
-
-    cdist inventory del-host [-h] [-l LOGLEVEL] [-q] [-v] [-b]
-                             [--colors COLORED_OUTPUT] [-g CONFIG_FILE]
-                             [-I INVENTORY_DIR] [-a] [-f HOSTFILE]
-                             [host [host ...]]
-
-    cdist inventory del-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b]
-                            [--colors COLORED_OUTPUT] [-g CONFIG_FILE]
-                            [-I INVENTORY_DIR] [-a] [-f HOSTFILE]
                             [-T TAGFILE] [-t TAGLIST]
                             [host [host ...]]
 
-    cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b]
+    cdist inventory del-host [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                         [--colors COLORED_OUTPUT] [-g CONFIG_FILE]
+                             [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a]
-                         [-I INVENTORY_DIR] [-a] [-f HOSTFILE] [-H] [-t]
+                             [-f HOSTFILE]
                              [host [host ...]]
 
-    cdist preos [-h] [-l LOGLEVEL] [-q] [-v] [-c CONF_DIR] [-g CONFIG_FILE]
+    cdist inventory del-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
-                [-L]
+                            [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a]
+                            [-f HOSTFILE] [-T TAGFILE] [-t TAGLIST]
+                            [host [host ...]]
+
+    cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b] [--colors WHEN]
+                         [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a] [-f HOSTFILE]
+                         [-H] [-t]
+                         [host [host ...]]
+
+    cdist preos [-h] [-l LOGLEVEL] [-q] [-v] [-c CONF_DIR] [-g CONFIG_FILE] [-L]
                 [preos] ...
 
     cdist preos [preos-options] debian [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-a ARCH] [-B]
@@ -89,8 +83,7 @@ SYNOPSIS
                                        [-S SCRIPT] [-s SUITE] [-y REMOTE_COPY]
                                        target_dir
 
-    cdist shell [-h] [-l LOGLEVEL] [-q] [-v] [--colors COLORED_OUTPUT]
+    cdist shell [-h] [-l LOGLEVEL] [-q] [-v] [--colors WHEN] [-s SHELL]
-                [-s SHELL]
 
     cdist info [-h] [-a] [-c CONF_DIR] [-e] [-F] [-f] [-g CONFIG_FILE] [-t]
                [pattern]
@@ -111,10 +104,13 @@ All commands accept the following options:
 **-h, --help**
     Show the help screen.
 
-**--colors COLORED_OUTPUT**
+**--colors WHEN**
-    Use a colored output for different log levels.It can
+    Colorize cdist's output. If enabled, cdist will use different colors for
-    be a boolean or "auto" (default) which enables this
+    different log levels.
-    feature if stdout is a tty and disables it otherwise.
+    WHEN recognizes the values 'always', 'never', and 'auto' (the default).
+
+    If the value is 'auto', colored output is enabled if stdout is a TTY
+    unless the NO_COLOR (https://no-color.org/) environment variable is defined.
 
 **-l LOGLEVEL, --log-level LOGLEVEL**
     Set the specified verbosity level. The levels, in
@@ -685,6 +681,9 @@ The possible keywords and their meanings are as follows:
 :strong:`cache_path_pattern`
     Specify cache path pattern.
 
+:strong:`colored_output`
+    Colorize cdist's output. cf. the :code:`--colors` option.
+
 :strong:`conf_dir`
     List of configuration directories separated with the character conventionally
     used by the operating system to separate search path components (as in PATH),
@@ -906,9 +905,7 @@ CDIST_CACHE_PATH_PATTERN
     Custom cache path pattern.
 
 CDIST_COLORED_OUTPUT
-    Use a colored output for different log levels.
+    Colorize cdist's output. cf. the :code:`--colors` option.
-    It can be a boolean or 'auto' (default) which enables this feature if
-    stdout is a tty and disables it otherwise.
 
 CDIST_CONFIG_FILE
     Custom configuration file.