Tweak terminal colour handling.

CHANGES.md

@@ -100,6 +100,9 @@ invocation for remote scheduler host on a shared filesystem.
 large inexact offsets (year, months); restore time check for old-style
 (task-property) clock triggers.
 
+[#4568](https://github.com/cylc/cylc-flow/pull/4568) - Disable all CLI colour
+output if not to a terminal.
+
 [#4553](https://github.com/cylc/cylc-flow/pull/4553) - Add job submit time
 to the datastore.
 

cylc/flow/option_parsers.py

@@ -17,15 +17,26 @@
 
 from contextlib import suppress
 import logging
-from optparse import OptionParser, OptionConflictError, Values, Option
+from optparse import (
+    OptionParser,
+    OptionConflictError,
+    Values,
+    Option,
+    IndentedHelpFormatter
+)
 import os
 import re
-from ansimarkup import parse as cparse
+from ansimarkup import (
+    parse as cparse,
+    strip as cstrip
+)
+
 import sys
 from textwrap import dedent
 from typing import Any, Dict, Optional, List, Tuple
 
 from cylc.flow import LOG, RSYNC_LOG
+from cylc.flow.terminal import supports_color
 import cylc.flow.flags
 from cylc.flow.loggingutil import (
     CylcLogFormatter,
@@ -118,6 +129,41 @@ def take_action(self, action, dest, opt, value, values, parser):
             Option.take_action(self, action, dest, opt, value, values, parser)
 
 
+class CylcHelpFormatter(IndentedHelpFormatter):
+    def _format(self, text):
+        """Format help (usage) text on the fly to handle coloring.
+
+        Help is printed to the terminal before color initialization for general
+        command output.
+
+        If coloring is wanted:
+          - Add color tags to shell examples
+        Else:
+          - Strip any hardwired color tags
+
+        """
+        if (
+            self.parser.values.color == "always"
+            or (
+                self.parser.values.color == "auto"
+                and supports_color()
+            )
+        ):
+            # Add color formatting to examples text.
+            text = format_shell_examples(text)
+        else:
+            # Strip any hardwired formatting
+            text = cstrip(text)
+        return text
+
+    def format_usage(self, usage):
+        return super().format_usage(self._format(usage))
+
+    # If we start using "description" as well as "usage" (also epilog):
+    # def format_description(self, description):
+    #     return super().format_description(self._format(description))
+
+
 class CylcOptionParser(OptionParser):
 
     """Common options for all cylc CLI commands."""
@@ -183,13 +229,9 @@ def __init__(
             else:
                 argdoc = [('WORKFLOW', 'Workflow ID')]
 
-        if '--color=never' not in '='.join(sys.argv[2:]):
-            # Before option parsing, for `--help`, make comments grey in usage.
-            # (This catches both '--color=never' and '--color never'.)
-            usage = format_shell_examples(usage)
-
         if multiworkflow:
             usage += self.MULTIWORKFLOW_USAGE
+
         if multitask:
             usage += self.MULTITASK_USAGE
 
@@ -227,7 +269,12 @@ def __init__(
                 usage += "\n   " + arg[0] + pad + arg[1]
             usage = usage.replace('ARGS', args)
 
-        OptionParser.__init__(self, usage, option_class=CylcOption)
+        OptionParser.__init__(
+            self,
+            usage,
+            option_class=CylcOption,
+            formatter=CylcHelpFormatter()
+        )
 
     def add_std_option(self, *args, **kwargs):
         """Add a standard option, ignoring override."""

cylc/flow/scripts/cylc.py

@@ -646,7 +646,7 @@ def main():
                 # check if this is a command abbreviation or exit
                 command = match_command(command)
             if opts.help_:
-                execute_cmd(command, "--help")
+                execute_cmd(command, *cmd_args, "--help")
             else:
                 if opts.version:
                     cmd_args.append("--version")

cylc/flow/scripts/play.py

@@ -23,6 +23,3 @@
 )
 
 # CLI of "cylc play". See cylc.flow.scheduler_cli for details.
-
-if __name__ == "__main__":
-    main()

cylc/flow/terminal.py

@@ -203,7 +203,7 @@ def wrapper(*api_args: str) -> None:
             wrapped_args, wrapped_kwargs = [], {}
             if parser_function:
                 parser = parser_function()
-                opts, args = parser_function().parse_args(
+                opts, args = parser.parse_args(
                     list(api_args),
                     **parser_kwargs
                 )

tests/functional/cli/05-colour.t

@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#-------------------------------------------------------------------------------
+
+# Test that CLI colour output is disabled if output is not to a terminal.
+# Uses the "script" command to make stdout log file look like a terminal.
+
+. "$(dirname "$0")/test_header"
+set_test_number 8
+
+ANSI='\e\['
+
+# No redirection.
+script -q -c "cylc scan -t rich" log > /dev/null 2>&1
+grep_ok "$ANSI" log -P  # color
+
+script -q -c "cylc scan -t rich --color=never" log > /dev/null 2>&1
+grep_fail "$ANSI" log -P  # no color
+
+# Redirected.
+cylc scan -t rich > log
+grep_fail "$ANSI" log -P  # no color
+
+cylc scan -t rich --color=always > log
+grep_ok "$ANSI" log -P  # color
+
+# Check command help too (gets printed during command line parsing).
+
+# No redirection.
+script -q -c "cylc scan --help" log > /dev/null 2>&1
+grep_ok "$ANSI" log -P  # color
+
+script -q -c "cylc scan --help --color never" log > /dev/null 2>&1
+grep_fail "$ANSI" log -P  # no color
+
+# Redirected.
+cylc scan --help > log
+grep_fail "$ANSI" log -P  # no color
+
+cylc scan --help --color=always > log
+grep_ok "$ANSI" log -P  # color

tests/unit/test_option_parsers.py

@@ -17,6 +17,8 @@
 import pytest
 
 import sys
+import io
+from contextlib import redirect_stdout
 import cylc.flow.flags
 from cylc.flow.option_parsers import CylcOptionParser as COP
 
@@ -29,20 +31,6 @@ def parser():
     return COP('usage')
 
 
-@pytest.fixture(scope='module')
-def parser_nocolor():
-    argv = sys.argv
-    sys.argv = ['cmd', 'arg', '--help', '--color=never']
-    cop = COP(USAGE_WITH_COMMENT)
-    sys.argv = argv
-    return cop
-
-
-@pytest.fixture(scope='module')
-def parser_color():
-    return COP(USAGE_WITH_COMMENT)
-
-
 @pytest.mark.parametrize(
     'args,verbosity', [
         ([], 0),
@@ -66,11 +54,27 @@ def test_verbosity(args, verbosity, parser, monkeypatch):
     assert cylc.flow.flags.verbosity == verbosity
 
 
-def test_help_color(parser_color):
-    """Test for colorized comments in 'cylc cmd --help'."""
-    assert not parser_color.usage.startswith(USAGE_WITH_COMMENT)
+def test_help_color(monkeypatch):
+    """Test for colorized comments in 'cylc cmd --help --color=always'."""
+    # This colorization is done on the fly when help is printed.
+    monkeypatch.setattr("sys.argv", ['cmd', 'foo', '--color=always'])
+    parser = COP(USAGE_WITH_COMMENT)
+    parser.parse_args(None)
+    assert parser.values.color == "always"
+    f = io.StringIO()
+    with redirect_stdout(f):
+        parser.print_help()
+    assert not (f.getvalue()).startswith("Usage: " + USAGE_WITH_COMMENT)
 
 
-def test_help_nocolor(parser_nocolor):
+def test_help_nocolor(monkeypatch):
     """Test for no colorization in 'cylc cmd --help --color=never'."""
-    assert parser_nocolor.usage.startswith(USAGE_WITH_COMMENT)
+    # This colorization is done on the fly when help is printed.
+    monkeypatch.setattr(sys, "argv", ['cmd', 'foo', '--color=never'])
+    parser = COP(USAGE_WITH_COMMENT)
+    parser.parse_args(None)
+    assert parser.values.color == "never"
+    f = io.StringIO()
+    with redirect_stdout(f):
+        parser.print_help()
+    assert (f.getvalue()).startswith("Usage: " + USAGE_WITH_COMMENT)