Skip to content

fix: prevent ghost cursor on DECSCUSR style change#19

Merged
jesse23 merged 2 commits intomainfrom
jesse_test
Mar 27, 2026
Merged

fix: prevent ghost cursor on DECSCUSR style change#19
jesse23 merged 2 commits intomainfrom
jesse_test

Conversation

@jesse23
Copy link
Copy Markdown
Owner

@jesse23 jesse23 commented Mar 27, 2026

Summary

  • Fix vim cursor shape not changing visually when switching between normal and insert mode
  • Add ADR 015 documenting the root cause and fix
  • Remove stale docs/adrs/015.ordering-system.pricing-storage.md (unrelated leftover, deleted in prior commit 3fce33c)

Root cause

ADR 013 (cursor.ts) correctly intercepts DECSCUSR sequences and updates term.options.cursorStyle via ghostty-web's options Proxy → renderer.setCursorStyle(). But ghostty-web's render loop only clears the cursor row (to erase the old shape) under two conditions:

const s = cursor.x !== lastPos.x || cursor.y !== lastPos.y;
if (s || this.cursorBlink) {
  renderLine(cursor_row);  // erases old cursor
}
renderCursor(...);         // always draws new cursor on top

When vim switches normal → insert (\x1b[2 q steady block → \x1b[6 q steady bar), neither fires — cursor stays put, both modes non-blinking. Old block cursor background persists on canvas; bar is drawn on top of it, leaving the cursor visually unchanged ("ghost cursor").

Why ADR 013 manual verification passed: vim's full-screen redraw on open makes all rows dirty, so the first mode switch works correctly. The gap only appears on subsequent in-place mode switches (pressing i / Esc without moving the cursor first).

Fix

Capture initial style/blink at function entry, process all sequences, then compare final vs initial:

const initialStyle = term.options.cursorStyle;
const initialBlink = term.options.cursorBlink;
// ... process DECSCUSR sequences ...
if (
  (term.options.cursorStyle !== initialStyle || term.options.cursorBlink !== initialBlink) &&
  term.renderer && term.wasmTerm
) {
  term.renderer.render(term.wasmTerm, true, term.viewportY);
}

Comparing net initial→final (rather than a mid-loop flag) avoids a spurious repaint when multiple sequences in one chunk cancel each other out. Fires only on vim mode switches — no per-message overhead.

Changes

  • src/client/cursor.ts — compare initial vs final style/blink, force repaint on net change
  • src/client/cursor.test.ts — tests covering repaint trigger and the cancel-out no-op case
  • docs/adrs/015.client.cursor-ghost-repaint.md — ADR documenting root cause, options considered, and relationship to ADR 013

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a rendering edge case in the web client where DECSCUSR cursor style/blink changes (e.g., vim normal/insert toggles without cursor movement) can leave the old cursor shape painted on the canvas (“ghost cursor”). The PR also documents the root cause and workaround via a new ADR.

Changes:

  • Track whether DECSCUSR updates actually change cursorStyle/cursorBlink, and force a full renderer repaint when they do.
  • Add unit tests covering the repaint trigger behavior.
  • Add an ADR documenting the rendering gap and the chosen mitigation (and remove an unrelated prior ADR 015 file).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/client/cursor.ts Forces a full repaint when DECSCUSR changes cursor style/blink to clear stale cursor pixels.
src/client/cursor.test.ts Adds tests ensuring the repaint is called (or not) under the right conditions.
docs/adrs/015.ordering-system.pricing-storage.md Removes an unrelated ADR 015 document.
docs/adrs/015.client.cursor-ghost-repaint.md Adds ADR documenting the ghost-cursor root cause and fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/adrs/015.client.cursor-ghost-repaint.md
Comment thread src/client/cursor.ts Outdated
ghostty-web's render loop only clears the cursor row when the cursor moves or cursorBlink=true. Switching vim normal→insert (steady block→bar) satisfies neither: cursor stays put, both modes non-blinking. The old block cursor shape persisted on canvas; the new bar was drawn on top, leaving the cursor visually unchanged.

Force a full repaint (forceAll=true) in applyDecscusr when the net cursor style/blink differs from the initial values at function entry. Capturing initial vs final (rather than a mid-loop changed flag) avoids a spurious repaint when multiple sequences in one chunk cancel each other out.

ADR 013 manual verification passed because vim's full-screen redraw on open makes all rows dirty, clearing the cursor correctly. The gap only shows on subsequent in-place mode switches.

Adds ADR 015 documenting the ghost cursor root cause and the fix.

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@jesse23 jesse23 merged commit 02513e4 into main Mar 27, 2026
3 checks passed
@jesse23 jesse23 deleted the jesse_test branch March 27, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants