From 595a27de5202ec8b434ef78406eb9f229d05769b Mon Sep 17 00:00:00 2001 From: shenxianpeng Date: Sun, 16 Aug 2026 12:04:15 +0000 Subject: [PATCH] fix: support the NO_COLOR convention for disabling color MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit supports_color() from #551 answered FORCE_COLOR, the TTY and TERM, but not NO_COLOR — the variable users actually export globally to opt out of color (https://no-color.org). Any non-empty value now disables color, outranking detection and yielding only to an explicit FORCE_COLOR. Two gaps in the #551 tests are closed alongside. The reload-based tests recomputed the module constants under a patched environment and left the last reload's values in place for every test that ran afterwards; a fixture now re-derives them on teardown. And nothing exercised the copies commit_check.util binds at import — the ones the print functions actually read — so two subprocess tests now run the real import path end to end and assert on what gets printed. --- commit_check/__init__.py | 16 ++++-- tests/util_test.py | 111 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/commit_check/__init__.py b/commit_check/__init__.py index e38ad4c..d4f4761 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -22,8 +22,11 @@ def supports_color() -> bool: agent harness), where the escape sequences are noise, so this errs towards saying no when ``stdout`` is not a terminal. - ``FORCE_COLOR`` overrides the detection in both directions: ``0`` turns + ``FORCE_COLOR`` overrides everything else, in both directions: ``0`` turns color off even on a terminal, any other value turns it on even when piped. + ``NO_COLOR`` set to any non-empty value turns color off, per the + convention at https://no-color.org — it is what users export globally, so + it outranks detection but yields to an explicit force. An empty ``TERM`` is the same "no terminal type" signal as ``dumb``: the user has deliberately said there are no terminfo capabilities, so emit @@ -33,6 +36,8 @@ def supports_color() -> bool: forced = os.environ.get("FORCE_COLOR") if forced: return forced != "0" + if os.environ.get("NO_COLOR"): + return False if not sys.stdout.isatty(): return False if os.environ.get("TERM") in ("", "dumb"): @@ -41,10 +46,11 @@ def supports_color() -> bool: # ANSI color codes used for CLI output, empty when stdout cannot render color. -RED = "\033[91m" if supports_color() else "" -GREEN = "\033[92m" if supports_color() else "" -YELLOW = "\033[93m" if supports_color() else "" -RESET_COLOR = "\033[0m" if supports_color() else "" +_colored = supports_color() +RED = "\033[91m" if _colored else "" +GREEN = "\033[92m" if _colored else "" +YELLOW = "\033[93m" if _colored else "" +RESET_COLOR = "\033[0m" if _colored else "" # Follow conventional commits DEFAULT_COMMIT_TYPES = [ diff --git a/tests/util_test.py b/tests/util_test.py index e344524..26fb436 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -1,4 +1,6 @@ import importlib +import os +import sys import pytest import subprocess import commit_check @@ -763,12 +765,48 @@ class TestColor: color codes are dropped when ``stdout`` is not a terminal. """ + @pytest.fixture + def restored_module(self): + """Re-derive the module constants after a test that reloads. + + A reload recomputes ``commit_check.RED`` and friends under the + test's patched environment, and nothing else puts them back: the + last reload's values would leak into every test that runs + afterwards. Listed before ``mocker`` in the signature so this + teardown runs after the environment patches are undone. + """ + yield + importlib.reload(commit_check) + @pytest.mark.benchmark def test_not_supported_when_piped(self, mocker): mocker.patch.dict("os.environ", {}, clear=True) mocker.patch("sys.stdout.isatty", return_value=False) assert supports_color() is False + @pytest.mark.benchmark + def test_no_color_turns_color_off_on_a_tty(self, mocker): + """NO_COLOR is what users export globally (https://no-color.org).""" + mocker.patch.dict("os.environ", {"NO_COLOR": "1"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is False + + @pytest.mark.benchmark + def test_no_color_empty_falls_through_to_detection(self, mocker): + """The convention counts only a non-empty value as set.""" + mocker.patch.dict("os.environ", {"NO_COLOR": ""}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is True + + @pytest.mark.benchmark + def test_force_color_outranks_no_color(self, mocker): + """An explicit force wins over the global opt-out.""" + mocker.patch.dict( + "os.environ", {"NO_COLOR": "1", "FORCE_COLOR": "1"}, clear=True + ) + mocker.patch("sys.stdout.isatty", return_value=False) + assert supports_color() is True + @pytest.mark.benchmark def test_forced_even_when_piped(self, mocker): mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) @@ -809,7 +847,7 @@ def test_unset_term_still_allows_color_on_a_tty(self, mocker): assert supports_color() is True @pytest.mark.benchmark - def test_constants_empty_when_color_off(self, mocker): + def test_constants_empty_when_color_off(self, restored_module, mocker): """The constants are pre-emptied when stdout cannot render color.""" mocker.patch.dict("os.environ", {"TERM": "dumb"}, clear=True) mocker.patch("sys.stdout.isatty", return_value=True) @@ -820,7 +858,7 @@ def test_constants_empty_when_color_off(self, mocker): assert commit_check.RESET_COLOR == "" @pytest.mark.benchmark - def test_constants_set_when_color_on(self, mocker): + def test_constants_set_when_color_on(self, restored_module, mocker): """FORCE_COLOR=1 keeps the raw escape codes in place.""" mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) mocker.patch("sys.stdout.isatty", return_value=False) @@ -830,6 +868,75 @@ def test_constants_set_when_color_on(self, mocker): assert commit_check.YELLOW == "\033[93m" assert commit_check.RESET_COLOR == "\033[0m" + # Not benchmarked: these spawn an interpreter, and their cost is the + # process, not the code under test. + + def test_the_decision_reaches_printed_output(self): + """FORCE_COLOR=1 must colour what the print path emits. + + The reload tests above stop at the module constants, but the print + functions in ``commit_check.util`` hold their own copies, bound + once at import. Only a fresh interpreter exercises that hand-off, + so this is the test that fails if the decision stops reaching the + output a user sees. + """ + result = subprocess.run( + [ + sys.executable, + "-c", + "from commit_check.util import print_error_message;" + "print_error_message('message', 'err', 'value', rule_id='CC001')", + ], + capture_output=True, + encoding="utf-8", + env={**os.environ, "FORCE_COLOR": "1"}, + ) + assert result.returncode == 0, result.stderr + assert "\033[91m" in result.stdout + + #: Child program that pretends its stdout is a terminal before the + #: import, so terminal detection says yes and the environment becomes + #: the deciding factor. A plain pipe would disable color on its own + #: and mask whether NO_COLOR handling exists at all. + _TTY_CHILD = ( + "import sys, types;" + "out = sys.stdout;" + "sys.stdout = types.SimpleNamespace(" + "write=out.write, flush=out.flush, isatty=lambda: True);" + "from commit_check.util import print_error_message;" + "print_error_message('message', 'err', 'value', rule_id='CC001')" + ) + + def test_no_color_reaches_printed_output(self): + """NO_COLOR must be what decides, not the pipe. + + Two identical runs, differing only in ``NO_COLOR``. The first + proves the pretend-TTY works — it must come out colored, or the + second assertion would pass even with the NO_COLOR handling + deleted. + """ + env = {**os.environ, "TERM": "xterm"} + env.pop("FORCE_COLOR", None) + env.pop("NO_COLOR", None) + + colored = subprocess.run( + [sys.executable, "-c", self._TTY_CHILD], + capture_output=True, + encoding="utf-8", + env=env, + ) + assert colored.returncode == 0, colored.stderr + assert "\033[" in colored.stdout + + plain = subprocess.run( + [sys.executable, "-c", self._TTY_CHILD], + capture_output=True, + encoding="utf-8", + env={**env, "NO_COLOR": "1"}, + ) + assert plain.returncode == 0, plain.stderr + assert "\033[" not in plain.stdout + class TestPrintSuggestion: @pytest.mark.benchmark def test_print_suggestion(self, capfd):