Skip to content

Commit 484fe83

Browse files
committed
docs(adr): simplify ADR-006 prose, drop em-dashes
1 parent 1e8a8ff commit 484fe83

1 file changed

Lines changed: 30 additions & 58 deletions

File tree

Lines changed: 30 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,83 +1,55 @@
1-
# Hybrid tput + ANSI for terminal output
1+
# Keep ANSI for colors, use tput where it pays off
22

33
* Status: accepted
44
* Deciders: @Chemaclass
55
* Date: 2026-04-29
66

7-
Technical Story: [#247](https://github.com/TypedDevs/bashunit/issues/247) — evaluate replacing hardcoded ANSI escape sequences with `tput` (terminfo).
7+
Technical Story: [#247](https://github.com/TypedDevs/bashunit/issues/247)
88

9-
## Context and Problem Statement
9+
## Context
1010

11-
bashunit emits colored output and screen-clear sequences via hardcoded ANSI escapes (`\e[31m`, `\033[2J\033[H`). `tput` queries the `terminfo` database, adapting to terminal capabilities and providing safer, more portable codes. A previous attempt to fully migrate to `tput` (around PR #245) caused widespread test instability across CI environments, so any move toward `tput` must be incremental and reliable.
11+
bashunit prints colors and clears the screen with hardcoded ANSI escapes (`\e[31m`, `\033[2J\033[H`). The idea floated in #247 was to switch to `tput`, which reads terminfo and is in theory more portable.
1212

13-
Should bashunit replace ANSI escapes with `tput`?
13+
We tried something similar around PR #245 and it broke the test suite across CI envs. Lots of runners ship with `TERM=dumb` or no `TERM` at all, so `tput setaf` returns empty and colored output silently disappears.
1414

15-
## Decision Drivers
15+
So the question is not really "tput or ANSI" but "where does tput actually help us, and where does it just break things?"
1616

17-
* Bash 3.0+ portability (macOS, Linux, BSD, Windows runners)
18-
* Reliability across CI matrices (GitHub Actions, dumb terminals, non-TTY pipelines)
19-
* Minimal behavioral churn — tests and snapshots must keep passing
20-
* Single source of truth for color/control sequences
21-
* Auto-disable color when terminal does not support it (avoid garbled output)
17+
## Options
2218

23-
## Considered Options
19+
* A. Replace every ANSI escape with `tput`.
20+
* B. Keep ANSI everywhere, change nothing.
21+
* C. Keep ANSI for colors. Use tput only where it gives us something ANSI cannot.
2422

25-
* A. Full migration: replace every ANSI escape with `tput`
26-
* B. Status quo: keep ANSI everywhere
27-
* C. Hybrid: keep ANSI as primary mechanism via centralized helper, adopt `tput` for capability probing and select control sequences (e.g. screen clear)
23+
## Decision
2824

29-
## Decision Outcome
25+
Option C.
3026

31-
Chosen option: **C. Hybrid**.
27+
Reasoning:
3228

33-
Rationale: ANSI SGR codes are stable and identical to what `tput setaf` emits on color terminals, so a wholesale `tput` rewrite buys little while introducing the failure modes that broke the previous attempt (e.g. `tput` returning empty strings under `TERM=dumb`, missing `terminfo` entries on stripped-down CI images, subprocess overhead per call). The real wins from `tput` are (1) capability probing (`tput colors`) to auto-disable color when unsupported and (2) portability for non-color sequences like screen clear. We adopt those targeted uses while keeping the existing centralized `bashunit::sgr` helper as the only emitter of color escape sequences.
29+
* For colors, tput just emits the same ANSI codes we already write by hand. The only thing it adds is breaking on dumb terminals.
30+
* For things ANSI cannot do well, like probing whether the terminal supports color at all, or producing the right "clear screen" sequence on weird terminals, tput is genuinely useful.
31+
* We already use `tput cols` in `src/env.sh` with an ANSI/`stty` fallback. Same pattern fits here.
3432

35-
Concretely:
33+
What this PR does:
3634

37-
1. Keep `bashunit::sgr` and `_BASHUNIT_COLOR_*` constants as the only place that emits color sequences. All ad-hoc `\033[...m` literals in `src/` are migrated to these constants.
38-
2. Expose `bashunit::env::supports_color` returning false when `TERM=dumb` or `tput colors` reports fewer than 8 colors. Available for callers that need a capability check. **Not** wired into `colors.sh` init: GitHub Actions sets `TERM=dumb` on runners, and PR #245's instability was caused by exactly this style of auto-disable. Auto-disable is deferred until we add a CI-aware override (e.g. `CI=true` / `FORCE_COLOR`) and validate across the matrix.
39-
3. Replace the hardcoded `printf '\033[2J\033[H'` screen clear with `tput clear` when available, falling back to the ANSI sequence otherwise.
40-
4. `tput` is already used for `tput cols` in `src/env.sh:215-219` — that pattern (probe + ANSI fallback) is the model.
35+
1. All color escapes go through `bashunit::sgr` and the `_BASHUNIT_COLOR_*` constants. No more raw `\033[...m` literals in `src/coverage.sh` or `src/main.sh`.
36+
2. New `bashunit::env::supports_color` (false on `TERM=dumb` or `tput colors < 8`). Exposed but not wired into `colors.sh` init yet. The same auto-disable broke CI in PR #245 and again on the first push of this branch, so it waits until we add a `CI` / `FORCE_COLOR` override.
37+
3. New `bashunit::io::clear_screen` runs `tput clear` and falls back to `\033[2J\033[H` if tput is missing or returns nothing. Replaces the hardcoded clear in `--watch` mode.
4138

42-
### Positive Consequences
39+
## Consequences
4340

44-
* One place (`bashunit::sgr` + `_BASHUNIT_COLOR_*`) to change colors.
45-
* `supports_color` is now available for future use (e.g. CLI auto-detect, theming).
46-
* Screen clear works on terminals where the hardcoded sequence is wrong.
47-
* No subprocess explosion: `tput` is invoked once at init for capability probing, not per emitted color.
48-
* Avoids the failure mode from the previous attempt (per-call `tput setaf` returning empty strings under unusual `TERM` values).
41+
Good:
4942

50-
### Negative Consequences
43+
* One place to change colors.
44+
* Screen clear works on terminals where the literal ANSI is wrong.
45+
* `supports_color` is ready for the next step (auto-detect with a CI override).
5146

52-
* Slight init-time cost for the `tput colors` probe.
53-
* Test suite must mock `tput` in scenarios that depended on guaranteed-color output. Existing tests already mock `tput` for `find_terminal_width` (see `tests/unit/env_test.sh:144`), so the pattern is established.
47+
Bad:
5448

55-
## Pros and Cons of the Options
56-
57-
### A. Full migration
58-
59-
* Good, because terminfo is the canonical Unix way to handle terminals.
60-
* Good, because `tput` adapts to capability quirks (e.g. 8 vs 256 colors).
61-
* Bad, because the previous attempt destabilized CI tests across environments.
62-
* Bad, because every color emission becomes a subprocess call (`$(tput setaf 1)`), measurable overhead in tight loops like the runner output.
63-
* Bad, because `terminfo` databases on minimal CI images may lack capabilities, returning empty strings and silently breaking output.
64-
65-
### B. Status quo
66-
67-
* Good, because it is known to work across the entire current CI matrix.
68-
* Bad, because color does not auto-disable on dumb terminals.
69-
* Bad, because ad-hoc `\033[...m` literals in `coverage.sh` and `main.sh` bypass the centralized helper.
70-
71-
### C. Hybrid (chosen)
72-
73-
* Good, because it gets the practical benefits of `tput` (capability probing, portable control sequences) without the per-emission failure modes.
74-
* Good, because it consolidates color emission through one helper, simplifying future changes (themes, 256-color, truecolor).
75-
* Good, because it aligns with the existing `tput cols` pattern in `env.sh`.
76-
* Bad, because two mechanisms coexist; contributors must know to use the constants, not raw escapes. Mitigated by lint/grep rules and code review.
49+
* Two mechanisms (constants for colors, tput for clear/probe). Contributors need to know not to add raw escapes back.
7750

7851
## Links
7952

80-
* [Issue #247](https://github.com/TypedDevs/bashunit/issues/247)
81-
* [PR #245 — Increase contrast of test results](https://github.com/TypedDevs/bashunit/pull/245)
82-
* [terminfo(5) man page](https://man7.org/linux/man-pages/man5/terminfo.5.html)
83-
* [NO_COLOR specification](https://no-color.org/)
53+
* Issue [#247](https://github.com/TypedDevs/bashunit/issues/247)
54+
* PR [#245](https://github.com/TypedDevs/bashunit/pull/245)
55+
* [NO_COLOR spec](https://no-color.org/)

0 commit comments

Comments
 (0)