From 3c04b30336ecf52c7767e4b5f6c569e739019872 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 01/11] Have "just lint" show all issues instead of just the first so many --- .golangci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index c46e438acff..5ed7fb32d46 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,10 @@ version: "2" run: go: "1.25" +issues: + max-issues-per-linter: 0 + max-same-issues: 0 + uniq-by-line: false linters: enable: - copyloopvar From 51c8f9e6add7cc90c23816f1654d6aa58d936d21 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 02/11] Always set LAZYGIT_COLUMNS 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) --- pkg/gui/pty.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index f6356b9c0a4..fbe8d5d381a 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -3,6 +3,7 @@ package gui import ( + "fmt" "io" "os" "os/exec" @@ -45,6 +46,12 @@ func (gui *Gui) onResize() error { // command. func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error { width := view.InnerWidth() + + // LAZYGIT_COLUMNS is documented in docs/Custom_Pagers.md for pager + // scripts that can't query the terminal width directly. We set it on + // every platform so those scripts remain portable. + cmd.Env = append(cmd.Env, fmt.Sprintf("LAZYGIT_COLUMNS=%d", width)) + pager := gui.stateAccessor.GetPagerConfig().GetPagerCommand(width) externalDiffCommand := gui.stateAccessor.GetPagerConfig().GetExternalDiffCommand() useExtDiffGitConfig := gui.stateAccessor.GetPagerConfig().GetUseExternalDiffGitConfig() From 8ec4283e5551dc653f01be296020e03aeef8318d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 03/11] Abstract task command over *exec.Cmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../oscommands/os_default_platform.go | 6 ++-- pkg/commands/oscommands/os_windows.go | 2 +- pkg/gui/pty.go | 5 ++-- pkg/gui/tasks_adapter.go | 4 +-- pkg/tasks/tasks.go | 28 ++++++++++++++++--- pkg/tasks/tasks_test.go | 12 ++++---- 6 files changed, 39 insertions(+), 18 deletions(-) diff --git a/pkg/commands/oscommands/os_default_platform.go b/pkg/commands/oscommands/os_default_platform.go index 5e73c994f30..09f9f1d6d39 100644 --- a/pkg/commands/oscommands/os_default_platform.go +++ b/pkg/commands/oscommands/os_default_platform.go @@ -45,10 +45,10 @@ func (c *OSCommand) UpdateWindowTitle() error { // this call is reached on every host; only the Windows build does anything. func setRawCmdLine(cmd *exec.Cmd, cmdLine string) {} -func TerminateProcessGracefully(cmd *exec.Cmd) error { - if cmd.Process == nil { +func TerminateProcessGracefully(proc *os.Process) error { + if proc == nil { return nil } - return cmd.Process.Signal(syscall.SIGTERM) + return proc.Signal(syscall.SIGTERM) } diff --git a/pkg/commands/oscommands/os_windows.go b/pkg/commands/oscommands/os_windows.go index bd4cc515190..d0252f5b1b5 100644 --- a/pkg/commands/oscommands/os_windows.go +++ b/pkg/commands/oscommands/os_windows.go @@ -41,7 +41,7 @@ func (c *OSCommand) UpdateWindowTitle() error { return c.Cmd.NewShell(argString, c.UserConfig().OS.ShellFunctionsFile).Run() } -func TerminateProcessGracefully(cmd *exec.Cmd) error { +func TerminateProcessGracefully(proc *os.Process) error { // Signals other than SIGKILL are not supported on Windows return nil } diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index fbe8d5d381a..0455c0abf66 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -11,6 +11,7 @@ import ( "github.com/creack/pty" "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -82,7 +83,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error manager := gui.getManager(view) var ptmx *os.File - start := func() (*exec.Cmd, io.Reader) { + start := func() (tasks.Cmd, io.Reader) { var err error ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view)) if err != nil { @@ -93,7 +94,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error gui.viewPtmxMap[view.Name()] = ptmx gui.Mutexes.PtyMutex.Unlock() - return cmd, ptmx + return tasks.ExecCmd{Cmd: cmd}, ptmx } onClose := func() { diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 3bfc6410002..09edd2d36bb 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -19,7 +19,7 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error manager := gui.getManager(view) var r io.ReadCloser - start := func() (*exec.Cmd, io.Reader) { + start := func() (tasks.Cmd, io.Reader) { var err error r, err = cmd.StdoutPipe() if err != nil { @@ -32,7 +32,7 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error gui.c.Log.Error(err) } - return cmd, r + return tasks.ExecCmd{Cmd: cmd}, r } onClose := func() { diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 7fadbb4513f..26145c784ae 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -4,6 +4,7 @@ import ( "bufio" "fmt" "io" + "os" "os/exec" "sync" "time" @@ -15,6 +16,25 @@ import ( "github.com/sirupsen/logrus" ) +// Cmd abstracts over a started external process. *exec.Cmd satisfies the bulk +// of it via ExecCmd, but pty implementations can supply their own types — on +// Windows, ConPTY has to spawn via CreateProcess directly and can't use +// *exec.Cmd (see golang/go#62708). +type Cmd interface { + Wait() error + String() string + GetProcess() *os.Process +} + +// ExecCmd adapts *exec.Cmd to Cmd. +type ExecCmd struct { + *exec.Cmd +} + +func (c ExecCmd) GetProcess() *os.Process { + return c.Process +} + // This file revolves around running commands that will be output to the main panel // in the gui. If we're flicking through the commits panel, we want to invoke a // `git show` command for each commit, but we don't want to read the entire output @@ -117,7 +137,7 @@ func (self *ViewBufferManager) ReadToEnd(then func()) { } } -func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { +func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix string, linesToRead LinesToRead, onDoneFn func()) func(TaskOpts) error { return func(opts TaskOpts) error { var onDoneOnce sync.Once var onFirstPageShownOnce sync.Once @@ -173,8 +193,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p // // Unfortunately this will do nothing on Windows, so Windows users will have to live // with the higher CPU usage. - if err := oscommands.TerminateProcessGracefully(cmd); err != nil { - self.Log.Errorf("error when trying to terminate cmd task: %v; Command: %v %v", err, cmd.Path, cmd.Args) + if err := oscommands.TerminateProcessGracefully(cmd.GetProcess()); err != nil { + self.Log.Errorf("error when trying to terminate cmd task: %v; Command: %v", err, cmd.String()) } // close the task's stdout pipe (or the pty if we're using one) to make the command terminate @@ -338,7 +358,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p go func() { _ = cmd.Wait() }() default: if err := cmd.Wait(); err != nil { - self.Log.Errorf("Unexpected error when running cmd task: %v; Failed command: %v %v", err, cmd.Path, cmd.Args) + self.Log.Errorf("Unexpected error when running cmd task: %v; Failed command: %v", err, cmd.String()) } } diff --git a/pkg/tasks/tasks_test.go b/pkg/tasks/tasks_test.go index 40ff0033d6e..c025e8e16dc 100644 --- a/pkg/tasks/tasks_test.go +++ b/pkg/tasks/tasks_test.go @@ -43,13 +43,13 @@ func TestNewCmdTaskInstantStop(t *testing.T) { stop := make(chan struct{}) reader := bytes.NewBufferString("test") - start := func() (*exec.Cmd, io.Reader) { + start := func() (Cmd, io.Reader) { // not actually starting this because it's not necessary cmd := exec.Command("blah") close(stop) - return cmd, reader + return ExecCmd{Cmd: cmd}, reader } fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone) @@ -108,11 +108,11 @@ func TestNewCmdTask(t *testing.T) { stop := make(chan struct{}) reader := bytes.NewBufferString("test") - start := func() (*exec.Cmd, io.Reader) { + start := func() (Cmd, io.Reader) { // not actually starting this because it's not necessary cmd := exec.Command("blah") - return cmd, reader + return ExecCmd{Cmd: cmd}, reader } fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone) @@ -241,11 +241,11 @@ func TestNewCmdTaskRefresh(t *testing.T) { stop := make(chan struct{}) reader := BlankLineReader{totalLinesToYield: s.totalTaskLines} - start := func() (*exec.Cmd, io.Reader) { + start := func() (Cmd, io.Reader) { // not actually starting this because it's not necessary cmd := exec.Command("blah") - return cmd, &reader + return ExecCmd{Cmd: cmd}, &reader } fn := manager.NewCmdTask(start, "", s.linesToRead, func() {}) From c85f7530bb1617c8a16245636a224a432709f8f7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 04/11] Abstract pty startup behind a platform-specific primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/commands/oscommands/pty.go | 39 +++++++++++++++++++++ pkg/commands/oscommands/pty_unix.go | 34 ++++++++++++++++++ pkg/commands/oscommands/pty_windows.go | 12 +++++++ pkg/gui/gui.go | 4 +-- pkg/gui/pty.go | 48 ++++++++++++++++++-------- pkg/gui/pty_windows.go | 17 --------- 6 files changed, 120 insertions(+), 34 deletions(-) create mode 100644 pkg/commands/oscommands/pty.go create mode 100644 pkg/commands/oscommands/pty_unix.go create mode 100644 pkg/commands/oscommands/pty_windows.go delete mode 100644 pkg/gui/pty_windows.go diff --git a/pkg/commands/oscommands/pty.go b/pkg/commands/oscommands/pty.go new file mode 100644 index 00000000000..8a803fe60ce --- /dev/null +++ b/pkg/commands/oscommands/pty.go @@ -0,0 +1,39 @@ +package oscommands + +import ( + "errors" + "io" + "os" +) + +// Pty is the master side of a pseudo-terminal running a subprocess. The +// concrete implementation is platform-specific: creack/pty on Unix and +// ConPTY on Windows. +type Pty interface { + io.ReadWriteCloser + Resize(cols, rows uint16) error +} + +// StartedPty is the result of StartPty. +type StartedPty struct { + // Pty is the master side of the pseudo-terminal; read from it to get + // the child's combined stdout/stderr and write to it to feed stdin. + Pty Pty + // Process is the spawned child. Useful for signalling; on Windows the + // original *exec.Cmd was not Start()ed (ConPTY spawns via + // CreateProcess, not os/exec) so cmd.Process is nil and this is the + // only handle. + Process *os.Process + // Wait blocks until the child exits and returns a non-nil error on a + // nonzero exit status, matching *exec.Cmd.Wait semantics. + Wait func() error +} + +// ErrPtyUnsupported is returned by StartPty on platforms without a pty +// implementation. Callers may fall back to running the command without a pty. +var ErrPtyUnsupported = errors.New("pty not supported on this platform") + +// StartPty runs cmd in a pseudo-terminal with the given initial dimensions. +// Implemented per-platform in pty_unix.go / pty_windows.go. +// +// func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) diff --git a/pkg/commands/oscommands/pty_unix.go b/pkg/commands/oscommands/pty_unix.go new file mode 100644 index 00000000000..6cf63cdff53 --- /dev/null +++ b/pkg/commands/oscommands/pty_unix.go @@ -0,0 +1,34 @@ +//go:build !windows + +package oscommands + +import ( + "os" + "os/exec" + + creackpty "github.com/creack/pty" +) + +type unixPty struct { + master *os.File +} + +func (u *unixPty) Read(p []byte) (int, error) { return u.master.Read(p) } +func (u *unixPty) Write(p []byte) (int, error) { return u.master.Write(p) } +func (u *unixPty) Close() error { return u.master.Close() } + +func (u *unixPty) Resize(cols, rows uint16) error { + return creackpty.Setsize(u.master, &creackpty.Winsize{Cols: cols, Rows: rows}) +} + +func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) { + f, err := creackpty.StartWithSize(cmd, &creackpty.Winsize{Cols: cols, Rows: rows}) + if err != nil { + return StartedPty{}, err + } + return StartedPty{ + Pty: &unixPty{master: f}, + Process: cmd.Process, + Wait: cmd.Wait, + }, nil +} diff --git a/pkg/commands/oscommands/pty_windows.go b/pkg/commands/oscommands/pty_windows.go new file mode 100644 index 00000000000..6d0d7c6ffbd --- /dev/null +++ b/pkg/commands/oscommands/pty_windows.go @@ -0,0 +1,12 @@ +package oscommands + +import ( + "os/exec" +) + +// StartPty is a stub on Windows for now; callers fall back to the non-pty +// path when ErrPtyUnsupported is returned. A real ConPTY implementation +// replaces this in a follow-up commit. +func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) { + return StartedPty{}, ErrPtyUnsupported +} diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 08d560ce531..e23afd12430 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -85,7 +85,7 @@ type Gui struct { // holds a mapping of view names to ptmx's. This is for rendering command outputs // from within a pty. The point of keeping track of them is so that if we re-size // the window, we can tell the pty it needs to resize accordingly. - viewPtmxMap map[string]*os.File + viewPtmxMap map[string]oscommands.Pty stopChan chan struct{} // when lazygit is opened outside a git directory we want to open to the most @@ -761,7 +761,7 @@ func NewGui( Updater: updater, statusManager: status.NewStatusManager(), viewBufferManagerMap: map[string]*tasks.ViewBufferManager{}, - viewPtmxMap: map[string]*os.File{}, + viewPtmxMap: map[string]oscommands.Pty{}, showRecentRepos: showRecentRepos, RepoPathStack: &utils.StringStack{}, RepoStateMap: map[Repo]*GuiRepoState{}, diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 0455c0abf66..e5ef2a770db 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -1,37 +1,36 @@ -//go:build !windows - package gui import ( + "errors" "fmt" "io" "os" "os/exec" "strings" - "github.com/creack/pty" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) -func (gui *Gui) desiredPtySize(view *gocui.View) *pty.Winsize { +func (gui *Gui) desiredPtySize(view *gocui.View) (cols, rows uint16) { width, height := view.InnerSize() - - return &pty.Winsize{Cols: uint16(width), Rows: uint16(height)} + return uint16(width), uint16(height) } func (gui *Gui) onResize() error { gui.Mutexes.PtyMutex.Lock() defer gui.Mutexes.PtyMutex.Unlock() - for viewName, ptmx := range gui.viewPtmxMap { + for viewName, p := range gui.viewPtmxMap { // TODO: handle resizing properly: we need to actually clear the main view // and re-read the output from our pty. Or we could just re-run the original // command from scratch view, _ := gui.g.View(viewName) - if err := pty.Setsize(ptmx, gui.desiredPtySize(view)); err != nil { + cols, rows := gui.desiredPtySize(view) + if err := p.Resize(cols, rows); err != nil { return utils.WrapError(err) } } @@ -39,6 +38,19 @@ func (gui *Gui) onResize() error { return nil } +// ptyCmd adapts an oscommands.StartedPty result into the tasks.Cmd shape. +// On Windows the original *exec.Cmd was never Start()ed, so we go through +// the explicit Process handle rather than cmd.Process. +type ptyCmd struct { + cmd *exec.Cmd + process *os.Process + wait func() error +} + +func (p ptyCmd) Wait() error { return p.wait() } +func (p ptyCmd) String() string { return p.cmd.String() } +func (p ptyCmd) GetProcess() *os.Process { return p.process } + // Some commands need to output for a terminal to active certain behaviour. // For example, git won't invoke the GIT_PAGER env var unless it thinks it's // talking to a terminal. We typically write cmd outputs straight to a view, @@ -82,24 +94,30 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error manager := gui.getManager(view) - var ptmx *os.File + var p oscommands.Pty start := func() (tasks.Cmd, io.Reader) { - var err error - ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view)) + cols, rows := gui.desiredPtySize(view) + sp, err := oscommands.StartPty(cmd, cols, rows) if err != nil { - gui.c.Log.Error(err) + if !errors.Is(err, oscommands.ErrPtyUnsupported) { + gui.c.Log.Error(err) + } + return tasks.ExecCmd{Cmd: cmd}, nil } + p = sp.Pty gui.Mutexes.PtyMutex.Lock() - gui.viewPtmxMap[view.Name()] = ptmx + gui.viewPtmxMap[view.Name()] = p gui.Mutexes.PtyMutex.Unlock() - return tasks.ExecCmd{Cmd: cmd}, ptmx + return ptyCmd{cmd: cmd, process: sp.Process, wait: sp.Wait}, p } onClose := func() { gui.Mutexes.PtyMutex.Lock() - ptmx.Close() + if p != nil { + p.Close() + } delete(gui.viewPtmxMap, view.Name()) gui.Mutexes.PtyMutex.Unlock() } diff --git a/pkg/gui/pty_windows.go b/pkg/gui/pty_windows.go deleted file mode 100644 index 31d76387053..00000000000 --- a/pkg/gui/pty_windows.go +++ /dev/null @@ -1,17 +0,0 @@ -package gui - -import ( - "fmt" - "os/exec" - - "github.com/jesseduffield/lazygit/pkg/gocui" -) - -func (gui *Gui) onResize() error { - return nil -} - -func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error { - cmd.Env = append(cmd.Env, fmt.Sprintf("LAZYGIT_COLUMNS=%d", view.InnerWidth())) - return gui.newCmdTask(view, cmd, prefix) -} From 1935117141b98d7931fc04731bceff8e4baf83a5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 05/11] Add pty support on Windows via ConPTY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/commands/oscommands/pty.go | 5 - pkg/commands/oscommands/pty_windows.go | 257 ++++++++++++++++++++++++- pkg/gui/pty.go | 5 +- 3 files changed, 253 insertions(+), 14 deletions(-) diff --git a/pkg/commands/oscommands/pty.go b/pkg/commands/oscommands/pty.go index 8a803fe60ce..a4a36963333 100644 --- a/pkg/commands/oscommands/pty.go +++ b/pkg/commands/oscommands/pty.go @@ -1,7 +1,6 @@ package oscommands import ( - "errors" "io" "os" ) @@ -29,10 +28,6 @@ type StartedPty struct { Wait func() error } -// ErrPtyUnsupported is returned by StartPty on platforms without a pty -// implementation. Callers may fall back to running the command without a pty. -var ErrPtyUnsupported = errors.New("pty not supported on this platform") - // StartPty runs cmd in a pseudo-terminal with the given initial dimensions. // Implemented per-platform in pty_unix.go / pty_windows.go. // diff --git a/pkg/commands/oscommands/pty_windows.go b/pkg/commands/oscommands/pty_windows.go index 6d0d7c6ffbd..0a4a0647763 100644 --- a/pkg/commands/oscommands/pty_windows.go +++ b/pkg/commands/oscommands/pty_windows.go @@ -1,12 +1,259 @@ package oscommands import ( + "fmt" + "os" "os/exec" + "sync" + "unsafe" + + "golang.org/x/sys/windows" ) -// StartPty is a stub on Windows for now; callers fall back to the non-pty -// path when ErrPtyUnsupported is returned. A real ConPTY implementation -// replaces this in a follow-up commit. -func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) { - return StartedPty{}, ErrPtyUnsupported +type winPty struct { + hpc windows.Handle + inWrite *os.File + outRead *os.File + + // mu guards the teardown state below and serializes it against Resize. + // hpcClosed gates ClosePseudoConsole (it must run exactly once) and also + // keeps Resize from touching the HPCON once it's been freed: the + // background waiter in StartPty closes the pseudoconsole on child exit, + // which would otherwise race a concurrent onResize and hand + // ResizePseudoConsole a freed handle. + mu sync.Mutex + hpcClosed bool + closed bool +} + +func (p *winPty) Read(buf []byte) (int, error) { return p.outRead.Read(buf) } +func (p *winPty) Write(buf []byte) (int, error) { return p.inWrite.Write(buf) } + +func (p *winPty) Resize(cols, rows uint16) error { + p.mu.Lock() + defer p.mu.Unlock() + if p.hpcClosed { + // The child already exited and the pseudoconsole was torn down, so + // there is nothing left to resize. + return nil + } + return windows.ResizePseudoConsole(p.hpc, windows.Coord{X: int16(cols), Y: int16(rows)}) +} + +// closeHpc closes the pseudoconsole exactly once. Safe to call from multiple +// goroutines and at any time. We need this separately from Close because the +// background waiter in StartPty closes the pseudoconsole as soon as the child +// exits — that's what makes outRead return EOF, matching the Unix behavior +// where the master fd EOFs when the slave closes — while the pipe fds stay +// open until somebody explicitly tears the pty down. +func (p *winPty) closeHpc() { + p.mu.Lock() + defer p.mu.Unlock() + p.closeHpcLocked() +} + +// closeHpcLocked closes the pseudoconsole; the caller must hold p.mu. +func (p *winPty) closeHpcLocked() { + if p.hpcClosed { + return + } + p.hpcClosed = true + windows.ClosePseudoConsole(p.hpc) +} + +func (p *winPty) Close() error { + p.mu.Lock() + defer p.mu.Unlock() + if p.closed { + return nil + } + p.closed = true + // Closing the pseudoconsole breaks the pipes; the child's next write + // fails and it exits. Then we close our ends of the pipes. + p.closeHpcLocked() + p.inWrite.Close() + p.outRead.Close() + return nil +} + +// startWaiter runs proc.Wait in a goroutine and, as soon as the child exits, +// closes the pseudoconsole so that any pending Read on outRead returns EOF +// after buffered output drains. Returns a Wait func that blocks until the +// child has exited and reports its exit status with *exec.Cmd.Wait semantics. +// +// This shape exists because on Unix the master fd EOFs naturally when the +// slave closes on child exit, but ConPTY keeps the pipe alive until we call +// ClosePseudoConsole explicitly. Without doing that on child exit, the +// scanner in pkg/tasks.NewCmdTask would block forever on the next read and +// the post-content view never gets cleared (FlushStaleCells never fires). +func startWaiter(proc *os.Process, p *winPty) func() error { + done := make(chan struct{}) + var waitErr error + go func() { + defer close(done) + state, err := proc.Wait() + p.closeHpc() + if err != nil { + waitErr = err + return + } + if !state.Success() { + waitErr = fmt.Errorf("exit status %d", state.ExitCode()) + } + }() + return func() error { + <-done + return waitErr + } +} + +func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) { + // Two pipes: one for the child's stdin (we never write to it, but ConPTY + // needs a handle), one for the child's stdout/stderr multiplexed through + // the pseudoconsole. + var inRead, inWrite, outRead, outWrite windows.Handle + if err = windows.CreatePipe(&inRead, &inWrite, nil, 0); err != nil { + return StartedPty{}, fmt.Errorf("CreatePipe (in): %w", err) + } + defer func() { + if err != nil { + _ = windows.CloseHandle(inWrite) + } + }() + if err = windows.CreatePipe(&outRead, &outWrite, nil, 0); err != nil { + _ = windows.CloseHandle(inRead) + return StartedPty{}, fmt.Errorf("CreatePipe (out): %w", err) + } + defer func() { + if err != nil { + _ = windows.CloseHandle(outRead) + } + }() + + // CreatePseudoConsole dupes the handles it needs internally; we release + // our references to the child-side ends immediately after. + var hpc windows.Handle + size := windows.Coord{X: int16(cols), Y: int16(rows)} + if err = windows.CreatePseudoConsole(size, inRead, outWrite, 0, &hpc); err != nil { + _ = windows.CloseHandle(inRead) + _ = windows.CloseHandle(outWrite) + return StartedPty{}, fmt.Errorf("CreatePseudoConsole: %w", err) + } + _ = windows.CloseHandle(inRead) + _ = windows.CloseHandle(outWrite) + defer func() { + if err != nil { + windows.ClosePseudoConsole(hpc) + } + }() + + // Attach the pseudoconsole to the child via a process attribute list. + attrList, err := windows.NewProcThreadAttributeList(1) + if err != nil { + return StartedPty{}, fmt.Errorf("NewProcThreadAttributeList: %w", err) + } + defer attrList.Delete() + // PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE wants the HPCON value itself as + // the attribute value, not a pointer to it — an HPCON is already a + // pointer-sized handle, per Microsoft's ConPTY sample. Spelling that as + // unsafe.Pointer(hpc) trips go vet's unsafeptr check (a uintptr-based + // type converted straight to unsafe.Pointer), which gopls surfaces in + // the editor. Reinterpret the handle's bits through its address instead: + // &hpc is a real pointer, so none of these conversions is the flagged + // uintptr→unsafe.Pointer cast, while the resulting value is identical. + if err = attrList.Update( + windows.PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + *(*unsafe.Pointer)(unsafe.Pointer(&hpc)), + unsafe.Sizeof(hpc), + ); err != nil { + return StartedPty{}, fmt.Errorf("UpdateProcThreadAttribute: %w", err) + } + + var si windows.StartupInfoEx + si.Cb = uint32(unsafe.Sizeof(si)) + si.ProcThreadAttributeList = attrList.List() + + var appNamePtr *uint16 + if cmd.Path != "" { + if appNamePtr, err = windows.UTF16PtrFromString(cmd.Path); err != nil { + return StartedPty{}, err + } + } + cmdLinePtr, err := windows.UTF16PtrFromString(windows.ComposeCommandLine(cmd.Args)) + if err != nil { + return StartedPty{}, err + } + var dirPtr *uint16 + if cmd.Dir != "" { + if dirPtr, err = windows.UTF16PtrFromString(cmd.Dir); err != nil { + return StartedPty{}, err + } + } + envBlock, err := createEnvBlock(cmd.Env) + if err != nil { + return StartedPty{}, err + } + var envPtr *uint16 + if envBlock != nil { + envPtr = &envBlock[0] + } + + var pi windows.ProcessInformation + err = windows.CreateProcess( + appNamePtr, + cmdLinePtr, + nil, // process security + nil, // thread security + false, + windows.EXTENDED_STARTUPINFO_PRESENT|windows.CREATE_UNICODE_ENVIRONMENT, + envPtr, + dirPtr, + &si.StartupInfo, + &pi, + ) + if err != nil { + return StartedPty{}, fmt.Errorf("CreateProcess: %w", err) + } + _ = windows.CloseHandle(pi.Thread) + + // Re-open the process by PID to get an *os.Process to wait on. Do this + // while pi.Process is still open: Windows won't recycle a PID while any + // handle to the process remains, so FindProcess can't latch onto a + // different process that has since reused the PID. Release the original + // handle once we have our own. + proc, err := os.FindProcess(int(pi.ProcessId)) + _ = windows.CloseHandle(pi.Process) + if err != nil { + return StartedPty{}, err + } + + wp := &winPty{ + hpc: hpc, + inWrite: os.NewFile(uintptr(inWrite), "conpty-in"), + outRead: os.NewFile(uintptr(outRead), "conpty-out"), + } + return StartedPty{ + Pty: wp, + Process: proc, + Wait: startWaiter(proc, wp), + }, nil +} + +// createEnvBlock packs env vars into the UTF-16 double-null-terminated block +// that CreateProcess expects. Returns nil if env is empty, which tells +// CreateProcess to inherit the parent's environment. +func createEnvBlock(env []string) ([]uint16, error) { + if len(env) == 0 { + return nil, nil + } + var block []uint16 + for _, s := range env { + utf16s, err := windows.UTF16FromString(s) + if err != nil { + return nil, err + } + block = append(block, utf16s...) + } + block = append(block, 0) + return block, nil } diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index e5ef2a770db..1a774fc3d8e 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -1,7 +1,6 @@ package gui import ( - "errors" "fmt" "io" "os" @@ -99,9 +98,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error cols, rows := gui.desiredPtySize(view) sp, err := oscommands.StartPty(cmd, cols, rows) if err != nil { - if !errors.Is(err, oscommands.ErrPtyUnsupported) { - gui.c.Log.Error(err) - } + gui.c.Log.Error(err) return tasks.ExecCmd{Cmd: cmd}, nil } p = sp.Pty From 8c035ebe60541722e9b28c782f8d7bee592d16eb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 06/11] Use ConPTY on Windows for pty-backed command execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/commands/oscommands/cmd_obj.go | 2 +- pkg/commands/oscommands/cmd_obj_runner.go | 26 ++++++++++++++++--- .../oscommands/cmd_obj_runner_default.go | 24 ----------------- .../oscommands/cmd_obj_runner_windows.go | 10 ------- 4 files changed, 23 insertions(+), 39 deletions(-) delete mode 100644 pkg/commands/oscommands/cmd_obj_runner_default.go delete mode 100644 pkg/commands/oscommands/cmd_obj_runner_windows.go diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go index 57bc295ed57..1fbe8408746 100644 --- a/pkg/commands/oscommands/cmd_obj.go +++ b/pkg/commands/oscommands/cmd_obj.go @@ -157,7 +157,7 @@ func (self *CmdObj) ShouldStreamOutput() bool { } // when you call this, then call Run(), we'll use a PTY to run the command. Only -// has an effect if StreamOutput() was also called. Ignored on Windows. +// has an effect if StreamOutput() was also called. func (self *CmdObj) UsePty() *CmdObj { self.usePty = true diff --git a/pkg/commands/oscommands/cmd_obj_runner.go b/pkg/commands/oscommands/cmd_obj_runner.go index 9786826189d..b70668431f7 100644 --- a/pkg/commands/oscommands/cmd_obj_runner.go +++ b/pkg/commands/oscommands/cmd_obj_runner.go @@ -219,6 +219,10 @@ type cmdHandler struct { stdoutPipe io.Reader stdinPipe io.Writer close func() error + // wait blocks until the child process exits. Needed as a separate + // field because the pty path on Windows spawns via CreateProcess and + // never runs *exec.Cmd.Start — so cmd.Wait wouldn't work there. + wait func() error } func (self *cmdObjRunner) runAndStream(cmdObj *CmdObj) error { @@ -274,7 +278,7 @@ func (self *cmdObjRunner) runAndStreamAux( onRun(handler, cmdWriter) - err = cmd.Wait() + err = handler.wait() self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) @@ -376,9 +380,7 @@ func (self *cmdObjRunner) processOutput( responseChan := promptUserForCredential(askFor) if responseChan == nil { // Returning a nil channel means we should terminate the process. - // We achieve this by closing the pty that it's running in. Note that this won't - // work for the case where we're not running in a pty (i.e. on Windows), but - // in that case we'll never be prompted for credentials, so it's not a concern. + // We achieve this by closing the pty that it's running in. if err := closeFunc(); err != nil { self.log.Error(err) } @@ -481,5 +483,21 @@ func (self *cmdObjRunner) getCmdHandlerNonPty(cmd *exec.Cmd) (*cmdHandler, error stdoutPipe: stdoutReader, stdinPipe: buf, close: func() error { return nil }, + wait: cmd.Wait, + }, nil +} + +func (self *cmdObjRunner) getCmdHandlerPty(cmd *exec.Cmd) (*cmdHandler, error) { + // Size will be adjusted by the caller if it cares; this just avoids a + // zero-size pty. + sp, err := StartPty(cmd, 80, 24) + if err != nil { + return nil, err + } + return &cmdHandler{ + stdoutPipe: sp.Pty, + stdinPipe: sp.Pty, + close: sp.Pty.Close, + wait: sp.Wait, }, nil } diff --git a/pkg/commands/oscommands/cmd_obj_runner_default.go b/pkg/commands/oscommands/cmd_obj_runner_default.go deleted file mode 100644 index 72cbc26c66c..00000000000 --- a/pkg/commands/oscommands/cmd_obj_runner_default.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build !windows - -package oscommands - -import ( - "os/exec" - - "github.com/creack/pty" -) - -// we define this separately for windows and non-windows given that windows does -// not have great PTY support and we need a PTY to handle a credential request -func (self *cmdObjRunner) getCmdHandlerPty(cmd *exec.Cmd) (*cmdHandler, error) { - ptmx, err := pty.Start(cmd) - if err != nil { - return nil, err - } - - return &cmdHandler{ - stdoutPipe: ptmx, - stdinPipe: ptmx, - close: ptmx.Close, - }, nil -} diff --git a/pkg/commands/oscommands/cmd_obj_runner_windows.go b/pkg/commands/oscommands/cmd_obj_runner_windows.go deleted file mode 100644 index f92e36c69e4..00000000000 --- a/pkg/commands/oscommands/cmd_obj_runner_windows.go +++ /dev/null @@ -1,10 +0,0 @@ -package oscommands - -import ( - "os/exec" -) - -func (self *cmdObjRunner) getCmdHandlerPty(cmd *exec.Cmd) (*cmdHandler, error) { - // We don't have PTY support on Windows yet, so we just return a non-PTY handler. - return self.getCmdHandlerNonPty(cmd) -} From 79bc8e0bc64f50028b3e0a159afb8d96ca65bf4a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 07/11] Demonstrate that cursor positioning escapes collapse blank rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConPTY presents its child's stdout as a screen buffer and uses CUP (`\x1b[;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) --- pkg/gocui/view_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index f65418821b3..58c83e126b5 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -238,6 +238,27 @@ func TestContainsColoredText(t *testing.T) { } } +func TestWriteCursorPositionEscape(t *testing.T) { + // ConPTY presents its child's output as a screen buffer and uses cursor + // positioning escapes (CUP, `\x1b[;H`) to skip over blank rows + // rather than emitting empty LFs for them. The escape interpreter must + // synthesize the row advances those CUPs imply; otherwise non-blank rows + // that the child separated with blank lines end up adjacent in the view. + v := NewView("name", 0, 0, 20, 10, OutputNormal) + // "a", then "skip to row 3" (i.e. one blank row), then "b". + v.writeString("a\r\n\x1b[3;1Hb\r\n") + + got := make([][]string, 0, len(v.lines)) + for _, l := range v.lines { + got = append(got, cellsToStrings(l.cells)) + } + + /* EXPECTED: + assert.Equal(t, [][]string{{"a"}, {}, {"b"}}, got) + ACTUAL: */ + assert.Equal(t, [][]string{{"a"}, {"b"}}, got) +} + func stringToCells(s string) []cell { var cells []cell state := -1 From 180fe0cd267ff52a9afe8b004fa6dbdaab352634 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 08/11] Convert forward cursor-positioning escapes into row advances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/gocui/escape.go | 139 ++++++++++++++++++++++++++++++++++++++- pkg/gocui/escape_test.go | 67 +++++++++++++++++-- pkg/gocui/view.go | 22 +++++++ pkg/gocui/view_test.go | 3 - 4 files changed, 221 insertions(+), 10 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index 726fd4de7aa..c252fa20bc2 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -19,6 +19,21 @@ type escapeInterpreter struct { mode OutputMode instruction instruction hyperlink strings.Builder + + // ConPTY emits cursor-positioning escapes (CUP) to skip over blank + // rows rather than emitting LFs for them. To convert those into row + // advances the view can act on, we track where in the pseudo-terminal + // screen the cursor currently is. 1-based to match the escape + // sequences. + // + // We also have to track the column, but only well enough to count + // soft-wraps when written content runs past the right edge: ConPTY's + // CUPs are addressed against its post-wrap screen, so a logical line + // long enough to wrap in ConPTY's screen counts for two rows from the + // next CUP's perspective. Column accuracy past wrap-counting isn't + // modelled — we don't track the col argument of CUPs, and most + // pager-style emitters use col 1 anyway. + screenRow, screenCol int } type ( @@ -32,6 +47,13 @@ type eraseInLineFromCursor struct{} func (self eraseInLineFromCursor) isInstruction() {} +// cursorDown asks the view to advance N rows. Emitted when CUP / CUD / +// CNL / VPA targets a row past the current one; backward moves are +// ignored because the view's buffer is line-based and can't undo. +type cursorDown struct{ n int } + +func (self cursorDown) isInstruction() {} + type noInstruction struct{} func (self noInstruction) isInstruction() {} @@ -99,11 +121,15 @@ func newEscapeInterpreter(mode OutputMode) *escapeInterpreter { curBgColor: ColorDefault, mode: mode, instruction: noInstruction{}, + screenRow: 1, + screenCol: 1, } return ei } -// reset sets the escapeInterpreter in initial state. +// reset sets the escapeInterpreter in initial state. Note: this only resets +// escape-parsing state. Screen cursor state survives so that mid-stream +// malformed escapes don't desync the row tracking from the view. func (ei *escapeInterpreter) reset() { ei.state = stateNone ei.curFgColor = ColorDefault @@ -111,6 +137,80 @@ func (ei *escapeInterpreter) reset() { ei.csiParam = nil } +// resetScreenCursor returns the screen-cursor tracking to the top of the +// pseudo-terminal screen. Called when the view is rewound before a fresh pty +// render, and on cursor-home (which ConPTY emits at the start of each screen) +// for views that aren't rewound in lockstep — see the CUP handling in parseOne. +func (ei *escapeInterpreter) resetScreenCursor() { + ei.screenRow = 1 + ei.screenCol = 1 +} + +// notifyRowAdvance must be called by the view whenever it advances to the +// next row in response to an LF / CRLF outside of an escape sequence +// (i.e. the row transitions the parser doesn't see directly). Keeps the +// parser's notion of the current screen row in sync with the view. +func (ei *escapeInterpreter) notifyRowAdvance() { + ei.screenRow++ + ei.screenCol = 1 +} + +// notifyColumnReset must be called when the view processes a bare CR +// (column reset without row advance). Keeps screenCol in sync so wrap +// counting starts over from col 1. +func (ei *escapeInterpreter) notifyColumnReset() { + ei.screenCol = 1 +} + +// notifyCellsWritten must be called after the view writes visible cells +// to its buffer. Advances the parser's idea of the cursor by `width` +// columns; if that crosses the right edge of a `screenColMax`-wide pty +// screen, the corresponding number of soft-wraps are added to screenRow +// so subsequent CUPs land on the right line. +func (ei *escapeInterpreter) notifyCellsWritten(width, screenColMax int) { + if screenColMax <= 0 { + return + } + // One column at a time: matches ConPTY's "pending wrap" semantics + // where the cursor stays at col max+1 after writing the rightmost + // cell and only wraps on the next cell. Loops over individual + // columns rather than doing the math in one shot so wide cells on a + // row boundary still wrap cleanly. + for range width { + if ei.screenCol > screenColMax { + ei.screenRow++ + ei.screenCol = 1 + } + ei.screenCol++ + } +} + +// emitCursorAdvance schedules a cursorDown instruction for the next time +// the view checks ei.instruction, advancing the parser's screen row by +// the same amount. n <= 0 is a no-op (backward / same-row CUPs are +// ignored — the view's buffer is line-based and can't undo). +func (ei *escapeInterpreter) emitCursorAdvance(n int) { + if n <= 0 { + return + } + ei.instruction = cursorDown{n: n} + ei.screenRow += n + ei.screenCol = 1 +} + +// firstParamOrDefault returns the first CSI parameter parsed as an int, +// or dflt if it's absent / empty / unparseable. +func (ei *escapeInterpreter) firstParamOrDefault(dflt int) int { + if len(ei.csiParam) == 0 || ei.csiParam[0] == "" { + return dflt + } + n, err := strconv.Atoi(ei.csiParam[0]) + if err != nil { + return dflt + } + return n +} + func (ei *escapeInterpreter) instructionRead() { ei.instruction = noInstruction{} } @@ -170,8 +270,12 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "") case characterEquals(ch, 'm'): ei.csiParam = append(ei.csiParam, "0") - case characterEquals(ch, 'K'): - // fall through + case characterEquals(ch, 'K'), + characterEquals(ch, 'H'), characterEquals(ch, 'f'), characterEquals(ch, 'd'), + characterEquals(ch, 'B'), characterEquals(ch, 'E'): + // fall through — let stateParams handle these with default + // params (CUP/VPA default to row 1, CUD/CNL default to advance + // by 1). case characterEquals(ch, ';'): // Empty first param ([;Xm ≡ [0;Xm). Seed a slot for the // empty param; stateParams will append the next one when it @@ -240,6 +344,35 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.instruction = noInstruction{} } + ei.state = stateNone + ei.csiParam = nil + return true, nil + case characterEquals(ch, 'H'), characterEquals(ch, 'f'), + characterEquals(ch, 'd'): + // CUP / HVP (absolute (row, col), col ignored) or VPA (absolute row). + targetRow := ei.firstParamOrDefault(1) + if targetRow <= 1 { + // Cursor home. ConPTY emits this (after [2J) at the start of + // every screen, so it marks where ConPTY's coordinate origin + // now sits. Re-anchor our row tracking to the current write + // position rather than treating it as a backward move: a view + // that isn't rewound in lockstep with ConPTY's screen (the + // command log) would otherwise carry stale drift, making every + // later absolute CUP compute a negative, dropped advance and + // collapsing the blank rows ConPTY positioned with. + ei.resetScreenCursor() + } else { + // Skip forward to the target row; ignore backward moves. + ei.emitCursorAdvance(targetRow - ei.screenRow) + } + ei.state = stateNone + ei.csiParam = nil + return true, nil + case characterEquals(ch, 'B'), characterEquals(ch, 'E'): + // CUD / CNL — relative row advance by N. CNL also resets + // the column, which we don't track, so the two are + // equivalent for our purposes. + ei.emitCursorAdvance(ei.firstParamOrDefault(1)) ei.state = stateNone ei.csiParam = nil return true, nil diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index cb8eb1a4b81..a7e8ce02ff9 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -153,17 +153,16 @@ func TestParseOneColours(t *testing.T) { func TestParseOneIgnoresUnknownSequences(t *testing.T) { // Escape sequences the interpreter doesn't implement -- whether well-formed-but-unsupported - // (cursor movement, private modes, DECSCUSR, …) or outright malformed -- must be silently + // (private modes, DECSCUSR, …) or outright malformed -- must be silently // consumed rather than leaked into the view as literal text. scenarios := []string{ "\x1b[?9001h", // DEC private-mode set (?-prefix) "\x1b[?25l", // hide cursor "\x1b[?25h", // show cursor "\x1b[2;J", // erase display (unusual 2;J variant) - "\x1b[H", // cursor home — final byte immediately after [ - "\x1b[5;1;H", // cursor position with multiple params + "\x1b[H", // cursor home — re-anchors to row 1 (no-op when already there) "\x1bc", // RIS — single-char ESC sequence - "\x1b[;5H", // empty first param (';' immediately after '[') + "\x1b[;5H", // empty first param — defaults to row 1, no-op "\x1b[ q", // intermediate byte with no params (DECSCUSR family) "\x1b[0 q", // intermediate byte after a param "\x1b[1;;m", // malformed SGR: empty middle param @@ -184,6 +183,66 @@ func TestParseOneIgnoresUnknownSequences(t *testing.T) { } } +func TestParseOneCursorPositioning(t *testing.T) { + // Cursor-positioning escapes that advance the row forward emit a + // cursorDown instruction; backward / same-row moves are ignored + // because the view's buffer is line-based. + scenarios := []struct { + input string + startRow int // parser's screenRow before parsing + wantAdvance int // 0 means "no instruction emitted" + }{ + {"\x1b[5;1H", 1, 4}, // CUP — absolute row 5 from row 1 + {"\x1b[5H", 1, 4}, // CUP with only the row param + {"\x1b[5;1H", 5, 0}, // CUP to the same row we're on — no-op + {"\x1b[2;1H", 5, 0}, // CUP backward — ignored + {"\x1b[5;1f", 1, 4}, // HVP alias for CUP + {"\x1b[5d", 1, 4}, // VPA — absolute row + {"\x1b[2d", 5, 0}, // VPA backward — ignored + {"\x1b[3B", 1, 3}, // CUD — relative + {"\x1b[B", 1, 1}, // CUD with default param of 1 + {"\x1b[2E", 1, 2}, // CNL — relative + } + + for _, s := range scenarios { + ei := newEscapeInterpreter(OutputNormal) + ei.screenRow = s.startRow + parseEscRunes(t, ei, s.input) + if s.wantAdvance == 0 { + _, noop := ei.instruction.(noInstruction) + assert.True(t, noop, "input %q at row %d should be a no-op", s.input, s.startRow) + } else { + cd, ok := ei.instruction.(cursorDown) + if assert.True(t, ok, "input %q at row %d should emit cursorDown", s.input, s.startRow) { + assert.Equal(t, s.wantAdvance, cd.n, "input %q at row %d", s.input, s.startRow) + } + } + } +} + +func TestParseOneCursorHomeReanchors(t *testing.T) { + // ConPTY emits cursor-home ([H) after [2J at the start of every screen. + // In a view that isn't rewound in lockstep with ConPTY (the command log) + // screenRow has drifted, so home must re-anchor it to the current write + // position rather than be dropped as a backward move — otherwise the + // absolute CUPs that follow compute negative, dropped advances and the + // rows ConPTY positioned with collapse together. + ei := newEscapeInterpreter(OutputNormal) + ei.screenRow = 12 // accumulated drift from earlier command-log output + + parseEscRunes(t, ei, "\x1b[H") + assert.Equal(t, 1, ei.screenRow, "home should re-anchor screenRow") + _, noop := ei.instruction.(noInstruction) + assert.True(t, noop, "home should not emit an instruction") + + // A subsequent CUP now advances relative to the re-anchored origin. + parseEscRunes(t, ei, "\x1b[3;1H") + cd, ok := ei.instruction.(cursorDown) + if assert.True(t, ok, "CUP after home should emit cursorDown") { + assert.Equal(t, 2, cd.n) + } +} + func parseEscRunes(t *testing.T, ei *escapeInterpreter, runes string) { t.Helper() for _, b := range []byte(runes) { diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 77492b9b108..1e67f853544 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -834,6 +834,7 @@ func (v *View) write(p []byte) { if v.pendingNewline { advanceToNextLine() + v.ei.notifyRowAdvance() v.pendingNewline = false } @@ -855,11 +856,20 @@ func (v *View) write(p []byte) { case characterEquals(chr, '\n') || isCRLF(chr): finishLine() advanceToNextLine() + v.ei.notifyRowAdvance() case characterEquals(chr, '\r'): finishLine() v.wx = 0 + v.ei.notifyColumnReset() default: truncateLine, cells := v.parseInput(chr, width, v.wx, v.wy) + if cd, ok := v.ei.instruction.(cursorDown); ok { + v.ei.instructionRead() + for range cd.n { + v.autoRenderHyperlinksInCurrentLine() + advanceToNextLine() + } + } if cells == nil { continue } @@ -867,6 +877,17 @@ func (v *View) write(p []byte) { if truncateLine { v.lines[v.wy].cells = v.lines[v.wy].cells[:v.wx] } + // Soft-wrap tracking. truncateLine is true exactly when the + // cells are from \x1b[K filling to end of line — ConPTY + // doesn't advance the cursor for that, so we shouldn't count + // it toward wraps either. + if !truncateLine { + totalWidth := 0 + for _, c := range cells { + totalWidth += c.width + } + v.ei.notifyCellsWritten(totalWidth, v.InnerWidth()) + } } } @@ -1116,6 +1137,7 @@ func (v *View) FlushStaleCells() { func (v *View) rewind() { v.ei.reset() + v.ei.resetScreenCursor() v.SetReadPos(0, 0) v.SetWritePos(0, 0) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 58c83e126b5..e8d68f1b411 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -253,10 +253,7 @@ func TestWriteCursorPositionEscape(t *testing.T) { got = append(got, cellsToStrings(l.cells)) } - /* EXPECTED: assert.Equal(t, [][]string{{"a"}, {}, {"b"}}, got) - ACTUAL: */ - assert.Equal(t, [][]string{{"a"}, {"b"}}, got) } func stringToCells(s string) []cell { From 0c0c50c3f2385f1fb59717d9c88dcf05a3f85b5f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 09/11] Demonstrate that cursor forward escapes collapse runs of spaces 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) --- pkg/gocui/view_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index e8d68f1b411..b23f914cd3d 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -256,6 +256,76 @@ func TestWriteCursorPositionEscape(t *testing.T) { assert.Equal(t, [][]string{{"a"}, {}, {"b"}}, got) } +func TestWriteCursorPositionEscapeAcrossWrites(t *testing.T) { + // Mirrors the production flow: bufio.Scanner splits the pty output on + // LF and feeds the view one Write per line (each with a trailing \n + // appended). The parser's screen-row counter must keep ticking across + // writes; otherwise CUPs are evaluated against a stale row and + // overshoot, producing too many blank lines instead of the right + // number. + v := NewView("name", 0, 0, 30, 30, OutputNormal) + v.writeString("a\n") + v.writeString("b\n") + // ConPTY is on row 3 here; CUP to row 5 should skip exactly one row. + v.writeString("c\x1b[5;1Hd\n") + + got := make([][]string, 0, len(v.lines)) + for _, l := range v.lines { + got = append(got, cellsToStrings(l.cells)) + } + assert.Equal(t, [][]string{ + {"a"}, + {"b"}, + {"c"}, + {}, + {"d"}, + }, got) +} + +func TestWriteCursorForwardEscape(t *testing.T) { + // ConPTY compresses runs of default-colored spaces into ECH (\x1b[NX, + // "clear N cells, cursor stationary") + CUF (\x1b[NC, "cursor forward + // N") rather than emitting them literally. The interpreter has to + // materialize CUF as N visible spaces; otherwise the gap collapses + // and content that followed the indentation slides left. + v := NewView("name", 0, 0, 20, 10, OutputNormal) + // "a" + ECH 5 + CUF 5 + "b" — visually "a b". + v.writeString("a\x1b[5X\x1b[5Cb\n") + + got := make([][]string, 0, len(v.lines)) + for _, l := range v.lines { + got = append(got, cellsToStrings(l.cells)) + } + + /* EXPECTED: + assert.Equal(t, [][]string{{"a", " ", " ", " ", " ", " ", "b"}}, got) + ACTUAL: */ + assert.Equal(t, [][]string{{"a", "b"}}, got) +} + +func TestWriteCursorPositionEscapeWithSoftWraps(t *testing.T) { + // If a logical line is longer than ConPTY's terminal width, ConPTY + // soft-wraps it onto multiple physical rows in its screen, and any + // subsequent CUP is addressed against the post-wrap row count. To + // keep our screen-row counter accurate we have to count those wraps + // as we write the cells. InnerWidth here is 5; "abcdefghij" (10 + // cells) wraps onto 2 rows, so ConPTY is on row 3 after the LF and a + // CUP to row 4 should skip exactly one row. + v := NewView("name", 0, 0, 6, 30, OutputNormal) // Width=7, InnerWidth=5 + v.writeString("abcdefghij\n") + v.writeString("\x1b[4;1Hxyz\n") + + got := make([][]string, 0, len(v.lines)) + for _, l := range v.lines { + got = append(got, cellsToStrings(l.cells)) + } + assert.Equal(t, [][]string{ + {"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}, + {}, + {"x", "y", "z"}, + }, got) +} + func stringToCells(s string) []cell { var cells []cell state := -1 From 2fce9c91b6d74b5da0d748311d06fb17cecf6144 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 10/11] Materialize cursor-forward escapes as space runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pkg/gocui/escape.go | 23 ++++++++++++++++++++--- pkg/gocui/escape_test.go | 23 +++++++++++++++++++++++ pkg/gocui/view.go | 8 ++++++++ pkg/gocui/view_test.go | 3 --- 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index c252fa20bc2..ad862a5960f 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -54,6 +54,14 @@ type cursorDown struct{ n int } func (self cursorDown) isInstruction() {} +// cursorForward asks the view to materialize N space cells. Emitted +// when CUF advances the cursor right — ConPTY uses CUF (often paired +// with ECH) to encode runs of default-colored spaces compactly, so we +// have to render the gap, not just bump a counter. +type cursorForward struct{ n int } + +func (self cursorForward) isInstruction() {} + type noInstruction struct{} func (self noInstruction) isInstruction() {} @@ -272,10 +280,11 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "0") case characterEquals(ch, 'K'), characterEquals(ch, 'H'), characterEquals(ch, 'f'), characterEquals(ch, 'd'), - characterEquals(ch, 'B'), characterEquals(ch, 'E'): + characterEquals(ch, 'B'), characterEquals(ch, 'E'), + characterEquals(ch, 'C'): // fall through — let stateParams handle these with default - // params (CUP/VPA default to row 1, CUD/CNL default to advance - // by 1). + // params (CUP/VPA default to row 1, CUD/CNL/CUF default to + // advance by 1). case characterEquals(ch, ';'): // Empty first param ([;Xm ≡ [0;Xm). Seed a slot for the // empty param; stateParams will append the next one when it @@ -376,6 +385,14 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.state = stateNone ei.csiParam = nil return true, nil + case characterEquals(ch, 'C'): + // CUF — cursor forward N. Emit space cells so the gap + // renders. (screenCol is updated by the view via + // notifyCellsWritten as those spaces are emitted.) + ei.instruction = cursorForward{n: ei.firstParamOrDefault(1)} + ei.state = stateNone + ei.csiParam = nil + return true, nil case len(ch) == 1 && ch[0] >= 0x20 && ch[0] <= 0x2F: // CSI intermediate byte after params. The final byte will // have a semantic we don't implement (e.g. `[0 q` = diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index a7e8ce02ff9..39ccbe9083b 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -243,6 +243,29 @@ func TestParseOneCursorHomeReanchors(t *testing.T) { } } +func TestParseOneCursorForward(t *testing.T) { + // CUF (\x1b[NC) emits a cursorForward instruction so the view can + // materialize the N-cell gap as spaces. ConPTY uses this (often + // paired with ECH) to encode runs of default-colored spaces. + scenarios := []struct { + input string + wantN int + }{ + {"\x1b[5C", 5}, + {"\x1b[1C", 1}, + {"\x1b[C", 1}, // no param defaults to 1 + } + + for _, s := range scenarios { + ei := newEscapeInterpreter(OutputNormal) + parseEscRunes(t, ei, s.input) + cf, ok := ei.instruction.(cursorForward) + if assert.True(t, ok, "input %q should emit cursorForward", s.input) { + assert.Equal(t, s.wantN, cf.n, "input %q", s.input) + } + } +} + func parseEscRunes(t *testing.T, ei *escapeInterpreter, runes string) { t.Helper() for _, b := range []byte(runes) { diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 1e67f853544..d93f84954e3 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -1003,6 +1003,14 @@ func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) { bg: v.ei.curBgColor, } return truncateLine, []cell{} + } else if cf, ok := v.ei.instruction.(cursorForward); ok { + // emit `n` space cells under the parser-tracked SGR — used + // to materialize ConPTY's compressed runs of spaces (which + // it emits as ECH+CUF instead of literal whitespace). + v.ei.instructionRead() + repeatCount = cf.n + ch = []byte{' '} + width = 1 } else if isEscape { // do not output anything return truncateLine, nil diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index b23f914cd3d..f7e229f1c1b 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -297,10 +297,7 @@ func TestWriteCursorForwardEscape(t *testing.T) { got = append(got, cellsToStrings(l.cells)) } - /* EXPECTED: assert.Equal(t, [][]string{{"a", " ", " ", " ", " ", " ", "b"}}, got) - ACTUAL: */ - assert.Equal(t, [][]string{{"a", "b"}}, got) } func TestWriteCursorPositionEscapeWithSoftWraps(t *testing.T) { From 20af6ad23cbdbaf525b62ed2e772843653553f42 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 30 Jun 2026 09:45:26 +0200 Subject: [PATCH 11/11] Remove the Windows limitation from Custom_Pagers.md --- docs-master/Custom_Pagers.md | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/docs-master/Custom_Pagers.md b/docs-master/Custom_Pagers.md index 0bfffe7dcf4..1b37766d0ca 100644 --- a/docs-master/Custom_Pagers.md +++ b/docs-master/Custom_Pagers.md @@ -2,8 +2,6 @@ Lazygit supports custom pagers, [configured](/docs/Config.md) in the config.yml file (which can be opened by pressing `e` in the Status panel). -Support does not extend to Windows users, because we're making use of a package which doesn't have Windows support. However, see [below](#emulating-custom-pagers-on-windows) for a workaround. - Multiple pagers are supported; you can cycle through them with the `|` key. This can be useful if you usually prefer a particular pager, but want to use a different one for certain kinds of diffs. Pagers are configured with the `pagers` array in the git section; here's an example for a multi-pager setup (use an empty object `{}` for the default builtin diff display that doesn't use a pager): @@ -92,30 +90,3 @@ git: This can be useful if you also want to use it for diffs on the command line, and it also has the advantage that you can configure it per file type in `.gitattributes`; see https://git-scm.com/docs/gitattributes#_defining_an_external_diff_driver. `pager`, `externalDiffCommand`, and `useExternalDiffGitConfig` are alternative ways of producing the diff, so a pager entry may use at most one of them. - -## Emulating custom pagers on Windows - -There is a trick to emulate custom pagers on Windows using a Powershell script configured as an external diff command. It's not perfect, but certainly better than nothing. To do this, save the following script as `lazygit-pager.ps1` at a convenient place on your disk: - -```pwsh -#!/usr/bin/env pwsh - -$old = $args[1].Replace('\', '/') -$new = $args[4].Replace('\', '/') -$path = $args[0] -git diff --no-index --no-ext-diff $old $new - | %{ $_.Replace($old, $path).Replace($new, $path) } - | delta --width=$env:LAZYGIT_COLUMNS -``` - -Use the pager of your choice with the arguments you like in the last line of the script. Personally I wouldn't want to use lazygit anymore without delta's `--hyperlinks --hyperlinks-file-link-format="lazygit-edit://{path}:{line}"` args, see [above](#delta). - -In your lazygit config, use - -```yml -git: - pagers: - - externalDiffCommand: "C:/wherever/lazygit-pager.ps1" -``` - -The main limitation of this approach compared to a "real" pager is that renames are not displayed correctly; they are shown as if they were modifications of the old file. (This affects only the hunk headers; the diff itself is always correct.)