fix: vim visual mode on hover, port config, and Windows PTY#29
Merged
fix: vim visual mode on hover, port config, and Windows PTY#29
Conversation
- Block mousemove (e.buttons===0) when mouse tracking is active so ghostty-web's hover-as-button-32 misreport never triggers vim visual mode on mouse movement. - Set pointer capture on the canvas on pointerdown so drag operations (e.g. vim split resize) are not interrupted when the pointer crosses outside the canvas boundary. - Add window resize listener alongside ResizeObserver to catch monitor hot-plug and DPI changes that resize the viewport without triggering the element-level observer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
process.env.PORT was silently overriding the config-file port, causing webtty to target a different port than configured when PORT happened to be set in the shell environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bun.Terminal does not support the terminal option on Windows (throws ERR_INVALID_ARG_TYPE), causing the WebSocket handler to crash on the first PTY spawn and dropping every connection with "Connection lost". node-pty works correctly under Bun on Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Improves webtty’s client-side input/resize behavior and aligns server/CLI startup behavior and PTY backend selection with the project’s configuration and Windows compatibility goals.
Changes:
- Prevents unwanted Vim visual-mode activation on hover and avoids losing drags when the pointer leaves the terminal canvas.
- Improves terminal fitting on monitor hot-plug/DPI changes by adding a
window.resizetrigger in addition toResizeObserver. - Removes
process.env.PORTprecedence in favor of always usingconfig.json’s port, and switches Windows PTY handling to prefernode-pty.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/server/index.ts | Stop using process.env.PORT; always bind to config.port. |
| src/pty/index.ts | Disable Bun PTY backend on Windows; fall back to node-pty. |
| src/client/index.ts | Add window resize fitting; pointer capture for drags; block hover mousemove propagation when mouse tracking is enabled. |
| src/cli/http.ts | Read CLI port from config (explicitly ignoring env PORT). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prevents biome formatter failures on Windows where git autocrlf=true was converting LF to CRLF on checkout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CLI already reads port exclusively from config, so ambient PORT never silently overrides a user's config. The server itself still needs to honor PORT so integration tests can bind to a free port and the server can be run directly with a custom port without editing config.json. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the CLI runs from source (src/cli/index.ts), __dirname is src/cli. The useNode path previously resolved ../server/index.js to src/server/index.js which does not exist. Now detect the running-from-source case via the .ts extension and jump two levels up to dist/server/index.js instead. Also write a port config into the test tmpHome before each runCli call so the CLI subprocess reads the correct test port from loadConfig() rather than defaulting to 2346. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- client: coalesce ResizeObserver + window resize calls through requestAnimationFrame so both signals produce one fit() per frame instead of two back-to-back layout passes. - http: replace eager module-level PORT/BASE_URL constants with lazy getPort()/getBaseUrl() functions so loadConfig() is only called when a command actually needs the port, avoiding unnecessary disk I/O and config-file creation on every CLI import. - test: normalize backslash paths before the server/index substring check so the mock works correctly on Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
setPointerCapture on pointerdown interfered with ghostty-web's internal pointer tracking, causing vim to enter and immediately exit visual mode on any drag. The hover-mousemove blocker alone is sufficient to prevent the original ghost visual-mode-on-hover issue. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Blocking mousemove DOM events in capture phase disrupted ghostty-web's internal mouse state, breaking vim drag (visual mode entered then immediately exited). New approach: let ghostty-web see all mouse events normally, then drop the outgoing \x1b[<32;...M (button-32 motion) sequences from onData when no mouse button is held. ghostty-web uses button-32 for both hover and button-1 drag; tracking mousedown/mouseup lets us distinguish them and only suppress the hover case before it reaches the PTY. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ouse ghostty-web encodes hover motion (e.buttons===0) as SGR button-32 (\x1b[<32;col;rowM) — the same code as a left-button drag — because its internal mouseButtonsPressed bitmask is 0 when no button is held (0+32=32). Vim with mouse=a enables mode 1003 (any-event tracking), so every hover move arrived as <LeftDrag>, toggling visual mode on each cursor movement. Two fixes in onData: 1. Hover rewrite: a capture-phase mousemove listener reads the DOM event's authoritative e.buttons before ghostty-web's bubble-phase handler encodes it. When e.buttons===0, rewrite \x1b[<32; to \x1b[<35; (Cb=35 = 32+3, the correct SGR no-button code). Position is still forwarded so vim can track the cursor for subsequent drag operations. 2. Drag dedup: ghostty-web fires multiple mousemove events per character cell. Vim treats <LeftDrag> at the same cell twice as a visual-mode toggle, so consecutive identical \x1b[<32;col;rowM sequences are deduplicated. Both fixes are gated on hasMouseTracking() and the \x1b[<32; prefix, so they are no-ops when mouse tracking is inactive or when other Cb values (press, release, scroll) are involved.
Move rewriteHoverMotion and isDuplicateDrag out of index.ts into a dedicated mouse.ts module so the pure transform logic can be unit-tested without a browser environment. Add mouse.test.ts with 12 tests covering hover rewrite, drag dedup, and all pass-through cases (press, release, scroll, plain text). index.ts now imports from mouse.ts; runtime behaviour is unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
added 4 commits
April 26, 2026 16:22
- commands.test.ts: restore HOME by deleting the key (not setting string 'undefined') when it was unset before the test mutated it - pty/index.test.ts: use COMSPEC/cmd.exe on Windows; skip interactive data test under Bun on Windows (Bun ConPTY pipe closes after initial banner) - pty/node.ts: pass /d to cmd.exe to disable AutoRun (clink registry hook) - websocket.test.ts: run server under Node on Windows+Bun (mirrors http.ts workaround); use platform NL for ws.send commands; broaden prompt regex to match cmd.exe prompt followed by ESC sequence
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
equestAnimationFrame\ to avoid redundant layout work.
Test plan
🤖 Generated with Claude Code