Skip to content

refactor(eve): stop hand-maintaining what the dev TUI already knows - #1470

Merged
chadhietala merged 1 commit into
mainfrom
simplify-commit-review-tui-derived
Jul 31, 2026
Merged

refactor(eve): stop hand-maintaining what the dev TUI already knows#1470
chadhietala merged 1 commit into
mainfrom
simplify-commit-review-tui-derived

Conversation

@chadhietala

@chadhietala chadhietala commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Found

  • MAX_VISIBLE_SUGGESTIONS = 11 mirrors a 10-entry command registry by hand. command-typeahead.test.ts:184 asserts Math.min(PROMPT_COMMANDS.length, 10), which passes only because min(10,11) and min(10,10) agree — adding an 11th command and bumping the constant as its comment instructs would fail that test.
  • wrapVisibleLine spells its cursor step twice, once behind a single-use advanceTo closure; the empty-line early return is unreachable; three exports have no outside caller.

Did
Derived the constant from PROMPT_COMMANDS, one consume() closure for both walks, dropped the guard and the three export keywords. −24/+24; no user-visible change at 10 commands.

Validated
1011 CLI unit tests pass, including a new test pinning wrapVisibleLine("") and the unmodified 64 KiB linear-time regression test. Two expectation edits now reference the registry instead of literals. tsc --noEmit, lint, fmt, guard:invariants clean.

Two places kept a copy of something the code could derive.

`MAX_VISIBLE_SUGGESTIONS` was a hand-maintained mirror of the command
registry's length, with a comment instructing the next author to bump it —
and it had already drifted to 11 against 10 entries. Worse,
`command-typeahead.test.ts` asserted the bare-slash row count as
`Math.min(PROMPT_COMMANDS.length, 10)`, which matches today only because
`min(10, 11)` and `min(10, 10)` agree: adding an 11th command and bumping
the constant as instructed would have failed that test. The constant now
reads the registry, so the "never scroll a real command out of view"
invariant holds mechanically, and both test expectations reference the
registry instead of a literal.

In `wrapVisibleLine`, the #1442 linear-time rewrite spelled its
"consume one unit" step twice — once inside a single-use `advanceTo`
closure, once in the whitespace-collapse loop. One `consume()` closure
serves both walks, keeping the single forward pass the rewrite exists for.
The empty-line early return is also gone: with no units, the general path
already emits exactly one empty row, which a new test now pins.

`ansiPattern`, `ansiPrefixPattern`, and `codePointWidth` lose their
`export` — nothing outside the module referenced them.

Signed-off-by: Chad Hietala <chadhietala@gmail.com>
@chadhietala chadhietala added the simplify-devbox Simplification found by the automated devbox code review label Jul 31, 2026
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
eve-docs Ready Ready Preview Jul 31, 2026 3:33pm
eve-docs-4759 Ready Ready Preview, v0 Jul 31, 2026 3:33pm

@chadhietala
chadhietala merged commit f5d0533 into main Jul 31, 2026
89 of 90 checks passed
@chadhietala
chadhietala deleted the simplify-commit-review-tui-derived branch July 31, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

simplify-devbox Simplification found by the automated devbox code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants