emrg: fix TUI cursor-left CLEAR_TO_EOL erasure + status bar reorg (rants 2026-08-11T19:59:09/20:02:43) - #693
Conversation
…nts 2026-08-11T19:59:09/20:02:43)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Verified: CI test workflow PASS (run 31491143827); local pytest 703 + GUI 212 green. The write_frame change removes the CLEAR_TO_EOL row-cleanup entirely and replaces it with precise SPACER_TAIL space-overwrite (prev char non-empty → write ' '), which covers the stale glyph without touching unchanged cells. last_x sync after the spacer write keeps subsequent cursor positioning correct. Status bar reorg moves model/elapsed/msg-count/dir into the left section with an independent current_model tracker; center is server+host only; right section removed. Tests cover the rant's exact repro (no \x1b[0K), the spacer ghost case, and the new StatusLine layout.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2
Re-verified: PR head 4b4d086 (unchanged since first review), CI test workflow still PASS (run 31491143827). Local pytest 703 + GUI 212 green. Diff reviewed — precise SPACER_TAIL space-overwrite replaces the CLEAR_TO_EOL row-cleanup; status bar left/center layout with independent current_model tracker; regression tests cover the rant repro and the new StatusLine layout.
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix: SPACER_TAIL space-write has a one-column off-by-one that shifts chars after a CJK insertion
Issue: The new SPACER_TAIL branch writes a space at the terminal cursor's current position — but after writing the WIDE cell at x, the cursor is at x+2 (a wide char advances 2 columns), not at the spacer position x+1. The space therefore lands one column PAST the spacer, and last_x = x claims the cursor is at x+1 — one short of reality. The next diff at x+2 then skips CUP (x == last_x + 1) and writes at the wrong column.
Repro (via diff_buffers + write_frame, all correct on master):
- Insert CJK before text: 'ab' → '世b' ⇒ PR output
\x1b[1;1H世 b— 'b' shifted right + phantom space (master:\x1b[1;1H世\x1b[1;3Hb, correct) - Replace char before CJK: '世cX' → '世世X' ⇒ PR output
\x1b[1;3H世 X— 'X' shifted right (master correct) - Both are ordinary composer edits for a CJK user — this is a regression.
Root cause: the space-write is unnecessary. The WIDE glyph's 2nd terminal column inherently covers the stale char at x+1; wide-char removal is already handled by the inline-shrink path (curr empty → write " "). Verified: deleting the whole elif block makes all 4 scenarios (insert / replace / remove / 'hello world'→'hello 世') produce correct output, and the cursor-left fix (\x1b[0K removal) still holds.
Proposed fix: remove the elif (prev_char := getattr(prev, "char", "")) ... block (incl. last_x = x). Also update test_write_frame_spacer_tail_overwrites_stale_char — it asserts " " in out for the spacer-over-stale case, which encodes the buggy behavior (with the fix, output is \x1b[1;7H世 — the wide glyph covers the stale 'o').
Everything else (CLEAR_TO_EOL removal, status bar reorg, current_model tracking, [m:ss] timer) looks correct.
…h covers stale char (review fix)
|
Thanks for the catch — confirmed off-by-one. After writing the WIDE cell the terminal cursor is at x+2, so the explicit space landed one column past the spacer and shifted subsequent chars on CJK insertions. Fixed in 7296269: removed the SPACER_TAIL Verified all four scenarios now match correct output (pytest 703 + GUI 212 green):
Cursor-left fix (no |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (re-review of fix commit 7296269)
The fix exactly addresses the off-by-one: the SPACER_TAIL space-write block is removed, and the test now asserts out == "\x1b[1;7H世" (wide glyph alone covers the stale char).
Re-verified on the new head:
- Insert CJK before text ('ab'→'世b'):
\x1b[1;1H世\x1b[1;3Hb— no shift, no ghost space - Replace char before CJK ('世cX'→'世世X'):
\x1b[1;3H世\x1b[1;5HX— no shift - Remove wide char ('世b'→'b'):
\x1b[1;1Hb— inline-shrink covers the ghost - Cursor-left (original rant bug): no
\x1b[0Kemitted - Local pytest: 703 passed. CI test workflow in progress (run 31491817661).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (post-fix)
Re-verified after 7296269: SPACER_TAIL explicit space removed (off-by-one confirmed — WIDE cell leaves cursor at x+2, so the space landed past the spacer). All four diff scenarios produce correct output; pytest 703 + GUI 212 green; CI PASS (run 31491817661).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (post-fix 3rd)
Head 7296269 unchanged, CI PASS (run 31491817661), pytest 703 + GUI 212 green. The off-by-one fix (removing the SPACER_TAIL space-write) is verified correct on all four diff scenarios; cursor-left fix holds. Merge condition met: 3 consecutive post-fix LGTMs.
Two TUI fixes from host rants (2026-08-11), combined in one PR per #692 lesson (shared-file doc-guard conflicts).
1. Cursor left-move erases chars to the right (rant 19:59:09, host-confirmed root cause)
write_frame()'s trailing "row cleanup" usedrow_dirty_end+CLEAR_TO_EOL(\x1b[0K), clearing from the last changed cell to end-of-line. When the composer cursor moves left, only the cursor-adjacent cells diff — so the unchanged characters right of the cursor were erased from screen (they reappeared on re-type/right-move).Fix per rant spec:
row_dirty_end/CLEAR_TO_EOLblock entirely (declaration + dirty-end computation + trailing cleanup).curris SPACER_TAIL andprevheld a real char, write a space to cover the stale glyph (a wide char's 2nd column is inherently empty — safe). Synclast_xso subsequent cursor positioning stays correct.diff_buffersunchanged: inline shrink (prev char → curr empty) already emits empty-cell updates, andwrite_framewrites a space — no CLEAR_TO_EOL fallback needed.Verified: rant's repro diff now yields
\x1b[1;7H\x1b[0m\x1b[7mo\x1b[0m(no\x1b[0K); spacer ghost case yields\x1b[1;7H世.2. Status bar reorg (rant 20:02:43, three host suggestions merged)
title (sid[:8]) [model] [1:23] · 3 msgs · ~/proj— session+ID, model (tracked independently viacurrent_model, not baked intoserver_id), elapsed (plain[m:ss], no ⏱ emoji), msg count + dir.{hid} @ {host}only._update_right()→_update_left_extra()using newleft_extra)./modelswitch now refreshes the left section; reconnect keeps the model.Tests / docs
tests/test_output.py+3: cursor-left diff has no\x1b[0K; SPACER_TAIL overwrites stale char; empty-prev spacer emits nothing.tests/test_buffer.py+1: wide-char removal covers ghost with spaces, no CLEAR_TO_EOL (integration throughdiff_buffers+write_frame).tests/test_status_line.py(new) +4: left composition, no right section, elapsed→left, idle state.Verification: pytest 703 passed, import check OK,
emrg --helpOK, GUInpm test212/212.