Support custom pagers and passphrase prompts on Windows#5740
Open
stefanhaller wants to merge 11 commits into
Open
Support custom pagers and passphrase prompts on Windows#5740stefanhaller wants to merge 11 commits into
stefanhaller wants to merge 11 commits into
Conversation
This was referenced Jun 30, 2026
The env var was previously set only on Windows, where the no-op pty stub was just running the command without a pty and needed to expose the width to pager scripts another way. With ConPTY coming to Windows the rationale disappears there, but the env var is documented in docs/Custom_Pagers.md for pager scripts that can't query the terminal width directly. Set it on every platform so those scripts remain portable, regardless of whether a pty is in play. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Windows ConPTY can't attach a child process to a pseudoconsole via os/exec — Go's stdlib doesn't expose PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE (golang/go#62708). The ConPTY path has to call CreateProcess directly, so it can't hand an *exec.Cmd back to the task runner. Widen NewCmdTask to accept a small Cmd interface satisfied by both *exec.Cmd (via the ExecCmd adapter) and the Windows ConPTY command type we're about to add. Change TerminateProcessGracefully to take *os.Process, which both cmd shapes can provide. Behavior is unchanged on every platform. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move the pty master behind a small interface (Read/Write/Close/Resize), and push the actual startup into a platform-specific StartPty function in pkg/commands/oscommands. The Unix implementation still uses creack/pty; the Windows implementation is a stub that returns ErrPtyUnsupported, at which point newPtyTask falls back to a plain cmd task — matching the existing Windows behavior. The primitive lives in oscommands rather than pkg/gui because the cmd_obj_runner pty handler (also in oscommands) is going to consume it too, and tasks → oscommands is the existing dependency direction. Same observable behavior on every platform; this just carves out a seam for a real ConPTY implementation on Windows. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Replace the StartPty stub with a real ConPTY implementation: CreatePipe + CreatePseudoConsole + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE + CreateProcess. Pagers and external diff tools now get real terminal behavior instead of being handed pipes. One Windows-specific quirk worth flagging: ConPTY does not EOF the output pipe when the child exits; conhost keeps it alive until ClosePseudoConsole is called explicitly. A background waiter goroutine calls ClosePseudoConsole as soon as proc.Wait returns, so callers see EOF on outRead — restoring the Unix master-fd-EOFs-when-slave-closes semantics they depend on. The ErrPtyUnsupported sentinel and the no-pty fallback in newPtyTask are gone now that both platforms have a real implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The per-platform getCmdHandlerPty split existed because the Unix side had creack/pty and the Windows side had nothing — so it fell back to a non-pty handler. Now that oscommands.StartPty provides a pty on both platforms, the two files collapse into one cross-platform implementation and the stub is gone. cmdHandler grows a 'wait' field because the pty path on Windows spawns via CreateProcess and never runs exec.Cmd.Start — so cmd.Wait wouldn't work there. Non-pty handlers set wait = cmd.Wait; pty handlers set it to the wait closure StartPty returns. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ConPTY presents its child's stdout as a screen buffer and uses CUP (`\x1b[<row>;<col>H`) to skip over blank rows rather than emitting LFs for them. Our escape interpreter swallows CUP via the catch-all "valid CSI final byte we don't implement" branch, so the blank rows the child put between non-blank ones disappear and the surrounding lines collapse together — which is what makes the delta-rendered diff in the screenshot look like its blank lines and section breaks were removed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ConPTY presents its child's output as a screen buffer and uses CUP / CUD / CNL / VPA to skip over blank rows rather than emitting LFs. The previous behaviour swallowed all of those and the visible content collapsed together. Now the escape parser tracks the screen-relative cursor row, and any CSI that moves the cursor past the current row emits a cursorDown instruction that the view turns into the matching number of empty lines. Column tracking is deliberately omitted: doing it correctly would mean duplicating the view's grapheme-cluster width math in the parser, and ConPTY in practice positions to column 1 after a CR-equivalent, which the existing wx-reset path already handles. ConPTY-internal scrolling needs no special handling either: it only emits cursor-positioning escapes within the first, un-scrolled screenful — once its screen scrolls it switches to plain linefeeds, which the view advances on directly regardless of the tracked cursor. Backward cursor moves are silently dropped — the view's buffer is append-style and can't undo earlier writes. The exception is cursor-home (CUP to row 1): ConPTY emits it at the start of every screen, so rather than drop it we re-anchor the row tracking to the current write position. Without that, a view not rewound in lockstep with ConPTY's screen (the command log, which streams pty output without a rewind) accumulates drift, and every later absolute CUP becomes a dropped backward move that collapses the rows ConPTY positioned with. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ConPTY compresses runs of default-colored spaces into ECH + CUF (\x1b[NX\x1b[NC) rather than emitting them literally. Both currently fall through the parser's swallow path, so the gap they describe collapses entirely and content that the child wrote with leading indentation ends up slid left against the previous cell. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ConPTY compresses runs of default-colored spaces into ECH + CUF (\x1b[NX\x1b[NC) instead of emitting them literally. ECH is still a no-op for us — our buffer is built sequentially and has nothing to erase — but CUF has to materialize as N visible space cells so the gap actually appears, otherwise content the child wrote with leading indentation slides left against the preceding cell. The view's cursorForward branch reuses the same machinery as tab expansion: substitute the trigger byte for a space and let the repeatCount path emit the cells under the parser-tracked SGR. The existing notifyCellsWritten plumbing then advances screenCol over the gap, keeping subsequent CUP targets aligned. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
1 participant
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.
Custom pagers were not available on Windows because the feature depends on a working PTY implementation, and the PTY library we are using doesn't have support for Windows (see creack/pty#155). This PR adds this support on our side; the same PTY library is still used on Mac and Linux, but on Windows we now have our own implementation using ConPTY.
This also enables prompting for SSH passphrases for users who don't have an ssh-agent running, which previously also didn't work on Windows. There's one limitation here: pressing
fin the Files panel meansfetch --allby default (unless you turn that off usinggit.fetchAll: false), in which case passphrase prompting still doesn't work; the reason is that git spawns a bunch of child processes for each remote to be fetched, and on Windows these don't inherit the PTY like they do on Mac and Linux.Fixes #1453
Fixes #5525