feat(tui): render through ratatui and own the input loop - #224
Conversation
Replace the hand-built ANSI string renderer with `impl Widget for &UiModel`, keeping `UiModel` as the state type, and move the client into raw mode so it owns the screen and the keyboard. These were meant to be two changes. They do not separate: ratatui assumes it owns the screen and the cursor, which is only true in raw mode. Under line-oriented input the terminal echoes into cells the buffer diff cannot see, ratatui hides the cursor unless told otherwise so that echo lands wherever the last diff write ended, and ratatui only clears on a horizontal shrink, so a grown viewport keeps whatever the smaller frame left behind. `Layout` splits heading, body, status, and prompt, and the sidebar, divider, and message pane. The single-column fallback still triggers below 30 columns or 8 rows, and the sidebar is still one third of the width clamped to 18-28. Terminal events are read on a blocking thread and handed to the runtime over a bounded channel, which keeps the new dependency surface at crossterm rather than an async event-stream stack. The client composes input itself because raw mode retires the terminal's line editor, and Ctrl-C or Ctrl-D detaches because raw mode suppresses the terminal's interrupt. Redirected output keeps the line-reading path and a fixed 80x24 viewport. Snapshot coverage moves from escape-sequence substrings to `Buffer` cells: the ANSI-16 rule is asserted per cell as neither `Color::Rgb` nor `Color::Indexed(n > 15)`, which the previous `38;2` check could not see. Verified under a pty that a resize alone reflows the frame, moving the divider from column 27 to 29 and extending addressed rows to 40. ratatui reaches Zlib-licensed `foldhash` through `hashbrown` as a hard dependency of `ratatui-core`, so `deny.toml` gains an exception scoped to that crate rather than a wider allow list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0144gZP6sbqUxk4m1qpjAAmU
9bdb3e6 to
55c9667
Compare
|
Thanks @bulters — this is an excellent contribution and very much the direction we intended for OC-023. Combining the ratatui rendering port with the raw input loop makes sense given the terminal-ownership issues you found. The bounded event handoff, ANSI-16 testing, resize behaviour and redirected-output path are all well considered. I’m also happy with the narrowly scoped Zlib exception for foldhash; that is preferable to widening the workspace allow-list. The size increase remains comfortably within our 10 MiB ceiling. I found two things I’d like addressed before merging:
Please also update docs/implementation-status.md, which still says the TUI is line-oriented, and correct the now-stale prompt_cursor() comment saying the client is still line-oriented. The remaining work you identified—live event consumption, reachable scroll mode, reconnect and signal/panic-safe restoration—can stay in follow-up PRs. Your analysis in #223 also looks like the right boundary: IPC demultiplexing belongs in the client layer rather than pushing protocol state into the TUI. The intermittent daemon test appears unrelated based on your branch-versus-main reproduction. Please open a separate issue with those results so we can track it without blocking this PR. Once those focused changes are pushed and the fork workflows are green, I’m happy for this to merge. Really appreciate the care taken here. |
Work toward OC-023 (#24): the rendering port and the input loop.
I originally proposed splitting these into two PRs. That split does not survive contact — ratatui assumes it owns the screen and the cursor, and that is only true in raw mode. A ratatui frame under line-oriented input corrupts itself: the terminal echoes typed characters into cells the buffer diff cannot see, ratatui hides the cursor unless told otherwise so the echo lands wherever the last diff write ended, and
resize.rsonly clears on a horizontal shrink so a grown viewport keeps whatever the smaller frame left behind. I had all three on screen before folding the loop in. Scroll/compose modes and reconnect/restoration still follow separately.What changes
Rendering.
UiModelrenders throughimpl Widget for &UiModelinstead of building an ANSI string by hand.Layout::verticalsplits heading / body / status / prompt andLayout::horizontalsplits sidebar / divider / message pane. The layout rules carry over unchanged: single-column fallback below 30 columns or 8 rows, sidebar at one third of width clamped to 18–28.Input. The client enters raw mode and reads terminal events on a blocking thread that hands them to the runtime over a bounded
tokio::sync::mpscchannel. That keeps the only new dependency on crossterm itself rather than pulling inevent-streamand a futures stack, which matters against the=pins, the cargo-deny policy and the size ceiling. Composition, backspace, escape-to-clear and enter-to-submit are handled by the client, because raw mode retires the terminal's own line editor.Ctrl-CandCtrl-Ddetach, since raw mode also suppresses the terminal's interrupt.Geometry.
Terminal::drawsizes from the backend, so the hardcodedrender(80, 24)is gone,Event::Resizerepaints immediately, and the narrow layout is reachable in the binary rather than only in tests. A resize forces a full repaint throughclear_regionplusswap_buffersrather thanTerminal::clear, which first queries the cursor position — that fails outright on a pipe and would race the stdin reader for the reply in a terminal.Redirected output. With stdout not a terminal there is no keyboard and no resize, so the client keeps reading whole lines and repainting a fixed 80×24 viewport.
omachat | catstill produces the frame it always did.Verification
Under a pty, starting at 80×24 and resized to 120×40 with no key pressed:
tests/ui.rskeeps the 80×24 and narrow cases and asserts againstBuffercells rather than escape-sequence substrings. The ANSI-16 rule becomes "no cell carriesColor::RgborColor::Indexed(n > 15)", checked per cell — stricter than the previous!contains("38;2"), which could not see an extended-palette index at all. Added cases cover the sidebar appearing wide and absent narrow, the status bar and prompt, caret placement on the prompt row, and aTestBackenddraw through a realTerminal.Six tests pass, plus the full
docs/development.mdsuite: fmt, clippy with-D warnings, workspace tests, rustdoc, both builds, version contract, release size, packaging.One caveat on that, since I would rather report it than round it up:
cargo test --workspacefailed once here onomachatd'sreconnect_drains_a_queued_private_message_once, withProtocol(WrongHttpMethod)atcrates/omachatd/tests/core.rs:579followed by the drain timeout at :639. It looks pre-existing and unrelated to this branch. Measured over 15 runs per tree on the same machine, it fails 1/15 on this branch and 1/15 onupstream/main. The lock diff here is purely additive (no package version changes at all) and nothing in this diff touchesomachatd, so both test binaries build from identical inputs. The stub server accepts exactly two connections in sequence and assumes the client makes exactly those two with nothing in between; a reconnect race would explain a non-upgrade request reachingaccept_async. Happy to open a separate issue with the reproduction if useful.Dependency and policy note
ratatui = "=0.30.2"withdefault-features = false, features = ["std", "crossterm"]. Defaults pull every backend — termwiz, termion, termina and the wezterm stack — so they are off. crossterm is used through ratatui's re-export rather than as a second direct dependency, so the backend and the event API cannot drift apart.This PR needs a
deny.tomldecision from you. ratatui reachesfoldhashthroughhashbrown, via bothkasuari(the layout solver) andlruinratatui-core. It is a hard dependency, not feature-gated, andfoldhashis Zlib-licensed, which is not on the allow list. ratatui 0.29 does not avoid it — it takes the same path throughlru.I added a scoped exception rather than widening the workspace list:
Zlib is permissive with no source obligation, and scoping it to one crate keeps the policy narrow.
cargo-deny 0.20.2 check licenses bans sourcesreportsbans ok, licenses ok, sources okwith this in place. If you would rather addZlibto the main allow list, or would rather not take ratatui on these terms at all, say so and I will rework it.Size
omachatgrows 536,720 -> 726,448 bytes withopt-level = "z"and fat LTO. The aggregate moves 5,924,280 -> 6,113,272 bytes, leaving 4.17 MiB under the 10 MiB ceiling. The other two binaries are unchanged.Known limits, deliberately left for the follow-ups
InputMode::Scrollis still never set; reachable scroll/compose modes are the next PR.Droponly. Withpanic = "abort"in the release profile that is not sufficient, and the panic hook plus SIGINT/SIGTERM handling is the PR after that.