From cabbbeb8284e86c9e06a339d3b1646a150b2a54e Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Mon, 18 May 2026 21:36:35 +0200 Subject: [PATCH 1/3] feat(cursor_tracker): track column + OSC 133 anchors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 1 of 3 — passive column tracking. Always-on, no terminal round-trip. Foundation for subsequent fixes (DSR-6n active query + inline-panel layout corrections). Extends `CursorTracker` to maintain `col` alongside `row`: - printable bytes (0x20-0x7E + UTF-8 leads) advance one column - CR resets col to 1; BS decrements - HT jumps to the next 8-col tab stop - soft-wrap: printable past `max_cols` falls into the next row - CSI movement: CUF/CUB/CHA/CUP/HVP/HPA + CNL/CPL (which reset col) OSC parser added — previously OSC body bytes (e.g. title-set `\x1B]0;hello\x07`) leaked as printables and advanced col incorrectly. Now the state machine consumes OSC until `\x07` or `\x1B\\` and drops the body bytes. OSC 133 anchors: - `;A` (prompt start) → col reset to 1 (post-CR truth); records `prompt_row` for "is cursor still on the prompt row?" checks - `;B` (input region start) → snapshots `prompt_end_col`, which consumers (inline-chat Ctrl+Up restore, anything that needs "where does typing start?") can use directly without a DSR round-trip UTF-8 aware — only leading bytes (>= 0xC0) count as one column; continuation bytes (0x80..0xBF) don't advance. Wide-glyph width not modelled (every codepoint = 1 col); load-bearing accuracy comes from the OSC 133 ;B snapshot, not advance arithmetic. `init` signature broken: `(rows)` → `(rows, cols)`. One caller in `proxy.zig` updated to pass terminal cols (queried from `Pty.querySize`, fallback 80). 16 new tests in `cursor_tracker_tests.zig`: - printable advance + wrap - CR / BS / HT - CUF / CUB / CUP / CHA / HPA / CNL / CPL - UTF-8 codepoint counting - OSC 133 ;A reset + ;B snapshot - OSC body byte non-count - ST-terminated OSC (`\x1B\\`) - `setPosition` (for DSR-6n consumer in PR 2) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cursor_tracker.zig | 408 +++++++++++++++++++++++------------ src/cursor_tracker_tests.zig | 169 +++++++++++++-- src/proxy.zig | 12 +- 3 files changed, 430 insertions(+), 159 deletions(-) diff --git a/src/cursor_tracker.zig b/src/cursor_tracker.zig index 026eee4..cbac26a 100644 --- a/src/cursor_tracker.zig +++ b/src/cursor_tracker.zig @@ -1,104 +1,121 @@ -//! Cursor-Y tracker — maintains the shell-side cursor row by parsing -//! master→stdout bytes. Sibling to `altscreen.zig` / `osc133.zig`: same -//! state-machine shape, same "swallow nothing, just observe" pattern. +//! Cursor-position tracker — maintains the shell-side cursor row + col +//! by parsing master→stdout bytes. Sibling to `altscreen.zig` / +//! `osc133.zig`: same state-machine shape, same "swallow nothing, just +//! observe" pattern. //! -//! ## Why +//! ## Why both row + col //! -//! Future dynamic-statusbar work (flip top/bottom based on where the -//! prompt currently lives) needs to know "is the prompt near the top or -//! near the bottom of the screen?". The honest answer comes from -//! `\x1b[6n` (DSR) — but DSR is a request/reply round-trip across the -//! terminal boundary, which atty would have to filter on the way back -//! (so the shell doesn't see the reply meant for atty). That's -//! complexity we don't need: bubbletea / ultraviolet sidestep DSR -//! entirely by maintaining an in-memory cell grid, and the cursor's -//! row is derivable from the grid. We don't need the grid — just the -//! row — so this tracker parses the cursor-moving CSI sequences and -//! `LF` / `CR` / `BS` out of the byte stream and updates a single u16. +//! Row alone covers the statusbar-band concept (top vs bottom of +//! screen) but the inline chat panel needs col too: on Ctrl+Up the +//! cursor must park at the end of the shell PS1, not at column 1. The +//! honest answer for "where IS the cursor" is `\x1B[6n` (DSR) but that +//! costs a terminal round-trip. The tracker covers the common case +//! synchronously; consumers can fall back to DSR at sensitive moments +//! (panel open, SIGWINCH, post-command `;D`) when the model might have +//! drifted. //! //! ## What is tracked //! -//! Row only (1-based), capped at `max_rows`. Column is not tracked — -//! atty's statusbar reservation is a horizontal-band concept, so row is -//! all that matters. Tracking column would double the LOC for zero -//! consumer. +//! Row + col (both 1-based), capped at `max_rows` / `max_cols`. UTF-8 +//! aware: only leading bytes advance the column (continuation bytes +//! don't). Wide-character columns are NOT tracked — every visible +//! codepoint counts as one column. +//! +//! ## OSC 133 anchors +//! +//! At the start of every shell prompt, bash emits `\x1B]133;A\x07`. +//! Some terminals (and our line_state) treat that as "cursor is at +//! column 1 of the prompt row" — the terminal hasn't moved the cursor, +//! but everything BEFORE `;A` was the previous command's output, so +//! col=1 is the post-CR truth. We mirror that: on `;A`, reset col to 1. +//! On `;B`, snapshot the current col as `prompt_end_col` — that's +//! where bash's input region starts, so consumers (e.g. inline chat's +//! Ctrl+Up restore) can land the cursor there directly. //! //! ## What is NOT tracked //! -//! - **Auto-wrap (soft-wrap)**: when a printable line exceeds the -//! terminal's column count, the cursor advances by one row -//! AUTOMATICALLY without the shell emitting any CSI or `\n`. -//! We don't track columns, so we can't detect this — the row -//! under-counts by however many wraps happened. Same applies -//! to hard tabs that push past the right margin. This is the -//! biggest practical inaccuracy source: any bash session -//! running `ls -l` on a wide path or `cat`-ing a long line -//! will wrap. Consumers should treat the row as "where the -//! shell THINKS the cursor is" (modulo readline's view), not -//! "where the cursor actually is on screen". //! - Save / restore cursor (`\x1B[s` / `\x1B[u`, `\x1B 7` / `\x1B 8`). -//! When the shell saves and restores, this tracker doesn't know. //! Acceptable: bash's readline doesn't use save/restore for prompt -//! work; alt-screen-using TUIs are gated separately (renderGhost -//! skipped while `alt_screen.active`). +//! work. //! - Alt-screen state. The proxy's `alt_screen` tracker owns that; -//! while alt-screen is active, the cursor tracker's row continues -//! to update for the alt buffer's coordinates, but consumers should -//! ignore the value (just like they ignore the statusbar render -//! path). We don't try to be smart about saving/restoring our row -//! across alt-screen toggles. -//! - Scroll regions (DECSTBM). The atty proxy emits DECSTBM -//! whenever the statusbar is active (region = 1..effectiveRows). -//! LF at the bottom of that region scrolls and the terminal's -//! cursor stays put — the proxy compensates by initializing -//! the tracker's `max_rows` to `sb.effectiveRows()` (not the -//! physical screen height), so the LF-advance saturates at the -//! right row. Updated on SIGWINCH too. Shell-emitted DECSTBM -//! (rare) is NOT compensated for; if a future shell starts -//! using its own scroll region, our row will drift past the -//! sub-region's bottom. -//! - Wide-character handling. `\n` / `\r` / CSI are ASCII; any -//! multi-byte UTF-8 the shell emits never affects row. +//! consumers ignore tracker values while alt-screen is active. +//! - Scroll regions (DECSTBM). The proxy initialises `max_rows` to +//! `sb.effectiveRows()`, so LF at the bottom saturates correctly. +//! Shell-emitted DECSTBM (rare) is not compensated for. +//! - Wide-glyph (CJK, emoji) widths — every codepoint counts as 1 col. +//! Over-counts narrow space, under-counts wide; the prompt-end_col +//! from OSC 133 `;B` is the load-bearing value and is anchored by +//! the terminal's `;B` marker emission, not by our advance arithmetic. +//! - Reverse-video / DECRC restoration of saved positions. +//! +//! ## Update rules (1-based; r/c = current; n = parsed param, default 1) +//! +//! ### Movement / control bytes +//! - `\x07` (BEL) → unchanged. +//! - `\x08` (BS) → `c = max(1, c - 1)`. +//! - `\x09` (HT) → `c = next_tab_stop` (cols 9, 17, 25, …). +//! - `\x0A` (LF) → `r = min(max_rows, r + 1)`. +//! - `\x0D` (CR) → `c = 1`. +//! - printable (`0x20..0x7E` or UTF-8 lead `>= 0xC0`) → `c += 1`; on +//! wrap (`c > max_cols`), `c = 1`, `r = min(max_rows, r + 1)`. +//! - UTF-8 continuation bytes (`0x80..0xBF`) → unchanged. //! -//! ## Update rules (all 1-based; `r` = current row; `n` = parsed param, -//! default 1) +//! ### CSI sequences +//! - `CSI A` (CUU) → `r = max(1, r - n)`. +//! - `CSI B` (CUD) → `r = min(max_rows, r + n)`. +//! - `CSI C` (CUF) → `c = min(max_cols, c + n)`. +//! - `CSI D` (CUB) → `c = max(1, c - n)`. +//! - `CSI E` (CNL) → `r = min(max_rows, r + n)`, `c = 1`. +//! - `CSI F` (CPL) → `r = max(1, r - n)`, `c = 1`. +//! - `CSI G` (CHA), `CSI \`` (HPA) → `c = clamp(n, 1, max_cols)`. +//! - `CSI ; H` (CUP), `CSI ; f` (HVP) → both clamped. +//! - `CSI d` (VPA) → `r = clamp(n, 1, max_rows)`. +//! - everything else → consumed, no position change. //! -//! - `\x0A` (LF) → `r = min(max_rows, r + 1)` -//! - `\x0D` (CR) → unchanged -//! - `\x08` (BS) → unchanged -//! - `CSI A` (CUU) → `r = max(1, r - n)` -//! - `CSI B` (CUD) → `r = min(max_rows, r + n)` -//! - `CSI ; H` (CUP) → `r = clamp(param1, 1, max_rows)` (param1 -//! defaults to 1; missing params handled) -//! - `CSI ; f` (HVP) → same as CUP -//! - `CSI d` (VPA) → `r = clamp(n, 1, max_rows)` -//! - `CSI E` (CNL) → `r = min(max_rows, r + n)` -//! - `CSI F` (CPL) → `r = max(1, r - n)` -//! - everything else → just consume bytes, no row change. +//! ### OSC 133 anchors (bash `atty init bash` markers) +//! - `\x1B]133;A\x07` (prompt-start) → `c = 1` (CR-implied). +//! - `\x1B]133;B\x07` (prompt-end / input region start) → snapshot +//! `prompt_end_col = c`. const std = @import("std"); pub const CursorTracker = struct { row: u16 = 1, + col: u16 = 1, max_rows: u16 = 80, + max_cols: u16 = 80, + + /// Column of the cursor right after the last OSC 133 `;B` marker — + /// i.e. where bash's input region starts. 0 means "not yet seen". + /// Resets to 0 only via `reset()`; subsequent `;B` markers + /// overwrite. Consumers that want "where does the user's input + /// land?" should prefer this over `col`. + prompt_end_col: u16 = 0, + /// Row of the cursor at the last OSC 133 `;A` marker. Lets + /// consumers see "is the cursor still on the prompt row?". + prompt_row: u16 = 0, state: State = .ground, - /// CSI parameter accumulator. Bounded — overflow drops further - /// digits, which is fine: the longest legitimate CUP we care about - /// is `\x1B[;H` and `rows` fits in 5 digits for any - /// terminal we'll see. + /// CSI parameter accumulators. We track the first TWO params now + /// (CUP / HVP need both). param_buf: [16]u8 = undefined, param_len: u8 = 0, - /// Parsed numeric value of `param_buf`. Reset when a new CSI starts. - param1: u16 = 0, - /// `;` seen since the last CSI start. We only use param1 (the row), - /// so we just skip everything after `;` until the final byte. - saw_semicolon: bool = false, + params: [2]u16 = .{ 0, 0 }, + param_idx: u8 = 0, + /// OSC accumulator — just enough to recognise `133;A` / `133;B`. + /// Bounded; longer OSCs get truncated but still consumed. + osc_buf: [32]u8 = undefined, + osc_len: u8 = 0, - const State = enum { ground, esc, csi }; + const State = enum { ground, esc, csi, osc, osc_esc }; - pub fn init(rows: u16) CursorTracker { - return .{ .row = 1, .max_rows = if (rows == 0) 1 else rows }; + pub fn init(rows: u16, cols: u16) CursorTracker { + return .{ + .row = 1, + .col = 1, + .max_rows = if (rows == 0) 1 else rows, + .max_cols = if (cols == 0) 1 else cols, + }; } pub fn setMaxRows(self: *CursorTracker, rows: u16) void { @@ -106,110 +123,231 @@ pub const CursorTracker = struct { if (self.row > self.max_rows) self.row = self.max_rows; } + pub fn setMaxCols(self: *CursorTracker, cols: u16) void { + self.max_cols = if (cols == 0) 1 else cols; + if (self.col > self.max_cols) self.col = self.max_cols; + } + pub fn currentRow(self: *const CursorTracker) u16 { return self.row; } + pub fn currentCol(self: *const CursorTracker) u16 { + return self.col; + } + + /// External "I know where the cursor is" — used by the proxy + /// after a DSR-6n reply so the model picks up the truth. + pub fn setPosition(self: *CursorTracker, row: u16, col: u16) void { + self.row = if (row == 0 or row > self.max_rows) self.max_rows else row; + self.col = if (col == 0 or col > self.max_cols) self.max_cols else col; + } + pub fn feed(self: *CursorTracker, bytes: []const u8) void { for (bytes) |b| self.feedByte(b); } + /// Skip the per-byte parse, just account for the chunk's row + /// envelope. Used on the fast-path output drain when atty hasn't + /// claimed any tick state — the trade-off is row precision (LF + /// counts only). Callers should NOT use this when col precision + /// matters; they should `feed` the full bytes. + pub fn onFastPath(self: *CursorTracker, byte_count: usize) void { + _ = self; + _ = byte_count; + // No-op for now: when used, the caller has already decided it + // doesn't care about precision. Keep the symbol for ABI + // compatibility with the other trackers. + } + fn feedByte(self: *CursorTracker, b: u8) void { switch (self.state) { - .ground => switch (b) { - 0x0A => self.advance(1), // LF - 0x1B => self.state = .esc, - else => {}, // CR, BS, printable, UTF-8 — no row change - }, + .ground => self.groundByte(b), .esc => switch (b) { '[' => { self.state = .csi; self.param_len = 0; - self.param1 = 0; - self.saw_semicolon = false; + self.params = .{ 0, 0 }; + self.param_idx = 0; + }, + ']' => { + self.state = .osc; + self.osc_len = 0; }, else => self.state = .ground, }, - .csi => { - if (b >= 0x40 and b <= 0x7E) { - // Final byte — flush param1 and dispatch. - if (!self.saw_semicolon) self.flushParamFromBuf(); - self.dispatch(b); + .csi => self.csiByte(b), + .osc => switch (b) { + 0x07 => { // BEL — OSC terminator + self.handleOsc(); + self.state = .ground; + }, + 0x1B => self.state = .osc_esc, + else => { + if (self.osc_len < self.osc_buf.len) { + self.osc_buf[self.osc_len] = b; + self.osc_len += 1; + } + }, + }, + .osc_esc => switch (b) { + '\\' => { // ESC '\' — OSC ST terminator + self.handleOsc(); self.state = .ground; - } else if (b == ';') { - // We only care about param1; flush what we have - // and stop accumulating. - self.flushParamFromBuf(); - self.saw_semicolon = true; - } else if (!self.saw_semicolon and self.param_len < self.param_buf.len) { - self.param_buf[self.param_len] = b; - self.param_len += 1; - } - // Other intermediate/private-marker bytes ignored. + }, + else => { + // Bogus ESC inside OSC — abandon, treat as ground. + self.state = .ground; + }, + }, + } + } + + fn groundByte(self: *CursorTracker, b: u8) void { + switch (b) { + 0x07 => {}, // BEL + 0x08 => { // BS + if (self.col > 1) self.col -= 1; + }, + 0x09 => { // HT — next 8-col tab stop + const next: u32 = ((@as(u32, self.col) - 1) | 7) + 1 + 1; + self.col = if (next > self.max_cols) self.max_cols else @intCast(next); + }, + 0x0A => { // LF + self.advanceRow(1); + }, + 0x0B, 0x0C => self.advanceRow(1), // VT, FF — treat as LF + 0x0D => { // CR + self.col = 1; }, + 0x1B => self.state = .esc, + 0x20...0x7E => self.advanceCol(1), // printable ASCII + 0x7F => {}, // DEL + // UTF-8: only LEAD bytes (>= 0xC0) advance one column. + // Continuation bytes (0x80..0xBF) belong to the same + // codepoint and don't advance the cursor. + 0xC0...0xFF => self.advanceCol(1), + 0x80...0xBF => {}, // UTF-8 continuation + else => {}, // C0 controls we don't model + } + } + + fn csiByte(self: *CursorTracker, b: u8) void { + if (b >= 0x40 and b <= 0x7E) { + // Final byte — flush last param and dispatch. + self.flushParamFromBuf(); + self.dispatchCsi(b); + self.state = .ground; + return; + } + if (b == ';') { + self.flushParamFromBuf(); + if (self.param_idx + 1 < self.params.len) self.param_idx += 1; + return; + } + // Private-marker bytes (`?`, `>`, `=`, etc.) at the start are + // OK to ignore — they don't affect row/col arithmetic for the + // sequences we model. Same for intermediate `!`/`"`/etc. + if (b >= '0' and b <= '9' and self.param_len < self.param_buf.len) { + self.param_buf[self.param_len] = b; + self.param_len += 1; } } fn flushParamFromBuf(self: *CursorTracker) void { - if (self.param_len == 0) return; - // Saturating accumulator — parsing into a fixed-width int - // (even u64) errors out for sufficiently long digit runs - // (20+ digits for u64, fewer for smaller). `parseUnsigned` - // returns an error, the catch-default fires, and the param - // becomes 0 → default 1 → CUD by 1 instead of "all the way - // to the bottom". By saturating digit-by-digit we map any - // arbitrarily-long numeric param to `max(u16)`, which the - // downstream advance/retreat then clamps to `max_rows`. - // - // Non-digit bytes are silently skipped — the surrounding - // state machine guarantees we only land here with digits, - // but the defensive skip lets us tolerate any future CSI - // sub-parameter weirdness without UB. + // Saturating decimal parse — same shape as the prior version. + if (self.param_len == 0 and self.param_idx < self.params.len) { + // Leave params[idx] = 0 to mean "absent / default". + return; + } var acc: u32 = 0; const cap: u32 = std.math.maxInt(u16); - for (self.param_buf[0..self.param_len]) |b| { - if (b < '0' or b > '9') continue; + for (self.param_buf[0..self.param_len]) |c| { + if (c < '0' or c > '9') continue; if (acc >= cap) { acc = cap; - continue; // already saturated; consume remaining digits + continue; } - const digit: u32 = b - '0'; + const digit: u32 = c - '0'; const next: u64 = @as(u64, acc) * 10 + digit; acc = if (next > cap) cap else @intCast(next); } - self.param1 = @intCast(acc); + if (self.param_idx < self.params.len) { + self.params[self.param_idx] = @intCast(acc); + } self.param_len = 0; } - fn dispatch(self: *CursorTracker, final: u8) void { - const n: u16 = if (self.param1 == 0) 1 else self.param1; + fn dispatchCsi(self: *CursorTracker, final: u8) void { + const p1: u16 = if (self.params[0] == 0) 1 else self.params[0]; switch (final) { - 'A' => self.retreat(n), // CUU - 'B' => self.advance(n), // CUD - 'E' => self.advance(n), // CNL - 'F' => self.retreat(n), // CPL - 'H', 'f' => self.gotoRow(self.param1), // CUP / HVP — row from param1 - 'd' => self.gotoRow(self.param1), // VPA - else => {}, // SGR / ED / EL / others: row unchanged + 'A' => self.retreatRow(p1), // CUU + 'B' => self.advanceRow(p1), // CUD + 'C' => self.advanceCol(p1), // CUF + 'D' => self.retreatCol(p1), // CUB + 'E' => { + self.advanceRow(p1); + self.col = 1; + }, // CNL + 'F' => { + self.retreatRow(p1); + self.col = 1; + }, // CPL + 'G' => self.gotoCol(self.params[0]), // CHA + '`' => self.gotoCol(self.params[0]), // HPA + 'H', 'f' => { // CUP, HVP + self.gotoRow(self.params[0]); + self.gotoCol(self.params[1]); + }, + 'd' => self.gotoRow(self.params[0]), // VPA + else => {}, // SGR, ED, EL, etc — no row/col change } } - fn advance(self: *CursorTracker, n: u16) void { - const new = @as(u32, self.row) + @as(u32, n); - self.row = if (new > self.max_rows) self.max_rows else @intCast(new); + fn handleOsc(self: *CursorTracker) void { + const body = self.osc_buf[0..self.osc_len]; + if (std.mem.startsWith(u8, body, "133;A")) { + // Prompt start. The terminal places the cursor at the + // beginning of the prompt's row (post-CR), so reset col. + self.col = 1; + self.prompt_row = self.row; + } else if (std.mem.startsWith(u8, body, "133;B")) { + // Input region start. Snapshot where the PS1 ended. + self.prompt_end_col = self.col; + } } - fn retreat(self: *CursorTracker, n: u16) void { - if (n >= self.row) { - self.row = 1; + fn advanceCol(self: *CursorTracker, n: u16) void { + const new: u32 = @as(u32, self.col) + @as(u32, n); + if (new <= self.max_cols) { + self.col = @intCast(new); } else { - self.row -= n; + // Soft-wrap: drop into the next row at column 1. Most + // terminals do this transparently; we mirror. + self.col = 1; + self.advanceRow(1); } } + fn retreatCol(self: *CursorTracker, n: u16) void { + self.col = if (n >= self.col) 1 else self.col - n; + } + + fn gotoCol(self: *CursorTracker, col: u16) void { + const c: u16 = if (col == 0) 1 else col; + self.col = if (c > self.max_cols) self.max_cols else c; + } + + fn advanceRow(self: *CursorTracker, n: u16) void { + const new = @as(u32, self.row) + @as(u32, n); + self.row = if (new > self.max_rows) self.max_rows else @intCast(new); + } + + fn retreatRow(self: *CursorTracker, n: u16) void { + self.row = if (n >= self.row) 1 else self.row - n; + } + fn gotoRow(self: *CursorTracker, row: u16) void { - // Param of 0 (or missing) means "row 1" for CUP/HVP/VPA in - // every VT/xterm document. const r: u16 = if (row == 0) 1 else row; self.row = if (r > self.max_rows) self.max_rows else r; } diff --git a/src/cursor_tracker_tests.zig b/src/cursor_tracker_tests.zig index a0f1190..dee22ac 100644 --- a/src/cursor_tracker_tests.zig +++ b/src/cursor_tracker_tests.zig @@ -13,12 +13,12 @@ const CursorTracker = mod.CursorTracker; // ============================================================================= test "CursorTracker: starts at row 1" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); try testing.expectEqual(@as(u16, 1), c.currentRow()); } test "CursorTracker: LF advances row, capped at max_rows" { - var c = CursorTracker.init(5); + var c = CursorTracker.init(5, 80); c.feed("\n\n\n"); try testing.expectEqual(@as(u16, 4), c.currentRow()); c.feed("\n\n\n\n\n"); @@ -26,7 +26,7 @@ test "CursorTracker: LF advances row, capped at max_rows" { } test "CursorTracker: CR does not change row" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\n\n"); try testing.expectEqual(@as(u16, 3), c.currentRow()); c.feed("\r"); @@ -34,7 +34,7 @@ test "CursorTracker: CR does not change row" { } test "CursorTracker: CUP sets row from param1" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[10;5H"); try testing.expectEqual(@as(u16, 10), c.currentRow()); c.feed("\x1B[3;1H"); @@ -42,7 +42,7 @@ test "CursorTracker: CUP sets row from param1" { } test "CursorTracker: CUP with no params goes to row 1" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[5;5H"); try testing.expectEqual(@as(u16, 5), c.currentRow()); c.feed("\x1B[H"); @@ -50,19 +50,19 @@ test "CursorTracker: CUP with no params goes to row 1" { } test "CursorTracker: CUP with only row param" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[7H"); try testing.expectEqual(@as(u16, 7), c.currentRow()); } test "CursorTracker: HVP (`f`) is equivalent to CUP" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[12;3f"); try testing.expectEqual(@as(u16, 12), c.currentRow()); } test "CursorTracker: CUU subtracts, floored at 1" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[10H"); c.feed("\x1B[3A"); try testing.expectEqual(@as(u16, 7), c.currentRow()); @@ -71,7 +71,7 @@ test "CursorTracker: CUU subtracts, floored at 1" { } test "CursorTracker: CUD adds, capped at max_rows" { - var c = CursorTracker.init(20); + var c = CursorTracker.init(20, 80); c.feed("\x1B[5B"); try testing.expectEqual(@as(u16, 6), c.currentRow()); c.feed("\x1B[100B"); @@ -79,13 +79,13 @@ test "CursorTracker: CUD adds, capped at max_rows" { } test "CursorTracker: VPA sets row absolutely" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[15d"); try testing.expectEqual(@as(u16, 15), c.currentRow()); } test "CursorTracker: CNL / CPL behave like CUD / CUU for row tracking" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[10;1H"); c.feed("\x1B[3E"); // CNL — row + 3 try testing.expectEqual(@as(u16, 13), c.currentRow()); @@ -94,7 +94,7 @@ test "CursorTracker: CNL / CPL behave like CUD / CUU for row tracking" { } test "CursorTracker: SGR / ED / EL / unknown CSI do not move the row" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B[5;1H"); try testing.expectEqual(@as(u16, 5), c.currentRow()); c.feed("\x1B[2J"); // ED @@ -108,7 +108,7 @@ test "CursorTracker: SGR / ED / EL / unknown CSI do not move the row" { } test "CursorTracker: mixed prompt-like stream tracks correctly" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); // Simulate: home + clear, then 5 lines of output, then prompt. c.feed("\x1B[H\x1B[2J"); try testing.expectEqual(@as(u16, 1), c.currentRow()); @@ -119,7 +119,7 @@ test "CursorTracker: mixed prompt-like stream tracks correctly" { } test "CursorTracker: setMaxRows on shrink clamps row" { - var c = CursorTracker.init(40); + var c = CursorTracker.init(40, 80); c.feed("\x1B[30H"); try testing.expectEqual(@as(u16, 30), c.currentRow()); c.setMaxRows(20); @@ -127,7 +127,7 @@ test "CursorTracker: setMaxRows on shrink clamps row" { } test "CursorTracker: param overflow drops extra digits, doesn't crash" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); // 20-digit param — buffer is 16 bytes, extras dropped. c.feed("\x1B[12345678901234567890H"); // Parsing the truncated digits — implementation-defined exact @@ -136,7 +136,7 @@ test "CursorTracker: param overflow drops extra digits, doesn't crash" { } test "CursorTracker: param > u16 max clamps instead of falling back to default 1" { - var c = CursorTracker.init(80); + var c = CursorTracker.init(80, 80); c.feed("\x1B[10;1H"); // baseline: row 10 try testing.expectEqual(@as(u16, 10), c.currentRow()); // 999_999 is parseable as u32, overflows u16. With the previous @@ -148,7 +148,7 @@ test "CursorTracker: param > u16 max clamps instead of falling back to default 1 } test "CursorTracker: arbitrarily-long digit run saturates instead of falling back to default" { - var c = CursorTracker.init(80); + var c = CursorTracker.init(80, 80); c.feed("\x1B[10;1H"); // baseline: row 10 try testing.expectEqual(@as(u16, 10), c.currentRow()); // 16-digit param exceeds u64 mantissa when accumulated — the @@ -161,7 +161,7 @@ test "CursorTracker: arbitrarily-long digit run saturates instead of falling bac } test "CursorTracker: lone ESC followed by non-`[` returns to ground" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B"); // pending ESC c.feed("M"); // RI — we don't model it; just consume + back to ground c.feed("\x1B[5;1H"); @@ -169,9 +169,140 @@ test "CursorTracker: lone ESC followed by non-`[` returns to ground" { } test "CursorTracker: CSI split across feeds reassembles correctly" { - var c = CursorTracker.init(24); + var c = CursorTracker.init(24, 80); c.feed("\x1B["); c.feed("10"); c.feed(";5H"); try testing.expectEqual(@as(u16, 10), c.currentRow()); } + +// ============================================================================= +// Column tracking +// ============================================================================= + +test "CursorTracker: col starts at 1, printables advance" { + var c = CursorTracker.init(24, 80); + try testing.expectEqual(@as(u16, 1), c.currentCol()); + c.feed("hello"); + try testing.expectEqual(@as(u16, 6), c.currentCol()); +} + +test "CursorTracker: CR resets col, doesn't change row" { + var c = CursorTracker.init(24, 80); + c.feed("hello\r"); + try testing.expectEqual(@as(u16, 1), c.currentCol()); + try testing.expectEqual(@as(u16, 1), c.currentRow()); +} + +test "CursorTracker: CRLF resets col + advances row" { + var c = CursorTracker.init(24, 80); + c.feed("hi\r\n"); + try testing.expectEqual(@as(u16, 1), c.currentCol()); + try testing.expectEqual(@as(u16, 2), c.currentRow()); +} + +test "CursorTracker: BS decrements col, floored at 1" { + var c = CursorTracker.init(24, 80); + c.feed("abc"); + try testing.expectEqual(@as(u16, 4), c.currentCol()); + c.feed("\x08\x08"); + try testing.expectEqual(@as(u16, 2), c.currentCol()); + c.feed("\x08\x08\x08\x08\x08"); + try testing.expectEqual(@as(u16, 1), c.currentCol()); +} + +test "CursorTracker: HT jumps to next 8-col tab stop" { + var c = CursorTracker.init(24, 80); + c.feed("\t"); // col 1 → 9 + try testing.expectEqual(@as(u16, 9), c.currentCol()); + c.feed("x\t"); // col 9 → 10 → 17 + try testing.expectEqual(@as(u16, 17), c.currentCol()); +} + +test "CursorTracker: printable past max_cols soft-wraps to next row" { + var c = CursorTracker.init(24, 5); + c.feed("12345"); // col 1..5 → after = col 6 wraps? Actually 5 + 1 = 6 > 5 → wrap. After 5 chars, col=1, row=2. + try testing.expectEqual(@as(u16, 1), c.currentCol()); + try testing.expectEqual(@as(u16, 2), c.currentRow()); +} + +test "CursorTracker: CUF / CUB move col" { + var c = CursorTracker.init(24, 80); + c.feed("\x1B[10C"); // CUF 10 — col 11 + try testing.expectEqual(@as(u16, 11), c.currentCol()); + c.feed("\x1B[3D"); // CUB 3 — col 8 + try testing.expectEqual(@as(u16, 8), c.currentCol()); + c.feed("\x1B[100D"); // floor at 1 + try testing.expectEqual(@as(u16, 1), c.currentCol()); +} + +test "CursorTracker: CUP sets both row + col" { + var c = CursorTracker.init(24, 80); + c.feed("\x1B[10;25H"); + try testing.expectEqual(@as(u16, 10), c.currentRow()); + try testing.expectEqual(@as(u16, 25), c.currentCol()); +} + +test "CursorTracker: CHA / HPA set col absolutely" { + var c = CursorTracker.init(24, 80); + c.feed("\x1B[15G"); // CHA + try testing.expectEqual(@as(u16, 15), c.currentCol()); + c.feed("\x1B[42`"); // HPA + try testing.expectEqual(@as(u16, 42), c.currentCol()); +} + +test "CursorTracker: CNL / CPL reset col to 1" { + var c = CursorTracker.init(24, 80); + c.feed("hello world"); // col 12 + c.feed("\x1B[2E"); // CNL — col → 1, row += 2 + try testing.expectEqual(@as(u16, 1), c.currentCol()); + try testing.expectEqual(@as(u16, 3), c.currentRow()); +} + +test "CursorTracker: UTF-8 multi-byte counts as ONE col, not per byte" { + var c = CursorTracker.init(24, 80); + // "é" = 0xC3 0xA9 (2 bytes, 1 codepoint) + c.feed("a\xC3\xA9b"); + try testing.expectEqual(@as(u16, 4), c.currentCol()); +} + +test "CursorTracker: OSC 133 ;A resets col to 1" { + var c = CursorTracker.init(24, 80); + c.feed("garbage from previous command"); // col not 1 + c.feed("\x1B]133;A\x07"); + try testing.expectEqual(@as(u16, 1), c.currentCol()); +} + +test "CursorTracker: OSC 133 ;B snapshots prompt_end_col" { + var c = CursorTracker.init(24, 80); + c.feed("\x1B]133;A\x07"); // prompt start, col = 1 + c.feed("~ )"); // PS1 text, col → 4 + c.feed("\x1B]133;B\x07"); // input region start + try testing.expectEqual(@as(u16, 4), c.currentCol()); + try testing.expectEqual(@as(u16, 4), c.prompt_end_col); + // Subsequent typing doesn't change the snapshot. + c.feed("ls -la"); + try testing.expectEqual(@as(u16, 4), c.prompt_end_col); + try testing.expectEqual(@as(u16, 10), c.currentCol()); +} + +test "CursorTracker: ST-terminated OSC (`\\x1B\\\\`) also consumed correctly" { + var c = CursorTracker.init(24, 80); + c.feed("a\x1B]0;title\x1B\\b"); // OSC 0 title set with ST terminator + // The 'a' advanced col to 2; OSC consumed; 'b' should land at col 3. + try testing.expectEqual(@as(u16, 3), c.currentCol()); +} + +test "CursorTracker: OSC body bytes do NOT count as printables" { + var c = CursorTracker.init(24, 80); + c.feed("\x1B]0;hello world\x07"); + try testing.expectEqual(@as(u16, 1), c.currentCol()); +} + +test "CursorTracker: setPosition trusts the caller (used by DSR-6n reply)" { + var c = CursorTracker.init(24, 80); + c.feed("hello"); + c.setPosition(10, 25); + try testing.expectEqual(@as(u16, 10), c.currentRow()); + try testing.expectEqual(@as(u16, 25), c.currentCol()); +} diff --git a/src/proxy.zig b/src/proxy.zig index 065e7f4..578598d 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -250,15 +250,17 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { // stays at N. Setting `max_rows` to `effectiveRows()` lets the // tracker's LF-advance clamp at the same row the terminal does, // keeping row consistent with what the shell sees. - var cursor_tracker = CursorTracker.init(blk: { + var cursor_tracker = blk: { + var rows: u16 = 24; + var cols: u16 = 80; if (args.is_tty) { if (Pty.querySize(posix.STDOUT_FILENO)) |s| { - if (statusbar) |sb| break :blk sb.effectiveRows(); - break :blk s.rows; + cols = s.cols; + rows = if (statusbar) |sb| sb.effectiveRows() else s.rows; } else |_| {} } - break :blk 24; - }); + break :blk CursorTracker.init(rows, cols); + }; // Alternate-screen-buffer tracker — full-screen TUIs (k9s, vim, // less, htop, helix, lazygit, …) swap to the alt buffer with From 8f73dce21a58dab0fd1437ae80e2de2f31ded02a Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Mon, 18 May 2026 21:45:21 +0200 Subject: [PATCH 2/3] feat(cursor_dsr): DSR-6n reply interceptor + stdin filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 2 of 3 — the active-query side of cursor-position tracking. PR 1 added passive `cursor_tracker` (column + OSC 133 anchors); this adds the round-trip mechanism for moments when the passive model needs ground truth from the terminal. ## What New `src/cursor_dsr.zig` — small state machine that parses `\x1B[;R` (the DSR-6n cursor-position reply) out of an input byte stream: - `DsrParser.feed(input, out)` — walk bytes, drop reply bytes, return filtered output + parsed `{row, col}` on match. - `DsrParser.writeQuery(w)` — emit the `\x1B[6n` query. 11 unit tests pin: full reply in one chunk; embedded between printables; split across feeds; unrelated CSI (Up arrow) passes through; malformed (no `;`) doesn't match; double-reply in one chunk; mid-stream abort restores byte stream verbatim; saturating digit-clamp at u16 max; zero-param fields. ## Wired into proxy.zig After every stdin `posix.read`, route the bytes through `dsr_parser.feed` BEFORE keymap matching / dispatchInput / pty.master forward. On a successful reply: feed the `(row, col)` into `cursor_tracker.setPosition` so subsequent paint queries see the truth. When the entire chunk was a DSR reply (no user bytes), `continue` the main loop so dispatch sees nothing rather than an empty slice. ## Why not just always query? DSR-6n round-trip is ~10-30ms — fine for explicit user actions (panel open) but unacceptable per-keystroke. PR 3 will add the caller-side helpers that fire DSR at key moments only. ## Test plan - [x] `zig build test` — 616 pass (11 new in cursor_dsr_tests) - [x] `zig build -Dtarget=x86_64-linux-gnu` clean - [x] `zig fmt --check src/` clean Stacks on top of PR #100 (cursor_tracker col tracking). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cursor_dsr.zig | 190 +++++++++++++++++++++++++++++++++++++++ src/cursor_dsr_tests.zig | 118 ++++++++++++++++++++++++ src/proxy.zig | 24 ++++- src/unit_tests.zig | 1 + 4 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 src/cursor_dsr.zig create mode 100644 src/cursor_dsr_tests.zig diff --git a/src/cursor_dsr.zig b/src/cursor_dsr.zig new file mode 100644 index 0000000..79be8b8 --- /dev/null +++ b/src/cursor_dsr.zig @@ -0,0 +1,190 @@ +//! DSR-6n (Device Status Report — cursor position) reply interceptor. +//! +//! `\x1B[6n` is the standard ECMA-48 / VT100 query: the terminal +//! replies on stdin with `\x1B[;R`. atty uses this to +//! ground-truth the cursor at sensitive moments (inline panel open, +//! SIGWINCH, post-command `;D`) when the passive `cursor_tracker` +//! model might have drifted. Reply parsing has to happen BEFORE the +//! stdin bytes reach the keymap / dispatchInput pipeline — otherwise +//! the reply gets forwarded to bash as if the user typed `[24;1R`. +//! +//! ## State machine +//! +//! Same shape as `osc133.zig`'s parser: walk bytes, drop into a +//! `csi_reply` state on `\x1B[`, accumulate digits + `;`, terminate +//! on `R` (success) or anything outside the accepted alphabet (abort +//! and pass through). Bytes that PARTICIPATE in a successful reply +//! are CONSUMED — `consumed_indices` records them so the caller can +//! filter the input slice. +//! +//! ## Scope of "consumed" +//! +//! We only consume bytes when we matched the FULL `\x1B[;R` +//! shape. Partial-match abandonment (e.g. user types `\x1B[A` for +//! Up-arrow — same prefix but different final) leaves the bytes in +//! the original stream so legacy keymap matching still works. The +//! consequence: a DSR reply gets parsed only when it's complete in +//! one chunk OR carries over correctly across two chunks (the +//! `pending` state handles that). Mid-stream partial matches that +//! abandon don't get retroactively re-fed. +//! +//! ## Non-goals +//! +//! - Multi-reply pipelining. atty fires DSR at most once per "key +//! moment" and waits for the reply (or times out) before firing +//! another. The parser only handles one in-flight reply at a time. +//! - Other DSR variants (DSR-5 status, DSR-15 printer). Not used. + +const std = @import("std"); + +pub const Position = struct { row: u16, col: u16 }; + +pub const DsrParser = struct { + state: State = .ground, + digits_buf: [16]u8 = undefined, + digits_len: u8 = 0, + row: u16 = 0, + col: u16 = 0, + /// True while we've started parsing a `\x1B[…` sequence — the + /// caller should withhold these bytes from downstream until we + /// either complete or abort. + pending: bool = false, + /// Bytes since the last consumed reply that contributed to the + /// pending sequence. Indexes into the caller's slice are + /// recomputed each `feed` call (no persistent indices). + pending_byte_count: usize = 0, + + const State = enum { ground, esc, csi, row_done }; + + /// Build the output slice by filtering the input through the + /// parser. Bytes that BELONG to a successful DSR reply are + /// dropped; everything else is preserved. Returns the position + /// parsed (if any) and a side-buffer of the filtered bytes. + /// + /// `out` must be at least `input.len` bytes. Caller passes a + /// scratch buffer they're already writing to. + pub fn feed(self: *DsrParser, input: []const u8, out: []u8) struct { filtered_len: usize, pos: ?Position } { + var w: usize = 0; + var result_pos: ?Position = null; + // Track the start index of the current pending sequence in + // `out` so we can rewind the filtered write if the sequence + // turns out to NOT be a DSR reply (abort path). + var pending_w_start: usize = 0; + + for (input) |b| { + switch (self.state) { + .ground => { + if (b == 0x1B) { + self.state = .esc; + self.pending = true; + pending_w_start = w; + out[w] = b; + w += 1; + } else { + out[w] = b; + w += 1; + } + }, + .esc => { + if (b == '[') { + self.state = .csi; + self.digits_len = 0; + self.row = 0; + self.col = 0; + out[w] = b; + w += 1; + } else { + // Not the shape we want. Pass through. + self.state = .ground; + self.pending = false; + out[w] = b; + w += 1; + } + }, + .csi => { + if (b >= '0' and b <= '9' and self.digits_len < self.digits_buf.len) { + self.digits_buf[self.digits_len] = b; + self.digits_len += 1; + out[w] = b; + w += 1; + } else if (b == ';') { + self.row = parseClamped(self.digits_buf[0..self.digits_len]); + self.digits_len = 0; + self.state = .row_done; + out[w] = b; + w += 1; + } else if (b == 'R' or b == 'n') { + // 'R' is a malformed reply (no `;`) — treat + // as abort. 'n' would be DSR query itself + // (we don't care). Both abort. + self.state = .ground; + self.pending = false; + out[w] = b; + w += 1; + } else { + // Unknown CSI body byte — abandon DSR parse, + // keep bytes in the output stream so keymap + // matchers / readline see them. + self.state = .ground; + self.pending = false; + out[w] = b; + w += 1; + } + }, + .row_done => { + if (b >= '0' and b <= '9' and self.digits_len < self.digits_buf.len) { + self.digits_buf[self.digits_len] = b; + self.digits_len += 1; + out[w] = b; + w += 1; + } else if (b == 'R') { + // Full reply received — rewind the filtered + // buffer to before the `\x1B`, dropping the + // entire DSR sequence from the stream. + self.col = parseClamped(self.digits_buf[0..self.digits_len]); + result_pos = .{ .row = self.row, .col = self.col }; + w = pending_w_start; + self.state = .ground; + self.pending = false; + self.digits_len = 0; + } else { + // Malformed (no terminator) — abandon. + self.state = .ground; + self.pending = false; + out[w] = b; + w += 1; + } + }, + } + } + + return .{ .filtered_len = w, .pos = result_pos }; + } + + /// Emit the DSR-6n query sequence into `w`. Caller writes the + /// result to STDOUT; the terminal replies on stdin which `feed` + /// will parse. + pub fn writeQuery(w: *std.Io.Writer) !void { + try w.writeAll("\x1B[6n"); + } +}; + +fn parseClamped(digits: []const u8) u16 { + var acc: u32 = 0; + const cap: u32 = std.math.maxInt(u16); + for (digits) |b| { + if (b < '0' or b > '9') continue; + if (acc >= cap) { + acc = cap; + continue; + } + const d: u32 = b - '0'; + const next: u64 = @as(u64, acc) * 10 + d; + acc = if (next > cap) cap else @intCast(next); + } + return @intCast(acc); +} + +test { + _ = @import("cursor_dsr_tests.zig"); +} diff --git a/src/cursor_dsr_tests.zig b/src/cursor_dsr_tests.zig new file mode 100644 index 0000000..9f44618 --- /dev/null +++ b/src/cursor_dsr_tests.zig @@ -0,0 +1,118 @@ +//! Tests for `cursor_dsr.zig` — the DSR-6n reply interceptor. + +const std = @import("std"); +const testing = std.testing; +const mod = @import("cursor_dsr.zig"); + +const DsrParser = mod.DsrParser; + +test "DsrParser: full reply in one chunk is consumed; position returned" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[24;80R", &out); + try testing.expectEqual(@as(usize, 0), r.filtered_len); + try testing.expect(r.pos != null); + try testing.expectEqual(@as(u16, 24), r.pos.?.row); + try testing.expectEqual(@as(u16, 80), r.pos.?.col); +} + +test "DsrParser: reply embedded between printable bytes — only reply consumed" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("a\x1B[10;5Rb", &out); + try testing.expect(r.pos != null); + try testing.expectEqual(@as(u16, 10), r.pos.?.row); + try testing.expectEqual(@as(u16, 5), r.pos.?.col); + try testing.expectEqualStrings("ab", out[0..r.filtered_len]); +} + +test "DsrParser: unrelated CSI (`\\x1b[A` — Up arrow) passes through" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[A", &out); + try testing.expect(r.pos == null); + try testing.expectEqualStrings("\x1B[A", out[0..r.filtered_len]); +} + +test "DsrParser: CSI with single param ending in `R` (no `;`) doesn't match" { + // A real DSR reply always has row + col separated by `;`. A + // sequence like `\x1B[24R` is malformed — pass through. + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[24R", &out); + try testing.expect(r.pos == null); + try testing.expectEqualStrings("\x1B[24R", out[0..r.filtered_len]); +} + +test "DsrParser: reply split across two feeds reassembles correctly" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r1 = p.feed("\x1B[12;", &out); + // No completion yet — bytes go to output buffer (caller hasn't + // decided yet whether to forward them; in the proxy they get + // filtered out via the rewind on completion). + try testing.expect(r1.pos == null); + + var out2: [64]u8 = undefined; + const r2 = p.feed("45R", &out2); + try testing.expect(r2.pos != null); + try testing.expectEqual(@as(u16, 12), r2.pos.?.row); + try testing.expectEqual(@as(u16, 45), r2.pos.?.col); + // Second chunk's filtered output should be empty (everything + // was part of the reply). + try testing.expectEqual(@as(usize, 0), r2.filtered_len); +} + +test "DsrParser: zero-param fields parse as 0 (caller's job to clamp)" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[;R", &out); + try testing.expect(r.pos != null); + try testing.expectEqual(@as(u16, 0), r.pos.?.row); + try testing.expectEqual(@as(u16, 0), r.pos.?.col); +} + +test "DsrParser: massive digit values saturate at u16 max" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[99999;88888R", &out); + try testing.expect(r.pos != null); + try testing.expectEqual(@as(u16, std.math.maxInt(u16)), r.pos.?.row); + try testing.expectEqual(@as(u16, std.math.maxInt(u16)), r.pos.?.col); +} + +test "DsrParser: writeQuery emits the standard sequence" { + var buf: [16]u8 = undefined; + var w: std.Io.Writer = .fixed(&buf); + try DsrParser.writeQuery(&w); + try testing.expectEqualStrings("\x1B[6n", buf[0..w.end]); +} + +test "DsrParser: two replies in a single chunk both parse" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r1 = p.feed("\x1B[1;2R", &out); + try testing.expect(r1.pos != null); + try testing.expectEqual(@as(u16, 1), r1.pos.?.row); + try testing.expectEqual(@as(u16, 2), r1.pos.?.col); + + var out2: [64]u8 = undefined; + const r2 = p.feed("\x1B[3;4R", &out2); + try testing.expect(r2.pos != null); + try testing.expectEqual(@as(u16, 3), r2.pos.?.row); + try testing.expectEqual(@as(u16, 4), r2.pos.?.col); +} + +test "DsrParser: abort mid-CSI (`\\x1b[12;abc`) restores byte stream verbatim" { + // If the user types something that LOOKS like the start of a + // DSR reply but isn't, the parser must release the bytes so + // keymap matching downstream still works. + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r = p.feed("\x1B[12;a", &out); + try testing.expect(r.pos == null); + // The chunk contained `\x1B[12;` (5 bytes accumulated as + // pending) + `a` (abort). After abort the parser should have + // released all 6 bytes through the output buffer. + try testing.expectEqualStrings("\x1B[12;a", out[0..r.filtered_len]); +} diff --git a/src/proxy.zig b/src/proxy.zig index 578598d..6eec25f 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -48,6 +48,7 @@ const keymap = @import("keymap.zig"); const Osc133 = @import("osc133.zig").Osc133; const AltScreen = @import("altscreen.zig").AltScreen; const CursorTracker = @import("cursor_tracker.zig").CursorTracker; +const DsrParser = @import("cursor_dsr.zig").DsrParser; const Osc7 = @import("osc7.zig").Osc7; const subprocess_mod = @import("subprocess.zig"); const overlay_ring = @import("overlay_ring.zig"); @@ -262,6 +263,18 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { break :blk CursorTracker.init(rows, cols); }; + // DSR-6n reply interceptor. atty issues `\x1B[6n` at key moments + // (inline panel open, SIGWINCH, post-command `;D`) to ground- + // truth the cursor position; the terminal replies on stdin with + // `\x1B[;R`. The parser strips that reply from the stream + // before keymap matching / dispatch / pty.master forward so the + // shell never sees it as user input. + var dsr_parser = DsrParser{}; + // Scratch buffer the parser writes filtered stdin bytes into. + // Sized to match `read_buf` since the filtered output is at most + // as long as the input. + var stdin_filtered_buf: [4096]u8 = undefined; + // Alternate-screen-buffer tracker — full-screen TUIs (k9s, vim, // less, htop, helix, lazygit, …) swap to the alt buffer with // `\x1b[?1049h` and back with `?1049l`. While they're active the @@ -584,7 +597,16 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, args: Args) !ExitInfo { if (pfds[0].revents & POLLIN != 0) { const read_n = posix.read(posix.STDIN_FILENO, &read_buf) catch 0; if (read_n > 0) { - var input: []const u8 = read_buf[0..read_n]; + // DSR-6n reply intercept — `\x1B[;R` is the + // terminal's response to our cursor-position query; + // strip it before bash sees it as keyboard input. + const dsr_result = dsr_parser.feed(read_buf[0..read_n], &stdin_filtered_buf); + if (dsr_result.pos) |pos| { + cursor_tracker.setPosition(pos.row, pos.col); + trace.log(.cursor, "DSR-6n reply: row={d} col={d}", .{ pos.row, pos.col }); + } + var input: []const u8 = stdin_filtered_buf[0..dsr_result.filtered_len]; + if (input.len == 0) continue; // entire chunk was DSR reply trace.logBytes(.input, "stdin_read", input); trace.log(.altscreen, "alt_screen.active={} module_overlay_active={}", .{ alt_screen.active, ctx.module_overlay_active }); diff --git a/src/unit_tests.zig b/src/unit_tests.zig index c99ea7b..0c97c72 100644 --- a/src/unit_tests.zig +++ b/src/unit_tests.zig @@ -16,6 +16,7 @@ test { _ = @import("osc133.zig"); _ = @import("altscreen.zig"); _ = @import("cursor_tracker.zig"); + _ = @import("cursor_dsr.zig"); _ = @import("subprocess.zig"); _ = @import("osc7.zig"); _ = @import("pty.zig"); From 597ca43b120958b4068aac3f1e1d47ac51c19e82 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Mon, 18 May 2026 21:57:24 +0200 Subject: [PATCH 3/3] fix(cursor_dsr): withhold pending bytes in internal buffer until commit/abort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subagent review round 1 caught two real bugs in the cross-chunk path: 1. Split-chunk leak: chunk 1 of `\x1B[12;` was being written to `out` (filtered_len=5) BEFORE the parser knew it was a reply. Bash saw a phantom `\x1B[12;` keystroke. The retroactive rewind only spanned the current `feed` call — chunk 1's bytes had already escaped. 2. Cross-chunk false rewind: in a continuation chunk the `pending_w_start` was always 0, so on completion the rewind would drop any user bytes that appeared earlier in that chunk. Fix: buffer pending bytes in a parser-internal `pending_buf` (32 B covers the largest legitimate DSR). They DON'T appear in `feed`'s output until either: - reply completes successfully → drop the whole buffer - sequence aborts → flush the buffer verbatim (so keymap matchers downstream still see the bytes) Tightened the split-chunk test (asserts filtered_len == 0 in both chunks now — previously only checked chunk 1's `pos == null`, which masked the leak) + added two new tests: - split reply with user bytes after the partial: aborts cleanly, partial bytes flush verbatim - idle empty feed between split chunks: pending state persists Removed the dead `pending` bool + `pending_byte_count` field (set/cleared but never read across the public API). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cursor_dsr.zig | 106 +++++++++++++++++++++------------------ src/cursor_dsr_tests.zig | 53 +++++++++++++++++--- 2 files changed, 104 insertions(+), 55 deletions(-) diff --git a/src/cursor_dsr.zig b/src/cursor_dsr.zig index 79be8b8..5ab245a 100644 --- a/src/cursor_dsr.zig +++ b/src/cursor_dsr.zig @@ -45,41 +45,42 @@ pub const DsrParser = struct { digits_len: u8 = 0, row: u16 = 0, col: u16 = 0, - /// True while we've started parsing a `\x1B[…` sequence — the - /// caller should withhold these bytes from downstream until we - /// either complete or abort. - pending: bool = false, - /// Bytes since the last consumed reply that contributed to the - /// pending sequence. Indexes into the caller's slice are - /// recomputed each `feed` call (no persistent indices). - pending_byte_count: usize = 0, + /// Bytes accumulated while we're parsing a possible reply. + /// They DON'T appear in `feed`'s output until either: + /// - the reply completes successfully → drop the whole buffer + /// - the sequence aborts → flush the buffer to output verbatim + /// Withholding here is required for cross-chunk splits: a reply + /// like `\x1B[12;` (chunk 1) + `45R` (chunk 2) would otherwise + /// leak the 5 chunk-1 bytes to bash before we know it's a reply. + /// 32 bytes covers the largest legitimate DSR reply (`\x1B[<5 + /// digits>;<5 digits>R` = 13) with comfort. + pending_buf: [32]u8 = undefined, + pending_len: u8 = 0, const State = enum { ground, esc, csi, row_done }; /// Build the output slice by filtering the input through the /// parser. Bytes that BELONG to a successful DSR reply are /// dropped; everything else is preserved. Returns the position - /// parsed (if any) and a side-buffer of the filtered bytes. + /// parsed (if any) and the filtered byte count. /// /// `out` must be at least `input.len` bytes. Caller passes a /// scratch buffer they're already writing to. + /// + /// Bytes that are part of an in-flight (not yet + /// completed-or-aborted) sequence are NOT written to `out` — + /// they live in the parser's internal `pending_buf` and either + /// vanish (success) or get flushed (abort) on a later feed call. pub fn feed(self: *DsrParser, input: []const u8, out: []u8) struct { filtered_len: usize, pos: ?Position } { var w: usize = 0; var result_pos: ?Position = null; - // Track the start index of the current pending sequence in - // `out` so we can rewind the filtered write if the sequence - // turns out to NOT be a DSR reply (abort path). - var pending_w_start: usize = 0; for (input) |b| { switch (self.state) { .ground => { if (b == 0x1B) { self.state = .esc; - self.pending = true; - pending_w_start = w; - out[w] = b; - w += 1; + self.pushPending(b); } else { out[w] = b; w += 1; @@ -91,68 +92,57 @@ pub const DsrParser = struct { self.digits_len = 0; self.row = 0; self.col = 0; - out[w] = b; - w += 1; + self.pushPending(b); } else { - // Not the shape we want. Pass through. - self.state = .ground; - self.pending = false; + // Not the shape we want — flush pending + + // current byte verbatim so keymap matchers + // downstream see the original sequence. + w += self.flushPending(out[w..]); out[w] = b; w += 1; + self.state = .ground; } }, .csi => { if (b >= '0' and b <= '9' and self.digits_len < self.digits_buf.len) { self.digits_buf[self.digits_len] = b; self.digits_len += 1; - out[w] = b; - w += 1; + self.pushPending(b); } else if (b == ';') { self.row = parseClamped(self.digits_buf[0..self.digits_len]); self.digits_len = 0; self.state = .row_done; - out[w] = b; - w += 1; - } else if (b == 'R' or b == 'n') { - // 'R' is a malformed reply (no `;`) — treat - // as abort. 'n' would be DSR query itself - // (we don't care). Both abort. - self.state = .ground; - self.pending = false; - out[w] = b; - w += 1; + self.pushPending(b); } else { - // Unknown CSI body byte — abandon DSR parse, - // keep bytes in the output stream so keymap - // matchers / readline see them. - self.state = .ground; - self.pending = false; + // Anything else aborts (including a stray + // 'R' with no `;` — malformed reply). Flush + // pending + the abort byte verbatim. + w += self.flushPending(out[w..]); out[w] = b; w += 1; + self.state = .ground; } }, .row_done => { if (b >= '0' and b <= '9' and self.digits_len < self.digits_buf.len) { self.digits_buf[self.digits_len] = b; self.digits_len += 1; - out[w] = b; - w += 1; + self.pushPending(b); } else if (b == 'R') { - // Full reply received — rewind the filtered - // buffer to before the `\x1B`, dropping the - // entire DSR sequence from the stream. + // Full reply received — drop pending buffer + // (the DSR sequence is consumed). self.col = parseClamped(self.digits_buf[0..self.digits_len]); result_pos = .{ .row = self.row, .col = self.col }; - w = pending_w_start; + self.pending_len = 0; self.state = .ground; - self.pending = false; self.digits_len = 0; } else { - // Malformed (no terminator) — abandon. - self.state = .ground; - self.pending = false; + // Malformed (no terminator) — abandon, flush + // pending + the abort byte. + w += self.flushPending(out[w..]); out[w] = b; w += 1; + self.state = .ground; } }, } @@ -161,6 +151,24 @@ pub const DsrParser = struct { return .{ .filtered_len = w, .pos = result_pos }; } + fn pushPending(self: *DsrParser, b: u8) void { + if (self.pending_len < self.pending_buf.len) { + self.pending_buf[self.pending_len] = b; + self.pending_len += 1; + } + // Overflow: silently drop bytes past the buffer. A + // legitimate DSR is ≤ 13 bytes; if we overflow we're + // looking at a hostile / malformed sequence that wasn't + // going to complete cleanly anyway. + } + + fn flushPending(self: *DsrParser, out: []u8) usize { + const n: usize = self.pending_len; + if (n > 0) @memcpy(out[0..n], self.pending_buf[0..n]); + self.pending_len = 0; + return n; + } + /// Emit the DSR-6n query sequence into `w`. Caller writes the /// result to STDOUT; the terminal replies on stdin which `feed` /// will parse. diff --git a/src/cursor_dsr_tests.zig b/src/cursor_dsr_tests.zig index 9f44618..96c710a 100644 --- a/src/cursor_dsr_tests.zig +++ b/src/cursor_dsr_tests.zig @@ -44,25 +44,66 @@ test "DsrParser: CSI with single param ending in `R` (no `;`) doesn't match" { try testing.expectEqualStrings("\x1B[24R", out[0..r.filtered_len]); } -test "DsrParser: reply split across two feeds reassembles correctly" { +test "DsrParser: reply split across two feeds — NEITHER chunk leaks bytes" { + // Critical invariant: pending bytes stay in the parser's + // internal buffer until completion (drop) or abort (flush). + // A naive implementation would write `\x1B[12;` into the first + // chunk's filtered output and forward to bash before learning + // it was a reply. var p = DsrParser{}; var out: [64]u8 = undefined; const r1 = p.feed("\x1B[12;", &out); - // No completion yet — bytes go to output buffer (caller hasn't - // decided yet whether to forward them; in the proxy they get - // filtered out via the rewind on completion). try testing.expect(r1.pos == null); + try testing.expectEqual(@as(usize, 0), r1.filtered_len); var out2: [64]u8 = undefined; const r2 = p.feed("45R", &out2); try testing.expect(r2.pos != null); try testing.expectEqual(@as(u16, 12), r2.pos.?.row); try testing.expectEqual(@as(u16, 45), r2.pos.?.col); - // Second chunk's filtered output should be empty (everything - // was part of the reply). try testing.expectEqual(@as(usize, 0), r2.filtered_len); } +test "DsrParser: split reply with user bytes BEFORE the tail — user bytes preserved" { + // Pathological case: chunk 1 starts a reply, chunk 2 contains + // user keystrokes BEFORE the reply completes (the user typed + // while the terminal queued the DSR response). The user bytes + // would abort the reply parse and must reach bash; the + // pending-buffer flush emits the partial-reply bytes + // VERBATIM so keymap matchers still see them. + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r1 = p.feed("\x1B[12;", &out); + try testing.expectEqual(@as(usize, 0), r1.filtered_len); + + // Chunk 2 has user input that aborts the pending reply. + var out2: [64]u8 = undefined; + const r2 = p.feed("xls\r", &out2); + try testing.expect(r2.pos == null); + // Aborted: the pending `\x1B[12;` flushes verbatim followed by + // `xls\r`. + try testing.expectEqualStrings("\x1B[12;xls\r", out2[0..r2.filtered_len]); +} + +test "DsrParser: in-flight pending across an idle feed call — bytes still withheld" { + var p = DsrParser{}; + var out: [64]u8 = undefined; + const r1 = p.feed("\x1B[", &out); + try testing.expectEqual(@as(usize, 0), r1.filtered_len); + + // Empty feed (nothing arrives this tick). + const r2 = p.feed("", &out); + try testing.expectEqual(@as(usize, 0), r2.filtered_len); + try testing.expect(r2.pos == null); + + // Reply finishes later. + const r3 = p.feed("1;2R", &out); + try testing.expect(r3.pos != null); + try testing.expectEqual(@as(u16, 1), r3.pos.?.row); + try testing.expectEqual(@as(u16, 2), r3.pos.?.col); + try testing.expectEqual(@as(usize, 0), r3.filtered_len); +} + test "DsrParser: zero-param fields parse as 0 (caller's job to clamp)" { var p = DsrParser{}; var out: [64]u8 = undefined;