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 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.) 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) -} 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/commands/oscommands/pty.go b/pkg/commands/oscommands/pty.go new file mode 100644 index 00000000000..a4a36963333 --- /dev/null +++ b/pkg/commands/oscommands/pty.go @@ -0,0 +1,34 @@ +package oscommands + +import ( + "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 +} + +// 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..0a4a0647763 --- /dev/null +++ b/pkg/commands/oscommands/pty_windows.go @@ -0,0 +1,259 @@ +package oscommands + +import ( + "fmt" + "os" + "os/exec" + "sync" + "unsafe" + + "golang.org/x/sys/windows" +) + +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/gocui/escape.go b/pkg/gocui/escape.go index 726fd4de7aa..ad862a5960f 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,21 @@ 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() {} + +// 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() {} @@ -99,11 +129,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 +145,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 +278,13 @@ 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'), + characterEquals(ch, 'C'): + // fall through — let stateParams handle these with default + // 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 @@ -240,6 +353,43 @@ 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 + 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 diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index cb8eb1a4b81..39ccbe9083b 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,89 @@ 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 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 77492b9b108..d93f84954e3 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()) + } } } @@ -982,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 @@ -1116,6 +1145,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 f65418821b3..f7e229f1c1b 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -238,6 +238,91 @@ 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)) + } + + 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)) + } + + 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 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 f6356b9c0a4..1a774fc3d8e 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -1,35 +1,35 @@ -//go:build !windows - package gui import ( + "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) } } @@ -37,6 +37,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, @@ -45,6 +58,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() @@ -74,24 +93,28 @@ 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) { - var err error - ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view)) + var p oscommands.Pty + start := func() (tasks.Cmd, io.Reader) { + cols, rows := gui.desiredPtySize(view) + sp, err := oscommands.StartPty(cmd, cols, rows) if err != nil { 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 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) -} 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() {})