capture-pane: trim trailing all-empty rows from styled (-e) output#330
Conversation
|
@psmux — heads-up, small ~5-line patch in (Earlier comments had their backticked code refs swallowed by my shell — please disregard those, this one is the proper version.) |
|
Hey @quazardous, thanks for the well documented PR and the clear explanation of the downstream use case. I checked tmux's source code to compare behavior. In tmux, The observation that 'tmux users on Linux saw the cursor land at the expected position' is likely due to the downstream rendering layer handling tmux output differently (perhaps the xterm.js integration on Linux queries cursor position separately or the tmux render pipeline re positions the cursor through a different mechanism), not because tmux embeds cursor positioning in capture output. That said, I understand the practical problem: when piping styled capture into xterm.js for a screen mirror, the cursor lands at the bottom instead of where the user's prompt sits. That's a real UX issue for your use case. However, I'm not comfortable merging this into capture-pane output directly because:
What I'd suggest instead: We could handle this through a separate mechanism. A couple of options:
Would any of those work for your screen mirror setup? Happy to collaborate on whichever approach fits best. The flag based approach would be the cleanest since it gives consumers explicit control. |
The non-styled siblings `capture_active_pane_text` and
`capture_active_pane_range` have for years trimmed trailing empty
rows from their output (the `while text.ends_with("\n\n") { text.pop(); }`
loop, with a comment about iTerm2 placing the next prompt at the
bottom of the window when the trailing blanks pushed the cursor down).
The styled path (`capture_active_pane_styled`, the `-e` flag) renders
every visible row regardless and emits all trailing blanks as either
`\n` or `\x1b[0m\n`. A downstream consumer that pipes the snapshot
into a terminal (xterm.js for a screen-mirror UI, a fresh xterm
window, …) leaves its cursor under the visible content — by as many
rows as the pane has trailing blanks.
This is what aiball#531 surfaced (psmux on Windows) and what the POC
linked from that ticket confirmed on tmux Linux as well: real
`display-message #{cursor_x}` at column 8, xterm.js `cursorX` at 81
(wrap to column 80 + 1) — same shift, just less visually obvious on
Linux because column 81 clamps to the right edge.
Fix: add the equivalent trim for the styled path. A plain
ends_with("\n\n") test doesn't work here because an empty row that
follows a styled one carries an `\x1b[0m` SGR reset between the
newlines, so the helper handles that pattern: pop a trailing line
whose content consists only of optional `\x1b[0m` resets, repeat
until the last line carries real content (or the buffer becomes
empty).
Applied to both the fast path (no scrollback) and the scrollback path
— same as the plain path treats them.
Replaces the prior cursor-positioning escape append that this PR
originally proposed. That approach changed `capture-pane -e -p`
output semantics (appended a CUP sequence) which the maintainer
correctly pushed back on. Trim-trailing-empty matches the plain
path's existing convention, introduces no new semantics, no new
flag, and benefits every consumer that pipes styled capture into a
terminal (not just our specific xterm.js screen-mirror use case).
Verified:
- `cargo build --release` clean
- 9 new unit tests in `trim_trailing_empty_styled_lines_tests` cover
empty/no-newline/single-trailing/blank-collapse/SGR-collapse/
mixed-blank-and-reset/all-empty-truncates/inline-resets-kept/
double-resets paths. 9/9 pass.
Reported by david via aiball ticket #531 + cross-platform POC at
https://github.com/quazardous/aiball/tree/poc/531-capture-cursor-tmux
5f10734 to
f5784ea
Compare
|
@psmux — refactored per your option (c) suggestion: trim trailing all-empty rows in the styled path instead of appending a CUP escape. This matches what Empirical validation across mux flavoursA downstream consumer (the aiball web terminal that surfaced this) ran the same harness against tmux on Linux. Numbers from that POC:
Same shift on tmux Linux as we'd seen on psmux Windows — just less visually obvious because the column 81 wraps to the right edge of an 80-col pane. So this isn't a psmux deviation in either direction — it's a downstream-pipeline issue every consumer of styled capture hits when trailing rows are blank. Aiball's defensive downstream fix (query Tests9 new unit tests in
9/9 pass. (The original CUP-append commit on this branch has been amended away — the branch now carries only the trim helper + its callers + tests.) |
|
Hey @quazardous, merged in 009cdef. Re-ran the helper unit suite locally and got 9/9 (�mpty_input_stays_empty, |
Summary
The styled capture path (
capture-pane -e -pwithout an explicit range) renders every visible row, including all-empty trailing rows which it emits as\x1b[0m\n/\n. A downstream consumer that pipes the snapshot into a terminal (xterm.js for a screen-mirror UI, a freshxtermwindow, …) lands the cursor wherever the last byte of the snapshot left it — typically the row right under the pane's content, not where the cursor actually sits in the pane.The non-styled siblings (
capture_active_pane_text,capture_active_pane_range) already trim trailing empty rows for the same reason (see the existingwhile text.ends_with("\n\n") { ... }+ comment about iTerm2). The styled path didn't have an equivalent because the trailing rows can carry SGR resets between newlines, so the plain newline-popping trim doesn't cleanly apply.Fix
Instead of attempting an SGR-aware trim, append an explicit
\x1b[<row+1>;<col+1>Hcursor-positioning escape at the very end of the output. The cursor lands where the pane actually has it, regardless of how many empty rows trail.The escape is appended only when the capture covers the default visible range (
s.is_none() && e.is_none()) — for explicit-S/-Eslicing the cursor may sit outside the captured area, so emitting it there would mislead consumers.The scrollback-aware styled path (
needs_scrollback = true) is left untouched: its primary use is to dump historical buffer, where re-positioning to the live cursor at the tail would be confusing — the captured rows are not the visible state.Test plan
cargo build --releasecleancargo test --release— 2027/2028 pass (1 pre-existing flaky Windows shell-capture timing test,commands::tests_new_commands::run_shell_stderr_is_captured, unrelated to this change)capture-pane -e -pevery second) sees the cursor land at the pane's actual position instead of below the visible content, on Windows with the patched binaryContext
Reported downstream by an xterm.js-based screen-mirror UI on Windows:
tmuxusers on Linux saw the cursor land at the expected position out of the box (something in the tmux render path positions it correctly), butpsmuxusers on Windows saw the cursor pushed below the prompt by the trailing empty rows. This patch makes the behaviour consistent for both flavours of consumer.