Respect NO_COLOR environment variable

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
 
@@ -140,7 +133,7 @@ def get_parsers():
                  'It can be a boolean or "auto" (default) which enables this '
                  '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 +494,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 +513,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,22 @@ class LogLevelOption(OptionBase):
 
 
 class ColoredOutputOption(BooleanOption):
-    BOOLEAN_STATES = dict(configparser.ConfigParser.BOOLEAN_STATES,
+    CHOICES = tuple(configparser.ConfigParser.BOOLEAN_STATES) + ('auto',)
-                          auto=_COLORED_OUTPUT_DEFAULT)
+    DEFAULT = 'auto'
+
+    def get_converter(self):
+        return self.translate
+
+    @staticmethod
+    def translate(val):
+        if 'NO_COLOR' in os.environ:
+            return False
+        elif isinstance(val, bool):
+            return val
+        elif val == 'auto':
+            return sys.stdout.isatty()
+        else:
+            return configparser.ConfigParser.BOOLEAN_STATES[val]
 
 
 _ARG_OPTION_MAPPING = {
@@ -337,12 +350,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 +506,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,20 +61,19 @@ 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)
-            color = self.COLOR_MAP.get(record.levelname)
+        if color:
-            if color:
+            msg = color + msg + self.RESET
-                msg = color + msg + self.RESET
         return msg
 
 
 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
 
-        formatter = ColorFormatter(self.FORMAT)
+        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():

configuration/cdist.cfg.skeleton

@@ -17,6 +17,8 @@
 #     Use a colored output for different log levels.
 #     It can be a boolean or 'auto' (default) which enables this feature if
 #     stdout is a tty and disables it otherwise.
+#     Colored output is always disabled if the NO_COLOR environment variable is
+#     defined (https://no-color.org/).
 # colored_output = auto
 #
 # conf_dir
@@ -51,7 +53,7 @@
 #
 # out_path
 #     Directory to save cdist output in.
-# out_path = 
+# out_path =
 #
 # parallel
 #     Process hosts in parallel. If -1 then the default, number of CPU's in
@@ -77,6 +79,6 @@
 # remote_shell = /bin/sh
 #
 # verbosity
-#     Set verbosity level. Valid values are: 
+#     Set verbosity level. Valid values are:
 #     ERROR, WARNING, INFO, VERBOSE, DEBUG, TRACE and OFF.
 # verbosity = INFO