Repository navigation
os/v3/puppy: a console-TUI message costs the legend a word, not an action - #1026
Conversation
…tion `draw()` built the footer as `[keys line] + [message]` and charged both as fixed rows. A window whose frame was exactly the nine actions plus the two rows the legend wraps to (40x12: a tmux split, an ssh window on a phone) was therefore over-subscribed the moment a key said anything, and ratatui clipped the END of the MENU for the message's twelve seconds (`main.rs`, `Duration::from_secs(12)`): 40x12 with `this console never exits: 7 opens a shell, 9 reboots` on screen: actions 1-8, `9 Reboot / power off` gone, footer still promising "1-9 actions". Measured on the shipped binary before this commit, in a pty whose pane is exactly 40x12; `9 Reboot / power off` is a string `vm-check.sh`'s `U2 menu` row and `menugate` grep for. The actions are ROWS and the footer is WORDS, so the words now give way, in a fixed order: the legend's tail words first, then the legend line itself, and only if even the message alone cannot share the frame with the nine actions does the message go. The nine actions are never charged. 40x12 after a keypress: all nine actions + the whole message (legend gone - the frame has no spare row). 40x13: all nine + the legend's first row + the whole message. 40x11: ten menu rows leave one footer row, the message wraps to two, so the message goes and the legend stays. 80x10: the one row left over goes to the message. 80x25 and 240x67: nothing is trimmed - the whole legend and the message, exactly as before (the two committed snapshot frames are byte-identical, `renders_at_the_two_console_sizes`). `a_message_gives_way_before_an_action_does` asserts the nine sizes above (frame is the terminal, all nine actions, the message whole and never cut mid-word), that the legend is untouched where it fits (80x25, 240x67, 100x13), and that 40x11 drops the message rather than an action. Putting the old footer construction back turns it red at 40x12. Measured on the static binary: `5c39b9182135b90d` (1,562,048 B), no INTERP, `--snapshot 80x25 --fixture` matches the committed frame; `cargo test` 47/47, clippy `-D warnings` clean, `cargo fmt --check` clean, `tools/copyright.py --check` clean, no new files. Closes #1015 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
#1026 ready for review at For the reviewer, the one measurement to check: 40x12 in a real pty whose pane is exactly 40x12 - a keypress used to draw actions 1-8 with the footer still promising No gate file touched, no image build, no board run, no device ticket. |
PR Reviewer Guide 🔍(Review updated until commit b91fd26)Here are some key observations to aid the review process:
|
|
State:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at f245bb9e
$ cd os/v3/puppy/console-tui && cargo test --offline
test result: ok. 47 passed; 0 failed; 0 ignored
47 is 46 + the one this PR adds, and that test is the review: a_message_gives_way_before_an_action_does asserts, at 40x12 / 40x13 / 56x12 / 80x10 / 80x11 / 100x13 / 30x16 / 80x25 / 240x67, that the frame is exactly the terminal, that all nine actions are still on screen, and that the message is drawn whole or not at all; then that the legend is untouched where it fits (80x25, 240x67 — the sizes the gates read); then the case where not even the message fits (40x11): the message is what goes, and 1-9 actions stays.
The ordering in draw() matches that intent: keys_segments() gives words, menu_rows() gives rows, budget = h - actions.len(), and the loop drops the legend's tail, then the legend, then the message — never an action. Charging the QR and the rule against the same footer.len() keeps the frame-is-the-terminal invariant that the neighbouring tests already pinned.
Nit: keys_segments() returns 'static segments while the message is cloned per draw; fine, but the else arm of the trimming loop resets segments = full.clone() after taking the message, which reads as "give up the legend to keep the message" — a one-line comment there would save the next reader the trace I just did.
|
Updated to main ( |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 39da6108
Rebase-only, verified twice over:
own contribution (+/- lines vs merge base), f245bb9e1 vs 39da6108:
+149/-20, sha256 of the diff lines = a3aa9b718158 (both heads)
$ cd os/v3/puppy/console-tui && cargo test --offline (at 39da6108, on main 8c1eaea3)
test result: ok. 47 passed; 0 failed; 0 ignored
39da6108 is a merge of main rather than a rebase (Merge branch 'main' into os/v3-tui-msg), which is why git diff against the merge base is byte-identical to the head I reviewed. I re-ran the suite at the new head instead of assuming the merge was harmless, because ui.rs is the one file both this and #1028 rewrite; 47/47 there is also 46 + a_message_gives_way_before_an_action_does, the test this PR adds.
Review stands as posted. Nothing new to flag.
…e order is conflict-free No behaviour change, no line of logic moved: the modal test now sits above `the_menu_rule_gives_way_before_an_action_does` (which #1026 appends after) and the README paragraph now sits above the MENU-rule paragraph, so the two PRs no longer share merge context. Measured: before this, merging #1026 into this branch conflicted on both os/v3/puppy/console-tui/{README.md,src/ui.rs}; after it, both orders merge cleanly and `cargo test` is 48/48 on the combined tree.
|
Persistent review updated to latest commit b91fd26 |
…1028) * os/v3/puppy: a console-TUI dialog loses its question before its hint `draw()` sizes the modal box to its body and clamps it to the frame (`bh = (body.len() + 2).min(area.height)`), then draws the body with `Paragraph`, so a body that does not fit is clipped at its END - and the last line of every dialog is the hint, the only row that says how to answer. Measured on the shipped static binary before this commit (main `434db3d2`, `0edbe573…`), real ptys, `TERM=linux`, key **1** on a blank-key box: 80x25, 60x12 `y mints one and restarts Lemonade, n cancels` intact 40x12 `y mints one and restarts` half 40x11, 40x10, 30x12, 24x12 absent `Modal::Confirm { yes: Yes::Reboot }` carries `y reboots the box, n cancels` and `Modal::Power` carries `r reboots, p powers off, any other key cancels`, so in a 40x11 ssh window a screen that is about to reboot the box explains neither key that reboots it - while `y` still does what the question says. No gate impact (the gates run at 80x25 and 240x67, where the whole box fits). The fix is the same rule the rest of this file follows - the block's most load-bearing row is the last one to go: the dialog is now `title | question | hint` as three sections, and a frame too short for all of it loses the question rows first (whole rows at a time), then the title, and never the hint. 40x12 after key 1: hint whole (`y mints one and restarts` / `Lemonade, n cancels`), one question row given up 40x11 : hint whole, two question rows given up 24x12 : hint whole and wrapped (`y mints one and` / `restarts Lemonade, n` / `cancels`) 80x25, 240x67 : unchanged - the whole box, byte-identical frames `the_modal_hint_is_never_the_row_that_goes` renders four dialogs (reboot confirm, blank-key confirm, power, the API key info box) at 24x12, 30x12, 40x10, 40x11, 40x12, 60x12, 80x25 and 240x67, and requires the frame to be the terminal and the hint to be on screen, whole, at every one of them. Putting the old body construction back turns it red at 24x12; the fix turns it green. Measured on the static binary `42e11547b870428c` (1,556,736 B, no `PT_INTERP`): `cargo test` 47/47, clippy `--all-targets -D warnings` clean, `cargo fmt --check` clean, `tools/copyright.py --check` rc 0, `--snapshot 80x25 --fixture` matches the committed frame. 2 files +~85/-17, no new files, no gate file touched, no image build, no board run, no device ticket. Closes #1027 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * placement only: keep this PR's hunks clear of #1026's, so either merge order is conflict-free No behaviour change, no line of logic moved: the modal test now sits above `the_menu_rule_gives_way_before_an_action_does` (which #1026 appends after) and the README paragraph now sits above the MENU-rule paragraph, so the two PRs no longer share merge context. Measured: before this, merging #1026 into this branch conflicted on both os/v3/puppy/console-tui/{README.md,src/ui.rs}; after it, both orders merge cleanly and `cargo test` is 48/48 on the combined tree. * README: the suite is 48 tests once #1026 and this PR are both in #1026 merged as 9315118 and this branch is rebased onto it, so the count line is 46 + one test each = 48. It is the one bookkeeping nit the #1028 review comment raised. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Closes #1015.
Root cause
draw()built the footer as[keys line] + [message]and charged both as fixed rows. A frame that was exactly thenine actions plus the two rows the legend wraps to (40x12 - a tmux split, an ssh window on a phone) was therefore
over-subscribed the moment a key said anything, and ratatui clipped the end of the MENU for the message's twelve
seconds (
main.rs,Duration::from_secs(12)):9 Reboot / power offis gone - and it is a stringvm-check.sh'sU2 menurow andmenugategrep for. No gateimpact (the gates run at 80x25 and 240x67), but the ninth action is off the screen in any short window for as long as
the message lives.
What changed
The actions are ROWS and the footer is WORDS, so the words give way, in a fixed order: the legend's tail words first
(
keys_segments()/footer_lines()), then the legend line itself, and only if even the message alone cannot share theframe with the nine actions does the message go. The nine actions are never charged.
9 Reboot / power off, legend + messageThe message is never cut mid-word: it is drawn whole, or not at all.
Evidence
434db3d2's build,0edbe573..., 1,554,664 B) in a real pty whose paneis exactly 40x12; the same size after this tree is the table's first row.
a_message_gives_way_before_an_action_doessweeps 40x12, 40x13, 56x12, 80x10, 80x11, 100x13, 30x16, 80x25, 240x67(frame is the terminal, all nine actions, the message whole and never cut), asserts the legend is untouched where it
fits (80x25, 240x67, 100x13), and asserts 40x11 drops the message rather than an action. Putting the old footer
construction back turns it red at 40x12; the fix turns it green.
TERM=linux, the static binary: 40x12 -> nine actions + message;qon the console (noSSH_CONNECTION) still stays with its message whole at 40x12;qwithSSH_CONNECTIONstill exits 0.cargo test47/47, clippy--all-targets -D warningsclean,cargo fmt --checkclean,tools/copyright.py --checkrc 0. Static rebuild5c39b9182135b90d, 1,562,048 B, noPT_INTERP, no sharedlibraries;
--snapshot 80x25 --fixturematches the committed frame.vm-check.sh,puppy-check.sh,menugate.shuntouched), noimage build, no board run, no device ticket.
Not in this PR
9 Reboot / power offoff the screen for 12 s in a 13-row window #1015 was about the row, not the timer.