From e118fd58ea0ded69c59caf64c2cea8b9c25cc879 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:15:59 +0200 Subject: [PATCH] fix: honour an empty NO_COLOR as unset An empty NO_COLOR turned colour off, which no-color.org reads as unset. FORCE_COLOR and CLICOLOR_FORCE now paint a pipe. --check with several platforms asks which to check rather than "Install for:", and --list-platforms says when nothing is detected here. --- install.ps1 | 2 +- install.py | 17 ++++++--- install.sh | 2 +- tests/test_install_experience.py | 60 ++++++++++++++++++++++++++++++++ wiki/installer.md | 3 +- 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/install.ps1 b/install.ps1 index da599e0c..cfb47f8b 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "ea5d767eeb7612312e6a8fbf5f67b578fdc746f2d34ccc46cf96b978be7424ce" + $defaultInstallSha256 = "739a28ac23d5b0d82d8324ecd7665b2501c71c64d60abba175575f7f16f24ddc" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 39a8d488..383f73db 100644 --- a/install.py +++ b/install.py @@ -325,6 +325,9 @@ class Terminal: """What the output stream can show: a live line, colour, or neither. A pipe or a log gets plain lines, and NO_COLOR and TERM=dumb are honoured. + As no-color.org reads it, an empty NO_COLOR is not set: `NO_COLOR= cmd` + turned colour off. FORCE_COLOR or CLICOLOR_FORCE paints a pipe, for a + log viewer that shows colour, and never makes the line live. """ def __init__(self, stream: TextIO | None = None, quiet: bool = False) -> None: @@ -332,8 +335,10 @@ class Terminal: self.quiet = quiet isatty = getattr(self.stream, "isatty", None) self.live = bool(isatty and isatty()) and os.environ.get("TERM") != "dumb" + forced = any(os.environ.get(name, "0") not in ("", "0") + for name in ("FORCE_COLOR", "CLICOLOR_FORCE")) self.colour = ( - self.live and "NO_COLOR" not in os.environ and _enable_ansi() + (self.live or forced) and not os.environ.get("NO_COLOR") and _enable_ansi() ) if not self.colour: self.live = self.live and os.name != "nt" @@ -5680,10 +5685,10 @@ def _manual_usage_hint(os_type: str, listing: str = "") -> list[str]: def _prompt_platform_choice( - platforms: list[tuple[str, Path]], + platforms: list[tuple[str, Path]], title: str = "Install for:", ) -> list[tuple[str, Path]]: """Prompt user to choose among detected platforms.""" - print("\nInstall for:") + print(f"\n{title}") for i, (name, path) in enumerate(platforms, 1): system = detected_os(name, path) label = f"{system}, {display_name(name)} files" if system and system != display_name(name) \ @@ -7343,6 +7348,8 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: for name, path in detected: system = detected_os(name, path) print(f" {name}: {path}" + (f" ({system})" if system else "")) + else: + print("\nDetected on this system: none") return # Platform detection or override. A destination taken from the default @@ -7463,7 +7470,9 @@ def _install(args: argparse.Namespace, real_stdout: TextIO) -> None: ]) if len(platforms) > 1 and not listing and interactive: - platforms = _prompt_platform_choice(platforms) + # --check installs nothing, and "Install for:" said it would. + platforms = _prompt_platform_choice( + platforms, "Check:" if args.check else "Install for:") if args.dest is not None and not listing: _missing_dest(args.dest, interactive and not (args.check or args.dry_run or args.yes)) diff --git a/install.sh b/install.sh index 5bd706ba..aefa54ef 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ esac TEMP_INSTALLER="" TEMP_DIRECTORY="" DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -DEFAULT_INSTALL_SHA256="ea5d767eeb7612312e6a8fbf5f67b578fdc746f2d34ccc46cf96b978be7424ce" +DEFAULT_INSTALL_SHA256="739a28ac23d5b0d82d8324ecd7665b2501c71c64d60abba175575f7f16f24ddc" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install_experience.py b/tests/test_install_experience.py index eb3614c9..243a3c55 100644 --- a/tests/test_install_experience.py +++ b/tests/test_install_experience.py @@ -499,6 +499,66 @@ class ARefusedFolderReportsEachFile(unittest.TestCase): list(range(1, len(downloaded) + 1))) +class _Tty(io.StringIO): + def isatty(self) -> bool: + return True + + +class ColourFollowsNoColorOrg(unittest.TestCase): + """no-color.org: only a NO_COLOR that is set and not empty turns colour + off. An empty one, as `NO_COLOR= cmd` leaves, turned it off too.""" + + def _colour(self, **env: str) -> bool: + clean = {k: v for k, v in os.environ.items() + if k not in ("NO_COLOR", "FORCE_COLOR", "CLICOLOR_FORCE", "TERM")} + with mock.patch.dict(os.environ, {**clean, **env}, clear=True), \ + mock.patch.object(install, "_enable_ansi", return_value=True): + return install.Terminal(_Tty()).colour + + def test_an_empty_no_color_leaves_colour_on(self): + self.assertTrue(self._colour()) + self.assertTrue(self._colour(NO_COLOR="")) + self.assertFalse(self._colour(NO_COLOR="1")) + + def test_force_color_paints_a_pipe(self): + with mock.patch.object(install, "_enable_ansi", return_value=True): + for env, expected in (({}, False), ({"FORCE_COLOR": "1"}, True), + ({"CLICOLOR_FORCE": "1"}, True), + ({"FORCE_COLOR": "0"}, False), + ({"FORCE_COLOR": "1", "NO_COLOR": "1"}, False)): + clean = {k: v for k, v in os.environ.items() + if k not in ("NO_COLOR", "FORCE_COLOR", "CLICOLOR_FORCE")} + with self.subTest(env=env), \ + mock.patch.dict(os.environ, {**clean, **env}, clear=True): + term = install.Terminal(io.StringIO()) + self.assertEqual(term.colour, expected) + self.assertFalse(term.live) + + +class SmallWordings(unittest.TestCase): + def test_check_asks_which_to_check(self): + """--check with several platforms asked "Install for:", and + installs nothing.""" + out = io.StringIO() + with mock.patch("builtins.input", lambda _prompt="": "1"), \ + contextlib.redirect_stdout(out): + install._prompt_platform_choice( + [("emudeck", Path("/e")), ("retroarch", Path("/r"))], "Check:") + self.assertIn("Check:", out.getvalue()) + self.assertNotIn("Install for:", out.getvalue()) + + def test_nothing_detected_is_said(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as home: + env = dict(os.environ, HOME=home, USERPROFILE=home, XDG_CONFIG_HOME=home, + RETROBIOS_OS="linux") + proc = subprocess.run( + [sys.executable, str(REPO_ROOT / "install.py"), "--list-platforms"], + env=env, capture_output=True, text=True, timeout=60, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("Detected on this system: none", proc.stdout) + + class WindowsMandatoryLock(unittest.TestCase): """Issue 82: on Windows the partial copy's lock is mandatory, so hashing it by reopening its path failed with a sharing violation, and every file diff --git a/wiki/installer.md b/wiki/installer.md index f41ad9ad..761300b6 100644 --- a/wiki/installer.md +++ b/wiki/installer.md @@ -1068,7 +1068,8 @@ no event at all. A program reads a missing `end` as that case. | `RETROBIOS_INSTALL_URL` | Where the bootstrap fetches `install.py`. HTTPS only | | `RETROBIOS_INSTALL_SHA256` | The SHA-256 the bootstrap requires of that installer, 64 hex characters. When either of these two is set and the fingerprint does not match, the error names the variable instead of suggesting a retry | | `RETROBIOS_BOOTSTRAP` | Set by `install.sh` (`sh`) and `install.ps1` (`ps1`) for the temporary installer they run, so that its hints give back the one-line command rather than `python3 install.py` | -| `NO_COLOR` | Plain text, no colour, even at a terminal. `TERM=dumb` also turns the live progress line off | +| `NO_COLOR` | Plain text, no colour, even at a terminal, when set to any value but an empty one. `TERM=dumb` also turns the live progress line off | +| `FORCE_COLOR`, `CLICOLOR_FORCE` | Colour even when the output is a pipe or a file, set to any value but `0` or an empty one. `NO_COLOR` wins over both, and the progress line stays one line per file | | `SSL_CERT_FILE`, `SSL_CERT_DIR` | The trusted certificates to check servers with, for a Python that finds none of its own | | `HTTPS_PROXY`, `NO_PROXY` | Honoured for every download through the standard library's default proxy handling: one names the proxy, the other the hosts to reach directly |