Repository navigation
feat(painter): prototype a frame painter behind REEDLINE_FRAME_PAINTER - #1299
Draft
kronberger-droid wants to merge 7 commits into
Draft
kronberger-droid wants to merge 7 commits into
kronberger-droid wants to merge 7 commits into
Conversation
The streaming painter re-derives row counts from strings in three places, and they disagree: a prompt ending in `\n` reads as one row, and one line wrapping past the bottom reads as fitting, so the terminal scrolls on every paint. Both repros sat `#[ignore]`d in `screen_tests.rs` since nushell#1249. They pass under the frame, since it has one row model: the grapheme walk that fills `Frame` rows. Opt-in rather than a swap, so the screen tests run against both painters and the streaming path stays byte-for-byte as it was. Dumb terminals keep the streaming path, since the committer positions every row absolutely. Not covered yet: the IDE menu still takes its column from `PromptLines::cursor_pos`, so menu placement after a `\n` prompt is unchanged; the frame is forgotten on every anchor change, so a resumed prompt redraws in full.
A row holding the prompt and the first line of the buffer was redrawn whole on every keystroke, prompt included, and an insert on an earlier row renumbered every later cell and redrew those rows too. Cells now compare on what they draw, and a changed row is rewritten from its first differing cell to the end: appending costs one glyph, inserting mid-row costs the tail, which without `ICH` it has to. The style at the restart column is replayed from the skipped cells' SGR prefixes rather than parsed into a model, so the raw-escape design holds: only `is_sgr` and `is_sgr_reset` need to recognize anything. A row whose `leading` state differs is redrawn whole; a common suffix is not sought, since an insert shifts the tail's columns anyway.
The hint case is nushell#974's shape and fails on the streaming painter, where `skip_buffer_lines` clips by `\n` and a soft-wrapped hint goes out whole. The resize case passes on both painters, so it pins what nushell#857 restored rather than a frame fix.
The room was measured below the anchor as it stood before the paint, but the paint scrolls to make room, as far as row 0 if it must, so the anchor has no say in it: the room is the screen minus the cursor's row. A menu opened near the bottom was sized for one row and drew two.
Contributor
|
This sounds very interesting. Seems like it might fix one of my biggest complaints about reedline. |
Brings in the status line (nushell#1268), the framed external printer (nushell#1257), control-character escaping (nushell#1241) and the cursor-answering `W::screen` test writer. The frame lays the status line out as rows below the hint and above a menu, giving way first when the block would not fit, as the streaming painter's `status_room` rule has it. `prepare_anchor` goes in favor of upstream's `reconcile_stale_anchor`, and the frame uses `begin_frame`/`end_frame` so `TERM=dumb` is skipped there too. The screen tests take upstream's file with the `Engine` parameter reapplied, so the status line and external printer cases run on both painters.
Collaborator
Author
|
Yeah that was the plan. I have been researching the possible painter types and this one seems to be the best choice to make the painter more reliable and makes reedline not dependent on ratatui or similar. And its actually less code than the current printer. |
Cells carried raw escape prefixes, so two strings that look the same on screen compared different, and a tail redraw had to replay the prefixes it skipped, which lost OSC 8 since only SGR was replayed. A cell now records the style in effect, as shared strings rather than table indices since the diff compares cells of two frames. OSC 133 marks are positions on the frame and go out once per prompt. Terminals that tag cells by the zone in effect at write time still see them in draw order on a full paint, and the input mark is repeated after the right prompt so typed text does not land in the right prompt's zone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A second paint path, opt-in and off by default, that lays the whole prompt out before writing anything.
Draft, so people can run it rather than read about it.
Why. The streaming painter counts rows three different ways (
str::lines()incompute_layoutandskip_buffer_lines, the grapheme walk inutils.rs), and they disagree:\nreads as one row, so the right prompt lands a row too highThe first two have sat
#[ignore]d inscreen_tests.rssince #1249.What. The frame painter uses the desired-screen diff model that curses and fish also use:
frame::layoutwalks prompt, buffer, hint, status line, menu and right prompt into rows of one-grapheme cells, with theadvance_graphemerule that already exists, so there is one row model.Painter::repaint_framediffs against the last frame and rewrites only the cells that changed, each row positioned absolutely from the anchor, cursor placed byMoveTo.PromptStartRow, the measure-and-grow logic and the synchronized-update bracket are the same code; the frame only answers what goes on which row.Switch.
REEDLINE_FRAME_PAINTER=1in the process environment selects the frame.Dumb terminals keep the streaming path.
No public API change, and with the variable unset the emitted bytes are what they were.
Before and after
abc\nwith a right promptabcrowAdditional notes
Tests.
Engineparameter, the status line and external printer cases included.-D warnings, fmt and the no-tty nextest run are green ondefaultandexternal_printer.Checked live in tmux against the streaming painter: wrapped prompt, menus, status line, right prompt and exit path render identically.
Not here.
PromptLines::cursor_posreedline::prompt::base::Promptshould be redesigned #526 is a layout rule in this model, so feat(painter): paint a prompt footer below the input #1242 would fold in as a partRelated: #1282, #974, #526.
Built with Claude Code, partly to test its capabilities and boundaries on a change of this size.
Design calls, review and the live checks are mine.