feat(cli): Colour the verdict when stderr is a terminal - #102
Merged
Conversation
[+] PASSED and [-] FAILED differ by one character, so scrolling a CI log a failure looked exactly like a success. The sigil vocabulary was already good and the README documents it; colour reinforces it rather than replacing it. The verdict is green or red and Error: is red. The four lines that lead up to a verdict -- [.] [:] [>] [~] -- are dimmed, so the verdict is what the eye lands on. Dimming only [.] and [:] would have left the rarer trace lines louder than the common ones. The failure list itself stays plain: those lines get copied out of terminals and into issues, and escapes would travel with them. --color takes auto, always or never and defaults to auto, which colours only when stderr is a character device. A pipe or a CI log therefore stays plain without anyone asking, which is the same test that keeps the existing end-to-end suite unaffected. NO_COLOR is honoured, and --color=always wins over it: a variable says what to do absent an instruction, and the flag is one. Detection is os.File.Stat against ModeCharDevice, so no dependency and no platform branching -- this repository has neither a build tag nor a GOOS check anywhere, and a legibility feature is a poor reason to introduce the first of either. ANSI is written on every platform when stderr is a terminal; a console that cannot render it is rarer than the code to detect one, and --color=never and NO_COLOR are both there for it. --color is deliberately not added to envFlags. NO_COLOR already covers setting this from the environment, and a second spelling would mean two ways to say one thing and a seventh entry in a list documented as six. Closes #98 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CrknafJSP5hF8u865cbnqX
A retry is the one trace line that reports trouble without being the verdict, and dimming it filed it with the request and response lines as background. A check that passed on the fourth attempt is not the same news as one that passed on the first, and the log should say so at a glance. Yellow rather than red, because the run has not failed: red is reserved for the verdict and for Error:, and spending it on an attempt that is about to be tried again would mean a passing run printed red lines. This leaves the mid-retry "[-] FAILED <duration>: <error>" line red. It shares its sigil with the final verdict, so the colour cannot be chosen by prefix the way every other line's is, and stating it at the call site would be the first place the sigil stops deciding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CrknafJSP5hF8u865cbnqX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Scrolling a CI log, a failed run looks exactly like a passing one —
[+] PASSEDand[-] FAILEDdiffer by a single character.The output carries no ANSI at all: no TTY check, no
NO_COLORhandling, no escape sequences anywhere in the source. The sigil vocabulary is good and the README documents it in full; nothing reinforces it visually.Solution
Colour the verdict green or red, dim the lines leading up to it, and only when stderr is a terminal.
[~]is yellow rather than dimmed with the rest of the trace: a retry is the one line that reports trouble without being the verdict, and a check that passed on the fourth attempt is not the same news as one that passed on the first. Red stays reserved for the verdict andError:, so a run that recovers never prints a red line.--colortakesauto(default),alwaysornever.autocolours only when stderr is a character device, so a pipe or CI log stays plain without anyone asking — the same property that leaves the existing e2e suite untouched.NO_COLORis honoured;--color=alwaysoverrides it, on the grounds that a variable says what to do absent an instruction and the flag is one.Three decisions worth stating
[>]is dimmed with the rest of the trace,[~]is not. Dimming only[.]and[:]would leave the rarer trace lines louder than the ordinary ones; the retry line is the deliberate exception, for the reason above.The mid-retry
[-] FAILED <duration>: <error>line stays red. It shares its sigil with the final verdict, so its colour cannot be chosen by prefix the way every other line's is — see Open question.The failure list stays plain. Those
- assertion: …lines get copied out of terminals into issues and chat, and escapes would travel with them. It also keeps the change at two choke points (Client.log,dief) instead of threading escapes through the failure-dump assembly.No dependency and no platform branching. Detection is
os.File.Stat()againstModeCharDevice. This repo has neither a build tag nor aGOOScheck anywhere in production code, and a legibility feature is a poor reason to introduce the first of either — so ANSI is written on every platform when stderr is a terminal. A console that cannot render it is rarer than the code needed to detect one, and--color=neverandNO_COLORboth exist for it.--coloris deliberately not added toenvFlags:NO_COLORalready covers the environment, and a second spelling would mean two ways to say one thing plus a seventh entry in a list documented as six.Verification
Unit tests take the terminal and the environment as arguments, so the whole decision table is testable without either. E2E tests cover what a caller observes through a pipe.
Both were mutation-checked — each of these fails the suite:
NO_COLORwrongly beats--color=alwaysautoignores the terminal and always coloursTwo branches are structurally untestable through the piped harness —
autocolouring on a terminal, andNO_COLORsuppressing it — because a pipe is already plain either way, so such a test would pass even if the code ignored both. Rather than ship a green test that proves nothing, those are covered by the unit table and were verified manually against a real pty:Open question
A run that exhausts its retries currently prints a red
[-] FAILEDline per attempt and a redError:, so three attempts produce four red lines for one verdict:Those per-attempt lines only ever appear while retrying, so they are arguably retry logs too and arguably belong in yellow. Left red for now because making them yellow means naming the colour at the call site — the first place the sigil stops deciding, which is an invariant this PR otherwise documents in both the code and
--help.Other Changes
--helpgains aColour:section; the README documents--colorandNO_COLORin Logging Options.Closes #98
🤖 Generated with Claude Code