Separate setting up a render from running it in a pty

A render is about to have a second way of giving the diff renderer the command's output, so the part that decides how it runs needs to be apart from the part that does. Move the setting up to newRenderTask, named for what it does now that a pty is one of two ways of doing it. Leave the pty with the pair of functions a task drives it by, and with naming the stdin filter to git as its pager, since git only runs a pager when it talks to a terminal. What a way needs to know about the render it runs travels as a renderSpec. Running the command plainly, with its output going straight into a pipe, is a way of its own already, and newCmdTask ends as newRenderTask does, by creating the task that reads the output into the view. Put the plain way on the same seam as plainRender, and share the ending as newTaskForRender. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-27 17:56:37 -04:00 · 2026-09-25 11:31:59 +02:00
parent ec3427fdcc
commit 118d9e8e1f
4 changed files with 203 additions and 140 deletions
@@ -21,7 +21,7 @@ func (gui *Gui) runTaskForView(view *gocui.View, task types.UpdateTask) error {
 		return gui.newCmdTask(view, v.Cmd, v.Prefix)

 	case *types.RunPtyTask:
-		return gui.newPtyTask(view, v.Cmd, v.Prefix)
+		return gui.newRenderTask(view, v.Cmd, v.Prefix)
 	}

 	return nil
@@ -0,0 +1,137 @@
+package gui
+
+import (
+	"fmt"
+	"io"
+	"os/exec"
+	"runtime"
+	"strings"
+
+	"github.com/jesseduffield/lazygit/pkg/config"
+	"github.com/jesseduffield/lazygit/pkg/gocui"
+	"github.com/jesseduffield/lazygit/pkg/tasks"
+	"github.com/samber/lo"
+)
+
+// renderSpec describes a render of a command's output into a view: what a way
+// of running the command (see runRender) needs to know about it.
+type renderSpec struct {
+	view *gocui.View
+	cmd  *exec.Cmd
+	// The width the renderer lays its rendering out to, and the width the view
+	// counts its own wrapping against. Read after the layout pass, which
+	// settles it.
+	width int
+	// The configured stdin filter, empty unless one is configured. How git gets
+	// to run it depends on the way the render runs. An external diff renderer
+	// is named to git in the environment before the render is set up, whichever
+	// way it runs, so it doesn't appear here; nothing else is a command of its
+	// own.
+	stdinFilter string
+}
+
+// newRenderTask renders cmd's output into view, through the diff renderer the
+// user has configured. The renderer lays its rendering out to the width of the
+// view, which only the layout settles, so the task is created after it.
+func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) error {
+	width := view.InnerWidth()
+
+	// Set LAZYGIT_COLUMNS for diff renderer scripts that can't query the terminal width directly.
+	cmd.Env = append(cmd.Env, fmt.Sprintf("LAZYGIT_COLUMNS=%d", width))
+
+	if gui.stateAccessor.GetDiffRendererConfigManager().GetDiffRendererType() == config.DiffRendererType_RawGit {
+		// If we're not using a custom diff renderer, then we don't need to use a pty
+		return gui.newCmdTask(view, cmd, prefix)
+	}
+
+	cmd.Args = withPtyGitConfig(cmd.Args, runtime.GOOS)
+
+	// Mark the view as loading synchronously now, before the layout pass: the
+	// actual task is created in afterLayout (below), which runs after layout, so
+	// without this the next layout pass would clamp the scroll position to the
+	// not-yet-loaded content.
+	gui.getManager(view).StartLoading()
+	// Hold the scrollbar at its current height while the re-render loads, so the
+	// thumb doesn't shrink and snap back when the first partial paint swaps in
+	// (see the matching call in newCmdTask).
+	view.FreezeScrollbarHeight()
+
+	// Run the render after layout so that it gets the correct size
+	gui.afterLayout(func() error {
+		// Need to get the width and the renderer command again because the layout
+		// might have changed the size of the view
+		width = view.InnerWidth()
+		diffRendererConfigManager := gui.stateAccessor.GetDiffRendererConfigManager()
+		stdinFilter := diffRendererConfigManager.GetStdinFilterCommand(width)
+		externalDiff := diffRendererConfigManager.GetExternalDiffCommand(gui.c.UserConfig().Git.DiffContextSize, width)
+
+		cmdStr := strings.Join(cmd.Args, " ")
+
+		// This communicates to diff renderers that we're in a very simple
+		// terminal that they should not expect to have much capabilities.
+		// Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities.
+		// Context: https://github.com/jesseduffield/lazygit/issues/3419
+		cmd.Env = removeExistingTermEnvVars(cmd.Env)
+		cmd.Env = append(cmd.Env, "TERM=dumb")
+
+		// An external diff command is named to git here, in the environment,
+		// because the width it renders at is only known after the layout, and
+		// the command's arguments were settled before it. An empty command
+		// means the user wants git's own diff.external config to apply, so
+		// leave the variable unset in that case; git takes it being set at all
+		// as an instruction, however little it says.
+		if externalDiff != "" {
+			cmd.Env = append(cmd.Env, "GIT_EXTERNAL_DIFF="+externalDiff)
+		}
+
+		spec := renderSpec{
+			view:        view,
+			cmd:         cmd,
+			width:       width,
+			stdinFilter: stdinFilter,
+		}
+		return gui.newTaskForRender(spec, prefix, cmdStr, gui.ptyRender)
+	})
+
+	return nil
+}
+
+// The start and onClose functions a render hands to its task: how to get the
+// command running and the output reader for it, and how to tear it down again
+// once the task is stopped.
+type (
+	startRender   func() (tasks.Cmd, io.Reader)
+	onCloseRender func()
+)
+
+// runRender is a way of running a render's command and getting at its output:
+// plainly, or in a pty. It returns the functions the task drives the command by.
+type runRender func(spec renderSpec) (startRender, onCloseRender)
+
+// newTaskForRender creates the task that reads the render's output into its
+// view, running the command the given way. key names what is rendered, so that
+// a re-render of the same content can be told from a render of other content.
+func (gui *Gui) newTaskForRender(spec renderSpec, prefix string, key string, run runRender) error {
+	start, onClose := run(spec)
+
+	manager := gui.getManager(spec.view)
+	linesToRead := gui.linesToReadFromCmdTask(spec.view)
+	return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), key)
+}
+
+func removeExistingTermEnvVars(env []string) []string {
+	return lo.Filter(env, func(envVar string, _ int) bool {
+		return !isTermEnvVar(envVar)
+	})
+}
+
+// Terminals set a variety of different environment variables
+// to identify themselves to processes. This list should catch the most common among them.
+func isTermEnvVar(envVar string) bool {
+	return strings.HasPrefix(envVar, "TERM=") ||
+		strings.HasPrefix(envVar, "TERM_PROGRAM=") ||
+		strings.HasPrefix(envVar, "TERM_PROGRAM_VERSION=") ||
+		strings.HasPrefix(envVar, "TERMINAL_EMULATOR=") ||
+		strings.HasPrefix(envVar, "TERMINAL_NAME=") ||
+		strings.HasPrefix(envVar, "TERMINAL_VERSION_")
+}
@@ -1,20 +1,16 @@
 package gui

 import (
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"runtime"
 	"strings"

 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
-	"github.com/jesseduffield/lazygit/pkg/config"
 	"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) (cols, rows uint16) {
@@ -53,116 +49,63 @@ func (p ptyCmd) Wait() error      { return p.wait() }
 func (p ptyCmd) String() string   { return p.cmd.String() }
 func (p ptyCmd) Terminate() error { return oscommands.TerminateProcessGracefully(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,
-// which is just an io.Reader. the pty package lets us wrap a command in a
-// pseudo-terminal meaning we'll get the behaviour we want from the underlying
-// command.
-func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error {
-	width := view.InnerWidth()
+// ptyRender runs the command in a pseudo-terminal. git invokes the stdin filter
+// named by GIT_PAGER only when it talks to a terminal, and a renderer reads the
+// width it lays out to off it.
+//
+// Must be called on the UI thread: it reads the view's dimensions, which the
+// layout writes.
+func (gui *Gui) ptyRender(spec renderSpec) (startRender, onCloseRender) {
+	view := spec.view
+	cmd := spec.cmd

-	// Set LAZYGIT_COLUMNS for diff renderer scripts that can't query the terminal width directly.
-	cmd.Env = append(cmd.Env, fmt.Sprintf("LAZYGIT_COLUMNS=%d", width))
+	// git runs the stdin filter itself, as the pager it is told about here.
+	// Named even when there is none, so that git doesn't reach for the user's
+	// core.pager instead.
+	cmd.Env = append(cmd.Env, "GIT_PAGER="+spec.stdinFilter)

-	if gui.stateAccessor.GetDiffRendererConfigManager().GetDiffRendererType() == config.DiffRendererType_RawGit {
-		// If we're not using a custom diff renderer, then we don't need to use a pty
-		return gui.newCmdTask(view, cmd, prefix)
+	cols, rows := gui.desiredPtySize(view)
+
+	var p oscommands.Pty
+	var fallbackPipe io.ReadCloser
+	start := func() (tasks.Cmd, io.Reader) {
+		// The pty (and diff renderer) wrap to this width; apply it here, on the
+		// task's goroutine once the previous task has stopped, so it doesn't
+		// race that task's writes (see View.SetContentWidth).
+		view.SetContentWidth(spec.width)
+
+		sp, err := oscommands.StartPty(cmd, cols, rows)
+		if err != nil {
+			gui.c.Log.Error(err)
+			// Fall back to running the command without a pty: the diff renderer is
+			// lost, but the command's output still renders.
+			execCmd, pipe := startCmdWithPipe(cmd, gui.c.Log)
+			fallbackPipe = pipe
+			return execCmd, pipe
+		}
+		p = sp.Pty
+
+		gui.Mutexes.PtyMutex.Lock()
+		gui.viewPtmxMap[view.Name()] = p
+		gui.Mutexes.PtyMutex.Unlock()
+
+		return ptyCmd{cmd: cmd, process: sp.Process, wait: sp.Wait}, p
 	}

-	cmd.Args = withPtyGitConfig(cmd.Args, runtime.GOOS)
-
-	// Mark the view as loading synchronously now, before the layout pass: the
-	// actual task is created in afterLayout (below), which runs after layout, so
-	// without this the next layout pass would clamp the scroll position to the
-	// not-yet-loaded content.
-	gui.getManager(view).StartLoading()
-	// Hold the scrollbar at its current height while the re-render loads, so the
-	// thumb doesn't shrink and snap back when the first partial paint swaps in
-	// (see the matching call in newCmdTask).
-	view.FreezeScrollbarHeight()
-
-	// Run the pty after layout so that it gets the correct size
-	gui.afterLayout(func() error {
-		// Need to get the width and the renderer command again because the layout
-		// might have changed the size of the view
-		width = view.InnerWidth()
-		diffRendererConfigManager := gui.stateAccessor.GetDiffRendererConfigManager()
-		pager := diffRendererConfigManager.GetStdinFilterCommand(width)
-		externalDiff := diffRendererConfigManager.GetExternalDiffCommand(gui.c.UserConfig().Git.DiffContextSize, width)
-
-		cmdStr := strings.Join(cmd.Args, " ")
-
-		// This communicates to diff renderers that we're in a very simple
-		// terminal that they should not expect to have much capabilities.
-		// Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities.
-		// Context: https://github.com/jesseduffield/lazygit/issues/3419
-		cmd.Env = removeExistingTermEnvVars(cmd.Env)
-		cmd.Env = append(cmd.Env, "TERM=dumb")
-
-		cmd.Env = append(cmd.Env, "GIT_PAGER="+pager)
-
-		// An external diff command is named to git here, in the environment,
-		// because the width it renders at is only known after the layout, and
-		// the command's arguments were settled before it. An empty command
-		// means the user wants git's own diff.external config to apply, so
-		// leave the variable unset in that case; git takes it being set at all
-		// as an instruction, however little it says.
-		if externalDiff != "" {
-			cmd.Env = append(cmd.Env, "GIT_EXTERNAL_DIFF="+externalDiff)
+	onClose := func() {
+		gui.Mutexes.PtyMutex.Lock()
+		if p != nil {
+			p.Close()
 		}
-
-		manager := gui.getManager(view)
-
-		// Size the pty from the view's dimensions here, on the UI thread; the
-		// start func below runs on the task's goroutine, which must not read the
-		// view's live dimensions while the UI thread is laying it out.
-		cols, rows := gui.desiredPtySize(view)
-
-		var p oscommands.Pty
-		var fallbackPipe io.ReadCloser
-		start := func() (tasks.Cmd, io.Reader) {
-			// The pty (and diff renderer) wrap to this width; apply it here, on the
-			// task's goroutine once the previous task has stopped, so it doesn't
-			// race that task's writes (see View.SetContentWidth).
-			view.SetContentWidth(width)
-
-			sp, err := oscommands.StartPty(cmd, cols, rows)
-			if err != nil {
-				gui.c.Log.Error(err)
-				// Fall back to running the command without a pty: the diff renderer is
-				// lost, but the command's output still renders.
-				execCmd, pipe := startCmdWithPipe(cmd, gui.c.Log)
-				fallbackPipe = pipe
-				return execCmd, pipe
-			}
-			p = sp.Pty
-
-			gui.Mutexes.PtyMutex.Lock()
-			gui.viewPtmxMap[view.Name()] = p
-			gui.Mutexes.PtyMutex.Unlock()
-
-			return ptyCmd{cmd: cmd, process: sp.Process, wait: sp.Wait}, p
+		if fallbackPipe != nil {
+			fallbackPipe.Close()
+			fallbackPipe = nil
 		}
+		delete(gui.viewPtmxMap, view.Name())
+		gui.Mutexes.PtyMutex.Unlock()
+	}

-		onClose := func() {
-			gui.Mutexes.PtyMutex.Lock()
-			if p != nil {
-				p.Close()
-			}
-			if fallbackPipe != nil {
-				fallbackPipe.Close()
-				fallbackPipe = nil
-			}
-			delete(gui.viewPtmxMap, view.Name())
-			gui.Mutexes.PtyMutex.Unlock()
-		}
-
-		linesToRead := gui.linesToReadFromCmdTask(view)
-		return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr)
-	})
-
-	return nil
+	return start, onClose
 }

 // withPtyGitConfig returns args with extra git configuration for commands
@@ -201,20 +144,3 @@ func withPtyGitConfig(args []string, goos string) []string {
 	result = append(result, "-c", "diff.autoRefreshIndex=false")
 	return append(result, args[1:]...)
 }
-
-func removeExistingTermEnvVars(env []string) []string {
-	return lo.Filter(env, func(envVar string, _ int) bool {
-		return !isTermEnvVar(envVar)
-	})
-}
-
-// Terminals set a variety of different environment variables
-// to identify themselves to processes. This list should catch the most common among them.
-func isTermEnvVar(envVar string) bool {
-	return strings.HasPrefix(envVar, "TERM=") ||
-		strings.HasPrefix(envVar, "TERM_PROGRAM=") ||
-		strings.HasPrefix(envVar, "TERM_PROGRAM_VERSION=") ||
-		strings.HasPrefix(envVar, "TERMINAL_EMULATOR=") ||
-		strings.HasPrefix(envVar, "TERMINAL_NAME=") ||
-		strings.HasPrefix(envVar, "TERMINAL_VERSION_")
-}
@@ -17,11 +17,10 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 		cmdStr,
 	).Debug("RunCommand")

-	manager := gui.getManager(view)
 	// Mark the view as loading synchronously (before the task's goroutine runs
 	// and before the next layout pass) so the layout doesn't clamp the scroll
 	// position to the not-yet-loaded content.
-	manager.StartLoading()
+	gui.getManager(view).StartLoading()
 	// Hold the scrollbar at the height the view has now (the previous render),
 	// while it still shows that render: once the re-render swaps in its first
 	// partial paint the displayed buffer is briefly short, and we don't want the
@@ -29,17 +28,23 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 	view.FreezeScrollbarHeight()

 	// Snapshot the view width here, on the UI thread, so the task goroutine
-	// doesn't read the view's live dimensions while it streams output. It's
-	// applied inside start() below rather than now, because start() runs once
-	// the previous task has stopped -- applying it here would race that task's
-	// still-running writes (see View.SetContentWidth).
-	contentWidth := view.InnerWidth()
+	// doesn't read the view's live dimensions while it streams output.
+	spec := renderSpec{view: view, cmd: cmd, width: view.InnerWidth()}

+	return gui.newTaskForRender(spec, prefix, cmdStr, gui.plainRender)
+}
+
+// plainRender runs the command as it is, with its output going straight into
+// a pipe.
+func (gui *Gui) plainRender(spec renderSpec) (startRender, onCloseRender) {
 	var r io.ReadCloser
 	start := func() (tasks.Cmd, io.Reader) {
-		view.SetContentWidth(contentWidth)
+		// The view wraps to this width; apply it here, on the task's goroutine
+		// once the previous task has stopped, so it doesn't race that task's
+		// still-running writes (see View.SetContentWidth).
+		spec.view.SetContentWidth(spec.width)

-		execCmd, pipe := startCmdWithPipe(cmd, gui.c.Log)
+		execCmd, pipe := startCmdWithPipe(spec.cmd, gui.c.Log)
 		r = pipe
 		return execCmd, pipe
 	}
@@ -51,12 +56,7 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 		}
 	}

-	linesToRead := gui.linesToReadFromCmdTask(view)
-	if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
-		gui.c.Log.Error(err)
-	}
-
-	return nil
+	return start, onClose
 }

 // startCmdWithPipe starts cmd with its stdout and stderr going to a single