refactor(functions): consolidate 198 duplicate color declarations into one helper #131

Merged
rootiest merged 11 commits from refactor/shared-color-palette into main 2026-09-08 05:44:17 +00:00
Owner

What

207 set -l c_* declarations across 39 files re-declared the same ~8 output roles, with names already drifted for the same color (c_reset/c_rst, c_cmd/c_bold, c_arg/c_primary). Replaced with functions/__fish_palette.fish (--no-scope-shadowing + bare set, no globals, no eval), called once per function. Five drifted names retired across 67 sites.

functions/fish_prompt.fish's hex Catppuccin palette is untouched — those are arguments to set_color, not captured escapes, and a different concern.

The near-miss this PR's test harness caught

The first conversion attempt de-duplicated the palette call per indentation depth instead of per contiguous run. That silently stripped every escape from logs --help (431 B → 150 B). It passed fish -n. It passed grep -c '__fish_palette' (the call was still in the file, just not in the block that needed it). Only a byte-level diff against the original output caught it. That demonstration is preserved as a comment above test_functions_keep_their_palette in tests/functional.fish — the permanent check is runtime escape-presence, never a static grep, on purpose.

Verification

  • 29/29 cases byte-identical against main (25 --help paths + 4 error paths, stdout and stderr compared separately).
  • The 4 config-settings TUI files with no --help were driven under a real PTY on both trees with an identical keystroke feed: 9 redraws, 15068 bytes, byte-identical.
  • functions/ net −159 lines (excluding the new 63-line helper, which carries the full documented header this repo's conventions require).
  • Startup cost: unchanged (87.5 vs 86.3 ms, then 86.3 vs 89.3 ms on a repeat run — direction flips between runs against a 16–18 ms spread). The helper is autoloaded, so it's never read at shell start.

Scope limits

12 files have no reachable --help and no side-effect-free error path (the __config_settings_* helpers, upgrade, wake-lock, etc.) — covered structurally (proof no rendering line moved), not at runtime. No test cases were added that invoke upgrade or wake-lock — not worth the side effects during a test run.

c_accent and c_ok are both (set_color green) but kept as distinct roles — merging would change rendered output in logs.fish and smart_exit.fish. See AGENTS/JOB-BRIEF-FINDINGS.md in this branch if you want that as a follow-up.

Testing

fish tests/run-tests.fish — 227/227 lint, 17/17 functional, 317/317 vault.

## What 207 `set -l c_*` declarations across 39 files re-declared the same ~8 output roles, with names already drifted for the same color (`c_reset`/`c_rst`, `c_cmd`/`c_bold`, `c_arg`/`c_primary`). Replaced with `functions/__fish_palette.fish` (`--no-scope-shadowing` + bare `set`, no globals, no `eval`), called once per function. Five drifted names retired across 67 sites. `functions/fish_prompt.fish`'s hex Catppuccin palette is untouched — those are arguments *to* `set_color`, not captured escapes, and a different concern. ## The near-miss this PR's test harness caught The first conversion attempt de-duplicated the palette call **per indentation depth** instead of per contiguous run. That silently stripped every escape from `logs --help` (431 B → 150 B). It passed `fish -n`. It passed `grep -c '__fish_palette'` (the call was still in the file, just not in the block that needed it). **Only a byte-level diff against the original output caught it.** That demonstration is preserved as a comment above `test_functions_keep_their_palette` in `tests/functional.fish` — the permanent check is runtime escape-presence, never a static grep, on purpose. ## Verification - 29/29 cases byte-identical against `main` (25 `--help` paths + 4 error paths, stdout and stderr compared separately). - The 4 `config-settings` TUI files with no `--help` were driven under a real PTY on both trees with an identical keystroke feed: 9 redraws, 15068 bytes, byte-identical. - `functions/` net **−159 lines** (excluding the new 63-line helper, which carries the full documented header this repo's conventions require). - Startup cost: unchanged (87.5 vs 86.3 ms, then 86.3 vs 89.3 ms on a repeat run — direction flips between runs against a 16–18 ms spread). The helper is autoloaded, so it's never read at shell start. ## Scope limits 12 files have no reachable `--help` and no side-effect-free error path (the `__config_settings_*` helpers, `upgrade`, `wake-lock`, etc.) — covered structurally (proof no rendering line moved), not at runtime. No test cases were added that invoke `upgrade` or `wake-lock` — not worth the side effects during a test run. `c_accent` and `c_ok` are both `(set_color green)` but kept as distinct roles — merging would change rendered output in `logs.fish` and `smart_exit.fish`. See `AGENTS/JOB-BRIEF-FINDINGS.md` in this branch if you want that as a follow-up. ## Testing `fish tests/run-tests.fish` — 227/227 lint, 17/17 functional, 317/317 vault.
rootiest added 10 commits 2026-09-08 00:58:25 +00:00
Compares rendered stdout and stderr of 29 colour-bearing cases between a
pristine baseline ref and the working tree, in isolated XDG_CONFIG_HOMEs
that carry the gitignored fish_variables so the opinionated guards resolve.

Two traps this harness exists to avoid, both of which silently produce a
meaningless comparison rather than an error:

  - `git archive main` omits fish_variables because it is untracked. Without
    it __fish_config_op_enabled is unresolvable and every guarded function
    short-circuits, so all cases render empty and trivially "match".
  - `qc --help` shells out to aichat and never reaches its colour path
    unless aichat is on PATH; the harness stubs it.
Declared --no-scope-shadowing with a bare set, so the 12 colour roles land
in the caller's scope under the same short names the consuming functions
already interpolate. Keeping the names means the conversion never edits an
output string in 33 of the 38 consumers.

No consumer is converted yet; byte-identity harness still reports 29/29.
The helper is a new file, so its diff is entirely additions and can never
be "purely structural". It declares colours rather than rendering any, and
tests/functional.fish asserts its 12 roles directly.
34 duplicated declarations replaced by 5 calls. Output strings untouched;
byte-identical across 29 harness cases.
27 duplicated declarations replaced by 5 calls. Output strings untouched.

Four of these five files have no --help path, so the byte-identity harness
cannot reach them. Covered instead by driving the real TUI under a pty on
both the baseline and converted trees with an identical paced keystroke
feed (down x3, Tab, Enter, Esc, Tab, down, q):

  9 redraws, 15068 bytes, byte-identical on both sides
  c_sel (ESC[35;1m) x9, c_hi (ESC[37;1m) x8, c_head (ESC[36;1m) x9
  all four draw helpers reached -- draw, pagetab, draw_subcat
  ("cascade default", "(category)"), draw_value ("Allow prev")
44 duplicated declarations replaced by 8 calls. Output strings untouched.

fish-deps, upgrade and wake-lock have no --help path and are deliberately
NOT added to the harness case list -- upgrade and wake-lock have side
effects, and runtime coverage is not worth mutating state during tests.
They are covered by --structural, which proves mechanically that no
rendering line changed.
37 duplicated declarations replaced by 8 calls. Output strings untouched;
byte-identical across 29 harness cases.
25 duplicated declarations replaced by 7 calls. Completes the 33 structural
conversions: 167 declarations removed, 33 calls added, and --structural
confirms every one of the 33 diffs is declaration removals plus inserted
calls only -- no rendering text touched.

functions/y.fish is the one file in the repo with no trailing newline on
main; its original file ending is preserved rather than normalised.
c_rst/c_txt -> c_reset, c_ttl -> c_head, c_bold -> c_cmd, c_primary -> c_arg
across 67 interpolation sites (c_rst 53, c_primary 10, c_bold 2, c_ttl 1,
c_txt 1). Each maps to a variable holding identical bytes, so output is
unchanged. Unlike the 33 structural conversions this edits text that
renders, so it is gated on its own per-file byte-diff against main:

  ok  logs --help         (431 B out, 0 B err)
  ok  mkcd --help         (437 B out, 0 B err)
  ok  qc --help           (936 B out, 0 B err)
  ok  rand_string --help  (883 B out, 0 B err)
  ok  smart_exit --help   (265 B out, 0 B err)
  ok  mkcd                (437 B out, 0 B err)   <- no-args error path

c_accent is deliberately NOT merged into c_ok -- both are (set_color green)
but a role is a semantic slot, not a colour. See JOB-BRIEF-FINDINGS.md.

logs.fish needed two calls (the C4 guard block and the --help block, both at
8-space indentation); it is the only multi-run file in the repository and the
exact trap a depth-deduplicating converter falls into. logs.fish also declared
c_bold without ever interpolating it -- that declaration is simply deleted.
Runtime escape-presence check over 24 user-facing functions.

Deliberately not a static grep for __fish_palette: measured on a broken
logs.fish whose --help block lost its declarations without gaining a call,
`fish -n` exited 0 and `grep -c __fish_palette` returned 1 while the help
output went 431 B -> 150 B with every escape stripped. The full suite was
green too. Only running the function and looking for an \e byte catches it;
that demonstration is recorded as a comment above the test.

Verified red as well as green -- deleting the __fish_palette call from
functions/mkcd.fish yields:

  --help lost its colour: mkcd
  FAIL  test_functions_keep_their_palette
  16/17 passed

fish_prompt.fish is excluded by name (its own hex palette); qc is excluded
because its --help shells out to aichat, which CI lacks.
rootiest added 1 commit 2026-09-08 05:44:02 +00:00
rootiest merged commit 31b89bbac7 into main 2026-09-08 05:44:17 +00:00
rootiest deleted branch refactor/shared-color-palette 2026-09-08 05:44:18 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rootiest/fish-config#131