refactor(config-settings): shared frame renderer + line-diff redraw #133

2026-09-08T00:58:52Z

rootiest commented

2026-09-08 00:58:52 +00:00

What

__config_settings_draw / _draw_subcat / _draw_value hand-copied the width-tier table, centering prefix, ON/OFF/DEFAULT badge switch, and box-drawing/erase sequence three times (36–42% line-identical pairwise). Consolidated into functions/__config_settings_frame.fish (five verbs: width, title, badge, cursor, row).

file	before	after
`__config_settings_draw.fish`	114	80
`__config_settings_draw_subcat.fish`	85	49
`__config_settings_draw_value.fish`	123	98
total	322	227 (−29.5%)

Flicker regression — resolved

The frame-refactor above measured 1.35× the old redraw cost and made the pre-existing screen flicker more noticeable. Rather than merge that trade-off, functions/config-settings.fish was reworked from "erase whole panel, then redraw whole panel" on every keypress to a line-diff renderer: a new pure function __config_settings_diff_redraw rewrites an on-screen frame in place, touching only the lines that actually changed (bare \n to skip an unchanged line, \e[2K\r<content>\n to rewrite a changed one) — no full-screen erase at all on the common path.

Dispatch (__cs_dispatch_draw) now captures each page's rendered lines into an array and derives panel height from the real output (count $new_frame) instead of a hand-maintained constant, removing a drift risk the old code had.
The main navigation loop and the inline value-editor's keystroke loop both use the diff path when panel height, terminal width, and a 52-column width floor are all satisfied since the last frame; resize, page switches, and sub-category drill-down enter/exit keep the original wrap-aware full erase+redraw (rare, structural changes where one flicker is expected and acceptable).
Design spec: AGENTS/specs/2026-09-08-config-settings-diff-redraw-design.md (repo-local, not tracked — see plan below for the full task history).

Went through a full subagent-driven implementation (5 tasks, each independently reviewed) plus a final whole-branch review that caught one Critical bug (the inline editor's exit repaint had stopped printing after the dispatch refactor, blanking the panel on every commit/cancel of a value edit) and two Important ones (a quoted command-substitution inside math that silently no-ops on resize-while-editing, and the diff path missing wrap-awareness below 52 columns) — all fixed and re-reviewed clean.

Still needs a human interactive pass (no /dev/tty was available to any agent in this pipeline, so nothing above was ever visually confirmed on a real terminal): open config-settings, confirm no flicker on arrow-key nav and on typing in the Sponge/Paths inline editor, and specifically exercise (a) editing a value then pressing Enter/Escape, (b) resizing while the editor is open, (c) a terminal narrower than 52 columns.

The verification gate

A 377-case golden captures raw output bytes — including the \e[<N>A\e[J erase sequence — at every width tier (COLUMNS 100/88/84/70), both scopes, cursor on every row. All commits on this branch produce zero diff against it — the three draw functions' byte output is unchanged throughout, including by the redraw-model rework, which only touches the orchestration around them.

The gate's own blind spot is deliberately demonstrated rather than hidden: at every tier the longest description is exactly the field width (43/43, 39/39, 35/35, 17/17) and Integrations is exactly 12 characters, so collapsing all three pages onto a plain truncate is byte-for-byte identical today and would pass the gate while silently discarding that hand-tuned property. A dedicated test step performs exactly that mutation and asserts the gate stays green — watching it pass is the point. The explicit fit token per call site (pad/cut/shorten) and the comments at each site are what actually protect the property; the golden can't.

Testing

fish tests/run-tests.fish — 233/233 lint, 409/409 assertions, 377/377 golden render cases, all green. New tests: line-count invariant lock and byte-exact coverage of __config_settings_diff_redraw's two line-classes (unchanged/changed), both ported into tests/test-session.fish following main's suite-discovery refactor (see merge note below).

Merged with `main`

This branch had fallen behind (main gained a shared color-palette helper __fish_palette and a full test-suite discovery rework since this branch was cut). Merged and resolved:

__config_settings_draw*.fish's inline set_color declarations → __fish_palette (same colors, shared helper).
tests/functional.fish (deleted upstream, replaced by tests/test-session.fish's check/section idiom) → this branch's 3 new tests ported into tests/test-session.fish in that idiom.
tests/config-settings-render.fish renamed to tests/test-config-settings-render.fish to match the new test-*.fish discovery glob, and its manual function-sourcing loop updated to also source __fish_palette.fish (needed once the draw functions started calling it — caught by the golden gate itself, which failed with every color escape missing until fixed).
tests/run-tests.fish and tests/test-session.fish otherwise taken as-is from main.

Now clean and mergeable.

## What `__config_settings_draw` / `_draw_subcat` / `_draw_value` hand-copied the width-tier table, centering prefix, ON/OFF/DEFAULT badge switch, and box-drawing/erase sequence three times (36–42% line-identical pairwise). Consolidated into `functions/__config_settings_frame.fish` (five verbs: `width`, `title`, `badge`, `cursor`, `row`). | file | before | after | |---|---|---| | `__config_settings_draw.fish` | 114 | 80 | | `__config_settings_draw_subcat.fish` | 85 | 49 | | `__config_settings_draw_value.fish` | 123 | 98 | | **total** | **322** | **227** (−29.5%) | ## Flicker regression — resolved The frame-refactor above measured 1.35× the old redraw cost and made the pre-existing screen flicker more noticeable. Rather than merge that trade-off, `functions/config-settings.fish` was reworked from "erase whole panel, then redraw whole panel" on every keypress to a **line-diff renderer**: a new pure function `__config_settings_diff_redraw` rewrites an on-screen frame in place, touching only the lines that actually changed (bare `\n` to skip an unchanged line, `\e[2K\r<content>\n` to rewrite a changed one) — no full-screen erase at all on the common path. - Dispatch (`__cs_dispatch_draw`) now captures each page's rendered lines into an array and derives panel height from the real output (`count $new_frame`) instead of a hand-maintained constant, removing a drift risk the old code had. - The main navigation loop and the inline value-editor's keystroke loop both use the diff path when panel height, terminal width, **and** a 52-column width floor are all satisfied since the last frame; resize, page switches, and sub-category drill-down enter/exit keep the original wrap-aware full erase+redraw (rare, structural changes where one flicker is expected and acceptable). - Design spec: `AGENTS/specs/2026-09-08-config-settings-diff-redraw-design.md` (repo-local, not tracked — see plan below for the full task history). Went through a full subagent-driven implementation (5 tasks, each independently reviewed) plus a final whole-branch review that caught one Critical bug (the inline editor's exit repaint had stopped printing after the dispatch refactor, blanking the panel on every commit/cancel of a value edit) and two Important ones (a quoted command-substitution inside `math` that silently no-ops on resize-while-editing, and the diff path missing wrap-awareness below 52 columns) — all fixed and re-reviewed clean. **Still needs a human interactive pass** (no `/dev/tty` was available to any agent in this pipeline, so nothing above was ever visually confirmed on a real terminal): open `config-settings`, confirm no flicker on arrow-key nav and on typing in the Sponge/Paths inline editor, and specifically exercise (a) editing a value then pressing Enter/Escape, (b) resizing while the editor is open, (c) a terminal narrower than 52 columns. ## The verification gate A 377-case golden captures raw output bytes — including the `\e[<N>A\e[J` erase sequence — at every width tier (COLUMNS 100/88/84/70), both scopes, cursor on every row. All commits on this branch produce **zero** diff against it — the three draw functions' byte output is unchanged throughout, including by the redraw-model rework, which only touches the orchestration around them. The gate's own blind spot is deliberately demonstrated rather than hidden: at every tier the longest description is exactly the field width (43/43, 39/39, 35/35, 17/17) and `Integrations` is exactly 12 characters, so collapsing all three pages onto a plain truncate is byte-for-byte identical *today* and would pass the gate while silently discarding that hand-tuned property. A dedicated test step performs exactly that mutation and asserts the gate stays green — watching it pass is the point. The explicit `fit` token per call site (`pad`/`cut`/`shorten`) and the comments at each site are what actually protect the property; the golden can't. ## Testing `fish tests/run-tests.fish` — 233/233 lint, 409/409 assertions, 377/377 golden render cases, all green. New tests: line-count invariant lock and byte-exact coverage of `__config_settings_diff_redraw`'s two line-classes (unchanged/changed), both ported into `tests/test-session.fish` following main's suite-discovery refactor (see merge note below). ## Merged with `main` This branch had fallen behind (`main` gained a shared color-palette helper `__fish_palette` and a full test-suite discovery rework since this branch was cut). Merged and resolved: - `__config_settings_draw*.fish`'s inline `set_color` declarations → `__fish_palette` (same colors, shared helper). - `tests/functional.fish` (deleted upstream, replaced by `tests/test-session.fish`'s `check`/`section` idiom) → this branch's 3 new tests ported into `tests/test-session.fish` in that idiom. - `tests/config-settings-render.fish` renamed to `tests/test-config-settings-render.fish` to match the new `test-*.fish` discovery glob, and its manual function-sourcing loop updated to also source `__fish_palette.fish` (needed once the draw functions started calling it — caught by the golden gate itself, which failed with every color escape missing until fixed). - `tests/run-tests.fish` and `tests/test-session.fish` otherwise taken as-is from `main`. Now clean and mergeable.

rootiest added 6 commits 2026-09-08 00:58:53 +00:00

test(config-settings): add byte-exact golden-output render harness 574b235b6f

The three config-settings draw functions are hand-tuned layout code whose
field widths, dash counts and pad targets are arithmetic on the width tier.
Any refactor of them has to be byte-identical, and nothing until now could
prove that.

Renders all 356 page/width/scope/cursor-row combinations -- every page at
COLUMNS 100/88/84/70, both scopes, cursor on every row, plus the inline
editor -- and byte-compares against a committed baseline.

The golden holds raw output: set_color escapes and box drawing exactly as
emitted, plus the wrap-aware \e[<N>A\e[J erase config-settings.fish would
emit for each panel, and per case the declared panel height against the
measured line count. Nothing is normalized; the gate is cmp(1) and a
one-space change anywhere fails it. Proven by four deliberate mutations,
one per draw function plus a panel-height change, each caught and reverted.

Runs entirely inside a throwaway HOME/XDG_CONFIG_HOME sandbox: the fixtures
must be real universal variables, and this repo doubles as a live
~/.config/fish. fish --no-config cannot be used, as -N silently degrades
set -U to global scope. Every utility is called through `command`, since
the config itself shadows rm, cat and mkdir and aliases cp -i.

feat(config-settings): add __config_settings_frame shared renderer 2ac8e5c0d4

The width tier, title-border arithmetic, ON/OFF/DEFAULT badge, cursor cell
and table row are currently hand-copied across the three draw functions.
This adds them once, with the two geometry identities derived rather than
hand-maintained: a row's chrome is a fixed 21 columns, so field_w is
iw - 21 - label_w (reproducing both iw-33 and iw-34), and a title border is
dashes = iw - visible(segment) - 1 (reproducing all three of iw-23,
iw-len-3 and iw-L-S-22).

The frame owns no page height. Every verb prints exactly one line or
fragment, so the fixed-16 category and value pages and the dynamic
7+n sub-category page keep their heights, and config-settings.fish's
erase is unaffected.

No caller yet, so rendering cannot move: the golden's existing page
section is byte-identical (verified with cmp -n over its previous size)
and the file only gains frame-verb cases appended after it.

refactor(config-settings): draw the category list through the frame 889f8e0768

Width tier, title border, badge, cursor cell and row line now come from
__config_settings_frame. The per-tier description sets stay here -- they
are this page's data, authored to fit each tier's field exactly -- and are
selected by width rather than by re-testing $COLUMNS.

Rows pass the `pad` fit policy explicitly. That is a decision, not a
default: `cut` would be a byte-for-byte no-op on these strings today and
would silently discard the property that they are tuned to their field.

Drops c_hi, which was assigned and never read.

Rendering unchanged: 377/377 render cases byte-identical, golden untouched.
Code lines 114 -> 80 (-34).

refactor(config-settings): draw the sub-category page through the frame ad9926861b

Width tier, title border, badge, cursor cell and row line now come from
__config_settings_frame, and the hand-verified title dash count
(iw - L - S - 22) is derived from the segment's visible width instead.

Introduces the `cut` fit policy, which is this page's documented
divergence from the category list: its labels and descriptions are static
data from __config_settings_subcats rather than per-tier authored text,
and several exceed the narrower tiers' fields. `string pad` only ever
grows a string, so they are truncated before padding. That reason now
lives in two places -- the frame's NOTES and each call site -- and the
DESCRIPTION block stating it here is unchanged.

Drops label_w and desc_w, both left assigned and never read once the
frame derives the field width; the comment recording the 13-wide label
field and how the description field absorbs it stays.

Rendering unchanged: 377/377 render cases byte-identical, golden untouched.
Code lines 85 -> 49 (-36).

refactor(config-settings): draw the value pages through the frame a824005893

Width tier, title border, boolean badge, cursor cell and row line now come
from __config_settings_frame. The type badges (PATH/INT/LIST/STR) stay
here -- they are this page's own vocabulary, used nowhere else -- and so
does the STR default arm, which covers a user-settable value rather than
being dead code.

Introduces the `shorten` fit policy: these fields hold arbitrary user
values and want an ellipsis, unlike the toggle page's per-tier text. The
inline editor keeps `pad`, because its field is already length-constrained
and carries a reverse-video caret whose escapes string shorten miscounts.

The panel stays exactly 16 lines -- chrome(6) + nrows + blanks -- so
config-settings.fish's panel_h and its erase are untouched.

Rendering unchanged: 377/377 render cases byte-identical, golden untouched.
Code lines 123 -> 98 (-25).

test: run the config-settings render golden in the test runner a3853a93d8

A byte-identity gate CI never runs will rot, and a rotted gate is worse
than no gate. Adds the render harness as Phase 4, in the same shape as the
vault suite: its own process, its own sandbox, no loaded config needed.

Kept to one self-contained block so it can be dropped or re-applied by
hand if the runner is restructured. The functional suite's 317/317 count
is untouched; the render cases report separately.

rootiest added 6 commits 2026-09-08 16:08:28 +00:00

test(config-settings): lock draw-function line counts to their panel heights 7b02348f64

feat(config-settings): add __config_settings_diff_redraw ece3e9dbfb

refactor(config-settings): dispatch captures the frame instead of printing it ab14837e39

feat(config-settings): diff redraw in the main navigation loop fae15b8d55

feat(config-settings): diff redraw in the inline value editor a0491b2bd1

fix(config-settings): repair diff-redraw review findings 3c4f720749

- Print the missing exit-repaint after the inline editor's
  __cs_dispatch_draw call (was silently changed to capture-only earlier
  in the branch; this call site was missed, blanking the panel on
  commit/cancel of an inline edit).
- Hoist (count $prev_edit_frame) out of a quoted math string in the
  inline editor's per-keystroke redraw -- fish doesn't expand a command
  substitution there, so math silently errored.
- Add a >= 52 column floor to both diff-path guards: below the
  narrowest tier's own 52-column box width, lines wrap and the diff
  path's one-physical-row-per-logical-line walk corrupts the display.
- Reword a stale test comment that described panel_h as mirroring a
  hand-set constant in __cs_dispatch_draw; it derives panel_h from real
  output now.
- Declare prev_edit_frame with -l alongside edit_frame instead of a
  bare set, matching the file's convention.
- Move prev_frame's declaration to its point of use instead of an
  empty top-level placeholder, matching old_h in the same block.

rootiest changed title from ~~refactor(config-settings): de-duplicate the TUI's three draw functions~~ to refactor(config-settings): shared frame renderer + line-diff redraw

2026-09-08 16:09:45 +00:00

rootiest added 1 commit 2026-09-08 16:20:23 +00:00

Merge remote-tracking branch 'origin/main' into refactor/config-settings-renderer afbc3e97d7

# Conflicts:
#	functions/__config_settings_draw.fish
#	functions/__config_settings_draw_subcat.fish
#	functions/__config_settings_draw_value.fish
#	tests/functional.fish

rootiest added the Area/Config Kind/Enhancement labels 2026-09-08 16:26:49 +00:00

rootiest commented

2026-09-08 16:28:42 +00:00