Render diffs without a PTY on Windows (#6025)

In #5740 we implemented PTY support for Windows; back then we thought
this is a prerequisite for supporting custom diff renderers (which were
still called "custom pagers" back then), because git will only use the
GIT_PAGER env var when it is running in a PTY. The problem is that the
Windows PTY, being based on ConPTY, does not behave like a Unix PTY,
which basically just passes through all data from the client. ConPTY
renders what it receives from the client into its own screen buffer, and
then re-encodes it from there for the terminal side. This has already
caused problems that are awkward to work around (e.g. ConPTY will
convert a series of multiple blank lines to a cursor positioning escape
sequence, so we need to parse that and convert it back, see
180fe0cd26); but now, with the upcoming OSC 1717 work, it turns out
that it's impossible to attach OSC 1717 metadata records to the cells
they belong to, because ConPTY sends those immediately to the terminal,
but the rest of the cell data some time later, and it's impossible to
reconstruct the original stream.

So use an ordinary pipe on Windows, where we start git and the diff
renderer on our side instead of telling git to drive the renderer. It's
a shame that we didn't realize it's possible; we could have done this
years ago without having to wait for a working Windows PTY.

One downside is that the diff renderer can no longer ask the terminal
how wide it is, so if it needs to know that (e.g. for a side-by-side
diff, or for horizontal lines that should be as wide as the view), then
it needs another way to find out. We set the `COLUMNS` environment
variable, which delta, difftastic and diff-so-fancy all support in their
latest versions, and for those renderers that don't, we provide a
`{{width}}` template variable that can be used in a diff renderer
command to pass it as a command-line argument.

pkg/commands/git_commands/commit_test.go

@@ -312,7 +312,7 @@ func TestCommitShowCmdObj(t *testing.T) {
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
 			diffRendererConfig:  &config.DiffRendererConfig{Type: "extDiff", Command: "difft --color=always"},
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.external=difft --color=always", "-c", "diff.noprefix=false", "show", "--ext-diff", "--unified=3", "--find-renames=50%", "--submodule", "--color=always", "--stat", "--decorate", "-p", "1234567890", "--"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--ext-diff", "--unified=3", "--find-renames=50%", "--submodule", "--color=always", "--stat", "--decorate", "-p", "1234567890", "--"},
 		},
 		{
 			testName:            "Show diff using git's external diff config",

pkg/commands/git_commands/git_command_builder.go

@@ -125,11 +125,9 @@ func (self *GitCommandBuilder) GitDirIf(condition bool, path string) *GitCommand
 
 func (self *GitCommandBuilder) AddCommonDiffArgs(diffRendererConfigManager *config.DiffRendererConfigManager, userConfig *config.UserConfig, forUI bool) *GitCommandBuilder {
 	contextSize := userConfig.Git.DiffContextSize
-	extDiffCmd := diffRendererConfigManager.GetExternalDiffCommand(contextSize)
 	useExtDiff := forUI && diffRendererConfigManager.GetDiffRendererType() == config.DiffRendererType_ExtDiff
 
 	return self.
-		ConfigIf(forUI && extDiffCmd != "", "diff.external="+extDiffCmd).
 		ArgIfElse(useExtDiff, "--ext-diff", "--no-ext-diff").
 		Arg(fmt.Sprintf("--unified=%d", contextSize)).
 		ArgIf(forUI && userConfig.Git.IgnoreWhitespaceInDiffView, "--ignore-all-space").

pkg/commands/git_commands/stash_test.go

@@ -139,7 +139,7 @@ func TestStashStashEntryCmdObj(t *testing.T) {
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
 			diffRendererConfig:  &config.DiffRendererConfig{Type: "extDiff", Command: "difft --color=always"},
-			expected:            []string{"git", "-C", "/path/to/worktree", "-c", "diff.external=difft --color=always", "stash", "show", "--ext-diff", "--unified=3", "--find-renames=50%", "-p", "--stat", "-u", "--color=always", "refs/stash@{5}"},
+			expected:            []string{"git", "-C", "/path/to/worktree", "stash", "show", "--ext-diff", "--unified=3", "--find-renames=50%", "-p", "--stat", "-u", "--color=always", "refs/stash@{5}"},
 		},
 		{
 			testName:            "Show diff using git's external diff config",

pkg/commands/oscommands/cmd_obj.go

@@ -108,6 +108,14 @@ func (self *CmdObj) GetEnvVars() []string {
 	return self.cmd.Env
 }
 
+// SetEnviron replaces the command's whole environment, for a command that has
+// to run in the same one as another command rather than in this process's.
+func (self *CmdObj) SetEnviron(env []string) *CmdObj {
+	self.cmd.Env = env
+
+	return self
+}
+
 // sets the working directory
 func (self *CmdObj) SetWd(wd string) *CmdObj {
 	self.cmd.Dir = wd

pkg/commands/oscommands/cmd_obj_builder.go

@@ -33,6 +33,16 @@ func (self *CmdObjBuilder) New(args []string) *CmdObj {
 	return cmdObj
 }
 
+// NewFromCmd wraps a command that has already been built, for a caller that
+// holds an *exec.Cmd and needs it as a CmdObj. The command itself is shared,
+// not copied, so whatever was set on it still applies.
+func (self *CmdObjBuilder) NewFromCmd(cmd *exec.Cmd) *CmdObj {
+	return &CmdObj{
+		cmd:    cmd,
+		runner: self.runner,
+	}
+}
+
 // A command with explicit environment from env
 func (self *CmdObjBuilder) NewWithEnviron(args []string, env []string) *CmdObj {
 	cmd := exec.Command(args[0], args[1:]...)

pkg/commands/oscommands/os.go

@@ -4,12 +4,10 @@ import (
 	"bytes"
 	"io"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 
 	"github.com/go-errors/errors"
-	"github.com/samber/lo"
 
 	"github.com/atotto/clipboard"
 	"github.com/jesseduffield/lazygit/pkg/common"
@@ -203,61 +201,25 @@ func (c *OSCommand) FileExists(path string) (bool, error) {
 
 // PipeCommands runs a heap of commands and pipes their inputs/outputs together like A | B | C
 func (c *OSCommand) PipeCommands(cmdObjs ...*CmdObj) error {
-	cmds := lo.Map(cmdObjs, func(cmdObj *CmdObj, _ int) *exec.Cmd {
-		return cmdObj.GetCmd()
-	})
+	c.logPipeline(cmdObjs)
 
-	logCmdStr := strings.Join(
-		lo.Map(cmdObjs, func(cmdObj *CmdObj, _ int) string {
-			return cmdObj.ToString()
-		}),
-		" | ",
-	)
-
-	c.LogCommand(logCmdStr, true)
-
-	for i := range len(cmds) - 1 {
-		stdout, err := cmds[i].StdoutPipe()
-		if err != nil {
-			return err
-		}
-
-		cmds[i+1].Stdin = stdout
+	cmds, parentEnds, err := wirePipeline(cmdObjs)
+	if err != nil {
+		return err
 	}
 
-	// keeping this here in case I adapt this code for some other purpose in the future
-	// cmds[len(cmds)-1].Stdout = os.Stdout
-
 	stderrs := make([]bytes.Buffer, len(cmds))
 	for i := range cmds {
 		cmds[i].Stderr = &stderrs[i]
 	}
 
-	// Start every command before waiting for any of them: waiting for a command
-	// closes our end of the pipe that feeds the next one, and a command that
-	// hasn't been started by then would inherit a closed stdin.
-	started := 0
-	var startErr error
-	for _, cmd := range cmds {
-		if err := cmd.Start(); err != nil {
-			startErr = err
-			break
-		}
-
-		started++
-	}
+	started, startErr := startPipeline(cmds, parentEnds)
 
 	finalErrors := []string{}
 
 	if startErr != nil {
 		c.Log.Error(startErr)
 		finalErrors = append(finalErrors, startErr.Error())
-
-		// Without the rest of the pipeline to drain them, the commands we did
-		// start could block forever writing to a full pipe.
-		for _, cmd := range cmds[:started] {
-			_ = cmd.Process.Kill()
-		}
 	}
 
 	for i, cmd := range cmds[:started] {

pkg/commands/oscommands/pipeline.go

@@ -0,0 +1,180 @@
+package oscommands
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/samber/lo"
+)
+
+// Pipeline is a chain of running commands, each one's output feeding the next
+// one's input, with the last one's output going somewhere the caller reads. Its
+// method set is the one a render task expects of a command (see tasks.Cmd), so
+// a pipeline can render a view just as a single command can.
+type Pipeline struct {
+	cmds   []*exec.Cmd
+	cmdStr string
+}
+
+// StartPipeline starts the given commands wired A | B | C and returns the
+// pipeline together with the reader for its output.
+//
+// Every command's stderr goes to that same output, so whatever a command
+// complains about is part of what the caller reads. A diff renderer's error
+// message belongs on screen with the diff it failed to render.
+//
+// Closing the reader is how a pipeline is brought down. The last command's next
+// write fails, so it exits, and the failure travels back up the chain as each
+// command in turn writes into a pipe whose reader is gone.
+func (c *OSCommand) StartPipeline(cmdObjs ...*CmdObj) (*Pipeline, io.ReadCloser, error) {
+	c.logPipeline(cmdObjs)
+
+	cmds, parentEnds, err := wirePipeline(cmdObjs)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	reader, writer, err := os.Pipe()
+	if err != nil {
+		closeAll(parentEnds)
+		return nil, nil, err
+	}
+	for _, cmd := range cmds {
+		cmd.Stderr = writer
+	}
+	cmds[len(cmds)-1].Stdout = writer
+	parentEnds = append(parentEnds, writer)
+
+	started, err := startPipeline(cmds, parentEnds)
+	if err != nil {
+		for _, cmd := range cmds[:started] {
+			_ = cmd.Wait()
+		}
+		_ = reader.Close()
+		return nil, nil, err
+	}
+
+	return &Pipeline{cmds: cmds, cmdStr: pipelineString(cmdObjs)}, reader, nil
+}
+
+func (self *Pipeline) String() string {
+	return self.cmdStr
+}
+
+// Wait waits for every command to exit and reports the failure nearest the end
+// of the pipeline. A command that fails leaves the ones before it writing into
+// a pipe nobody reads, so their own broken-pipe failures are consequences of it
+// rather than the cause worth reporting, while a command that fails early
+// leaves the ones after it with nothing to read and no reason to fail at all.
+func (self *Pipeline) Wait() error {
+	var lastErr error
+	for _, cmd := range self.cmds {
+		if err := cmd.Wait(); err != nil {
+			lastErr = fmt.Errorf("%s: %w", cmd.String(), err)
+		}
+	}
+
+	return lastErr
+}
+
+// Terminate asks every command to stop, without waiting for any of them. On
+// platforms where that does nothing, the pipeline comes down when its output
+// reader is closed; see StartPipeline.
+func (self *Pipeline) Terminate() error {
+	var firstErr error
+	for _, cmd := range self.cmds {
+		if err := TerminateProcessGracefully(cmd.Process); err != nil && firstErr == nil {
+			firstErr = err
+		}
+	}
+
+	return firstErr
+}
+
+// logPipeline enters a chain of commands into the command log, unless the first
+// command was marked not to be logged; it speaks for the pipeline. A render runs
+// its pipeline again on every selection change, so a caller has to be able to
+// keep it out of the log.
+func (c *OSCommand) logPipeline(cmdObjs []*CmdObj) {
+	if cmdObjs[0].ShouldLog() {
+		c.LogCommand(pipelineString(cmdObjs), true)
+	}
+}
+
+// pipelineString names a chain of commands the way a shell would write it.
+func pipelineString(cmdObjs []*CmdObj) string {
+	return strings.Join(
+		lo.Map(cmdObjs, func(cmdObj *CmdObj, _ int) string {
+			return cmdObj.ToString()
+		}),
+		" | ",
+	)
+}
+
+// wirePipeline connects each command's output to the next one's input, like
+// A | B | C, and returns the commands along with the parent's ends of those
+// pipes. The last command's output is left for the caller to direct.
+//
+// The parent's ends have to be closed once the commands are running.
+// startPipeline does that; see there for why it matters.
+func wirePipeline(cmdObjs []*CmdObj) ([]*exec.Cmd, []io.Closer, error) {
+	cmds := lo.Map(cmdObjs, func(cmdObj *CmdObj, _ int) *exec.Cmd {
+		return cmdObj.GetCmd()
+	})
+
+	parentEnds := []io.Closer{}
+	for i := range len(cmds) - 1 {
+		reader, writer, err := os.Pipe()
+		if err != nil {
+			closeAll(parentEnds)
+			return nil, nil, err
+		}
+
+		cmds[i].Stdout = writer
+		cmds[i+1].Stdin = reader
+		parentEnds = append(parentEnds, reader, writer)
+	}
+
+	return cmds, parentEnds, nil
+}
+
+// startPipeline starts every command and reports how many it got going. Every
+// one is started before any of them is waited for: waiting closes our end of
+// the pipe that feeds the next command, and one that hasn't been started by
+// then would inherit a closed stdin.
+//
+// Once they are all running, each of them holds its own ends of the pipes it
+// reads and writes, and the parent lets go of its copies. Both directions
+// matter. While the parent holds the read end of a link, a command writing
+// into it never learns that the command meant to read it is gone, and keeps
+// running after the pipeline has been brought down. While the parent holds the
+// write end, the command reading it never reaches the end of its input.
+//
+// When a command fails to start, the ones already running are killed, since
+// without the rest of the pipeline to drain them they could block forever
+// writing to a full pipe. They still have to be reaped, so the count covers
+// them too.
+func startPipeline(cmds []*exec.Cmd, parentEnds []io.Closer) (int, error) {
+	defer closeAll(parentEnds)
+
+	for i, cmd := range cmds {
+		if err := cmd.Start(); err != nil {
+			for _, started := range cmds[:i] {
+				_ = started.Process.Kill()
+			}
+
+			return i, err
+		}
+	}
+
+	return len(cmds), nil
+}
+
+func closeAll(closers []io.Closer) {
+	for _, closer := range closers {
+		_ = closer.Close()
+	}
+}

pkg/commands/oscommands/pipeline_test.go

@@ -0,0 +1,143 @@
+package oscommands
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// The pipeline tests need programs to run, and the test binary is the one
+// program every platform we test on is sure to have. pipelineMember builds a
+// command that re-runs this binary in the role a member of the pipeline is to
+// play; the roles are in TestPipelineMember.
+const pipelineRoleEnvVar = "LAZYGIT_TEST_PIPELINE_ROLE"
+
+func pipelineMember(role string) *CmdObj {
+	return NewDummyOSCommand().Cmd.
+		New([]string{os.Args[0], "-test.run=^TestPipelineMember$"}).
+		AddEnvVars(pipelineRoleEnvVar + "=" + role)
+}
+
+// TestPipelineMember is the program the pipeline tests run, not a test of its
+// own. It exits before the testing package reports anything, so that its output
+// is what the role wrote and nothing else.
+func TestPipelineMember(t *testing.T) {
+	switch os.Getenv(pipelineRoleEnvVar) {
+	case "":
+		t.Skip("not a test; the pipeline tests run this binary in a role")
+	case "count":
+		for i := 1; i <= 3; i++ {
+			fmt.Printf("line %d\n", i)
+		}
+	case "upcase":
+		input, _ := io.ReadAll(os.Stdin)
+		fmt.Print(strings.ToUpper(string(input)))
+	case "copy":
+		_, _ = io.Copy(os.Stdout, os.Stdin)
+	case "complain":
+		fmt.Fprintln(os.Stderr, "something went wrong")
+		os.Exit(3)
+	case "flood":
+		// A failed write means the reader is gone, and there is no point
+		// writing to nobody. On platforms that raise a signal for it instead,
+		// this process is already dead by the time the write returns.
+		for i := 1; ; i++ {
+			if _, err := fmt.Printf("line %d\n", i); err != nil {
+				break
+			}
+		}
+	}
+
+	os.Exit(0)
+}
+
+func TestStartPipelineStreamsTheOutputOfTheLastCommand(t *testing.T) {
+	pipeline, reader, err := NewDummyOSCommand().StartPipeline(
+		pipelineMember("count"),
+		pipelineMember("upcase"),
+	)
+	assert.NoError(t, err)
+
+	output, err := io.ReadAll(reader)
+	assert.NoError(t, err)
+	assert.Equal(t, "LINE 1\nLINE 2\nLINE 3\n", string(output))
+
+	assert.NoError(t, pipeline.Wait())
+	assert.NoError(t, reader.Close())
+}
+
+func TestStartPipelineReadsWhatTheCommandsComplainAbout(t *testing.T) {
+	pipeline, reader, err := NewDummyOSCommand().StartPipeline(
+		pipelineMember("count"),
+		pipelineMember("complain"),
+	)
+	assert.NoError(t, err)
+
+	output, err := io.ReadAll(reader)
+	assert.NoError(t, err)
+	assert.Equal(t, "something went wrong\n", string(output))
+
+	// The failure of the command nearest the output is the one reported, even
+	// though the one feeding it was left writing into a pipe nobody reads.
+	assert.ErrorContains(t, pipeline.Wait(), "exit status 3")
+
+	assert.NoError(t, reader.Close())
+}
+
+func TestClosingAPipelinesOutputBringsItDown(t *testing.T) {
+	pipeline, reader, err := NewDummyOSCommand().StartPipeline(
+		pipelineMember("flood"),
+		pipelineMember("copy"),
+	)
+	assert.NoError(t, err)
+
+	// Read some output first, so that both commands are past their startup and
+	// really running when the reader goes.
+	buf := make([]byte, len("line 1\n"))
+	_, err = io.ReadFull(reader, buf)
+	assert.NoError(t, err)
+	assert.Equal(t, "line 1\n", string(buf))
+
+	assert.NoError(t, reader.Close())
+
+	done := make(chan error, 1)
+	go func() { done <- pipeline.Wait() }()
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("the pipeline was still running long after its output was closed")
+	}
+}
+
+func TestStartPipelineReportsACommandItCannotStart(t *testing.T) {
+	osCommand := NewDummyOSCommand()
+
+	_, _, err := osCommand.StartPipeline(
+		pipelineMember("count"),
+		osCommand.Cmd.New([]string{"lazygit-no-such-command"}),
+	)
+	assert.Error(t, err)
+}
+
+func TestPipeCommandsReturnsWhenALaterCommandDiesEarly(t *testing.T) {
+	done := make(chan error, 1)
+	go func() {
+		done <- NewDummyOSCommand().PipeCommands(
+			pipelineMember("flood"),
+			pipelineMember("complain"),
+		)
+	}()
+
+	select {
+	case err := <-done:
+		assert.ErrorContains(t, err, "something went wrong")
+	case <-time.After(10 * time.Second):
+		t.Fatal("PipeCommands was still waiting for a command whose output nothing reads")
+	}
+}

pkg/commands/oscommands/pty_windows.go

@@ -144,9 +144,7 @@ func TerminateLivePtys() {
 // graceful signal worth waiting on — git and the common diff tools leave it
 // to the default handler, which calls ExitProcess at whatever instruction
 // the process happens to execute — so clients that got the event are
-// already dying. Killing at an arbitrary point cannot leak a stale
-// index.lock, because pty-rendered commands don't take that lock (see
-// withPtyGitConfig in pkg/gui/pty.go).
+// already dying.
 //
 // The pseudoconsole close gets its own goroutine because the kill must not
 // wait for it: on builds where ClosePseudoConsole blocks until the console

pkg/config/diff_renderer_config_manager.go

@@ -67,6 +67,7 @@ func (self *DiffRendererConfigManager) GetStdinFilterCommand(width int) string {
 	}
 
 	templateValues := map[string]string{
+		"width":       strconv.Itoa(width),
 		"columnWidth": strconv.Itoa(width/2 - 6),
 	}
 
@@ -87,7 +88,7 @@ func (self *DiffRendererConfigManager) GetColorArg() string {
 	return colorArg
 }
 
-func (self *DiffRendererConfigManager) GetExternalDiffCommand(diffContext uint64) string {
+func (self *DiffRendererConfigManager) GetExternalDiffCommand(diffContext uint64, width int) string {
 	currentDiffRendererConfig := self.currentDiffRendererConfig()
 	if currentDiffRendererConfig == nil || currentDiffRendererConfig.getType() != DiffRendererType_ExtDiff {
 		return ""
@@ -95,6 +96,7 @@ func (self *DiffRendererConfigManager) GetExternalDiffCommand(diffContext uint64
 
 	templateValues := map[string]string{
 		"diffContext": strconv.Itoa(int(diffContext)),
+		"width":       strconv.Itoa(width),
 	}
 
 	return utils.ResolvePlaceholderString(string(currentDiffRendererConfig.Command), templateValues)

pkg/config/diff_renderer_config_manager_test.go

@@ -63,6 +63,95 @@ func TestCurrentDiffRendererName(t *testing.T) {
 	}
 }
 
+func TestGetStdinFilterCommand(t *testing.T) {
+	scenarios := []struct {
+		name               string
+		diffRendererConfig DiffRendererConfig
+		width              int
+		expected           string
+	}{
+		{
+			name:               "a command without template variables is passed through",
+			diffRendererConfig: DiffRendererConfig{Command: "delta --paging=never"},
+			width:              120,
+			expected:           "delta --paging=never",
+		},
+		{
+			name:               "the width the diff is rendered at",
+			diffRendererConfig: DiffRendererConfig{Command: "delta --width={{width}}"},
+			width:              120,
+			expected:           "delta --width=120",
+		},
+		{
+			name:               "the width of one side of a side-by-side rendering",
+			diffRendererConfig: DiffRendererConfig{Command: "ydiff -p cat -w {{columnWidth}}"},
+			width:              120,
+			expected:           "ydiff -p cat -w 54",
+		},
+		{
+			name:               "a template variable can also be written with a leading dot",
+			diffRendererConfig: DiffRendererConfig{Command: "delta --width={{.width}}"},
+			width:              120,
+			expected:           "delta --width=120",
+		},
+		{
+			name:               "nothing is returned for a renderer of another type",
+			diffRendererConfig: DiffRendererConfig{Type: "extDiff", Command: "difft --width={{width}}"},
+			width:              120,
+			expected:           "",
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			userConfig := &UserConfig{}
+			userConfig.Git.DiffRenderers = []DiffRendererConfig{s.diffRendererConfig}
+			config := NewDiffRendererConfigManager(func() *UserConfig { return userConfig })
+
+			assert.Equal(t, s.expected, config.GetStdinFilterCommand(s.width))
+		})
+	}
+}
+
+func TestGetExternalDiffCommand(t *testing.T) {
+	scenarios := []struct {
+		name               string
+		diffRendererConfig DiffRendererConfig
+		expected           string
+	}{
+		{
+			name:               "a command without template variables is passed through",
+			diffRendererConfig: DiffRendererConfig{Type: "extDiff", Command: "difft --color=always"},
+			expected:           "difft --color=always",
+		},
+		{
+			name:               "the width the diff is rendered at",
+			diffRendererConfig: DiffRendererConfig{Type: "extDiff", Command: "difft --width={{width}}"},
+			expected:           "difft --width=120",
+		},
+		{
+			name:               "the width alongside the diff context size",
+			diffRendererConfig: DiffRendererConfig{Type: "extDiff", Command: "difft --width={{width}} --context={{diffContext}}"},
+			expected:           "difft --width=120 --context=3",
+		},
+		{
+			name:               "nothing is returned for a renderer of another type",
+			diffRendererConfig: DiffRendererConfig{Command: "delta --width={{width}}"},
+			expected:           "",
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			userConfig := &UserConfig{}
+			userConfig.Git.DiffRenderers = []DiffRendererConfig{s.diffRendererConfig}
+			config := NewDiffRendererConfigManager(func() *UserConfig { return userConfig })
+
+			assert.Equal(t, s.expected, config.GetExternalDiffCommand(3, 120))
+		})
+	}
+}
+
 func TestCurrentDiffRendererNameWithoutDiffRenderers(t *testing.T) {
 	config := NewDiffRendererConfigManager(func() *UserConfig { return &UserConfig{} })
 

pkg/gui/controllers/branches_controller.go

@@ -206,13 +206,13 @@ func (self *BranchesController) GetOnRenderToMain() func() {
 			} else {
 				cmdObj := self.c.Git().Branch.GetGraphCmdObj(branch.FullRefName())
 
-				ptyTask := types.NewRunPtyTask(cmdObj.GetCmd())
-				task = ptyTask
+				rendererTask := types.NewRunDiffRendererTask(cmdObj.GetCmd())
+				task = rendererTask
 
 				pr, ok := self.c.Model().PullRequestsMap[branch.Name]
 				if ok && presentation.ShouldShowPrForBranch(pr, branch.Name, self.c.UserConfig()) {
-					ptyTask.Prefix = presentation.FormatPullRequestHeader(pr, self.c.Tr)
-					ptyTask.Prefix += strings.Repeat("─", self.c.Contexts().Normal.GetView().InnerWidth()) + "\n"
+					rendererTask.Prefix = presentation.FormatPullRequestHeader(pr, self.c.Tr)
+					rendererTask.Prefix += strings.Repeat("─", self.c.Contexts().Normal.GetView().InnerWidth()) + "\n"
 				}
 			}
 

pkg/gui/controllers/commits_files_controller.go

@@ -177,7 +177,7 @@ func (self *CommitFilesController) GetOnRenderToMain() func() {
 
 		paths := self.pathsForDiff(node)
 		cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, paths, false)
-		task := types.NewRunPtyTask(cmdObj.GetCmd())
+		task := types.NewRunDiffRendererTask(cmdObj.GetCmd())
 
 		self.c.RenderToMainViews(types.RefreshMainOpts{
 			Pair: self.c.MainViewPairs().Normal,

pkg/gui/controllers/files_controller.go

@@ -356,7 +356,7 @@ func (self *FilesController) renderNonTextualConflict(node *filetree.FileNode) {
 			prefix += self.c.Tr.MergeConflictCurrentDiff
 		}
 		prefix += "\n\n"
-		self.renderToMainWithTask(types.NewRunPtyTaskWithPrefix(cmdObj.GetCmd(), prefix))
+		self.renderToMainWithTask(types.NewRunDiffRendererTaskWithPrefix(cmdObj.GetCmd(), prefix))
 		return
 	}
 
@@ -378,7 +378,7 @@ func (self *FilesController) renderWorkingTreeDiff(node *filetree.FileNode) {
 	refreshOpts := types.RefreshMainOpts{
 		Pair: self.c.MainViewPairs().Normal,
 		Main: &types.ViewUpdateOpts{
-			Task:     types.NewRunPtyTask(cmdObj.GetCmd()),
+			Task:     types.NewRunDiffRendererTask(cmdObj.GetCmd()),
 			SubTitle: self.c.Helpers().Diff.IgnoringWhitespaceSubTitle(),
 			Title:    title,
 		},
@@ -395,7 +395,7 @@ func (self *FilesController) renderWorkingTreeDiff(node *filetree.FileNode) {
 		refreshOpts.Secondary = &types.ViewUpdateOpts{
 			Title:    title,
 			SubTitle: self.c.Helpers().Diff.IgnoringWhitespaceSubTitle(),
-			Task:     types.NewRunPtyTask(cmdObj.GetCmd()),
+			Task:     types.NewRunDiffRendererTask(cmdObj.GetCmd()),
 		}
 	}
 

pkg/gui/controllers/helpers/diff_helper.go

@@ -74,11 +74,11 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm
 		}
 		cmdObj := self.c.Git().Diff.DiffCmdObj(args)
 		prefix := style.FgYellow.Sprintf("%s %s-%s\n\n", self.c.Tr.ShowingDiffForRange, from.ShortRefName(), to.ShortRefName())
-		return types.NewRunPtyTaskWithPrefix(cmdObj.GetCmd(), prefix)
+		return types.NewRunDiffRendererTaskWithPrefix(cmdObj.GetCmd(), prefix)
 	}
 
 	cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.FilterPathsForCommit(commit))
-	return types.NewRunPtyTask(cmdObj.GetCmd())
+	return types.NewRunDiffRendererTask(cmdObj.GetCmd())
 }
 
 func (self *DiffHelper) FilterPathsForCommit(commit *models.Commit) []string {
@@ -106,7 +106,7 @@ func (self *DiffHelper) RenderDiff() {
 		self.c.Tr.ShowingGitDiff,
 		"git diff "+strings.Join(args, " "),
 	)
-	task := types.NewRunPtyTaskWithPrefix(cmdObj.GetCmd(), prefix)
+	task := types.NewRunDiffRendererTaskWithPrefix(cmdObj.GetCmd(), prefix)
 
 	self.c.RenderToMainViews(types.RefreshMainOpts{
 		Pair: self.c.MainViewPairs().Normal,

pkg/gui/controllers/reflog_commits_controller.go

@@ -47,7 +47,7 @@ func (self *ReflogCommitsController) GetOnRenderToMain() func() {
 			} else {
 				cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.c.Helpers().Diff.FilterPathsForCommit(commit))
 
-				task = types.NewRunPtyTask(cmdObj.GetCmd())
+				task = types.NewRunDiffRendererTask(cmdObj.GetCmd())
 			}
 
 			self.c.RenderToMainViews(types.RefreshMainOpts{

pkg/gui/controllers/stash_controller.go

@@ -93,7 +93,7 @@ func (self *StashController) GetOnRenderToMain() func() {
 				task = types.NewRenderStringTask(self.c.Tr.NoStashEntries)
 			} else {
 				prefix := style.FgYellow.Sprintf("%s\n\n", stashEntry.Description())
-				task = types.NewRunPtyTaskWithPrefix(
+				task = types.NewRunDiffRendererTaskWithPrefix(
 					self.c.Git().Stash.ShowStashEntryCmdObj(stashEntry.Index).GetCmd(),
 					prefix,
 				)

pkg/gui/controllers/status_controller.go

@@ -146,7 +146,7 @@ func (self *StatusController) editConfig() error {
 
 func (self *StatusController) showAllBranchLogs() {
 	cmdObj := self.c.Git().Branch.AllBranchesLogCmdObj()
-	task := types.NewRunPtyTask(cmdObj.GetCmd())
+	task := types.NewRunDiffRendererTask(cmdObj.GetCmd())
 
 	title := self.c.Tr.LogTitle
 	if i, n := self.c.Git().Branch.GetAllBranchesLogIdxAndCount(); n > 1 {

pkg/gui/main_panels.go

@@ -20,8 +20,8 @@ func (gui *Gui) runTaskForView(view *gocui.View, task types.UpdateTask) error {
 	case *types.RunCommandTask:
 		return gui.newCmdTask(view, v.Cmd, v.Prefix)
 
-	case *types.RunPtyTask:
-		return gui.newPtyTask(view, v.Cmd, v.Prefix)
+	case *types.RunDiffRendererTask:
+		return gui.newRenderTask(view, v.Cmd, v.Prefix)
 	}
 
 	return nil

pkg/gui/main_view_render.go

@@ -0,0 +1,219 @@
+package gui
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"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 {
+	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)
+	}
+
+	// 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 {
+		// The layout may have changed the size of the view, so only now is the
+		// width to render at known, and with it the renderer command.
+		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,
+		}
+		run := gui.ptyRender
+		if rendersThroughAPipe() {
+			run = gui.pipedRender
+		}
+		return gui.newTaskForRender(spec, prefix, cmdStr, run)
+	})
+
+	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, in a pty, or through a pipe with the stdin filter as a command of our
+// own. 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 {
+	setColumnsEnvVar(spec.cmd, spec.width)
+
+	start, onClose := run(spec)
+
+	manager := gui.getManager(spec.view)
+	linesToRead := gui.linesToReadFromCmdTask(spec.view)
+	return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), key)
+}
+
+// renderWithoutPtyEnvVar makes a render take the piped path on a platform that
+// would otherwise use a pty, so that tests can exercise it anywhere.
+const renderWithoutPtyEnvVar = "LAZYGIT_RENDER_WITHOUT_PTY"
+
+// rendersThroughAPipe reports whether a render feeds the diff renderer the
+// command's output through a pipe rather than running it in a pty.
+//
+// On Windows it has to. ConPTY doesn't pass a command's output through; it
+// parses it into a screen buffer and re-encodes that for the terminal side,
+// and it hands a sequence it can't represent there the moment it parses it,
+// separately from the text around it. So what a renderer writes is not what
+// lazygit reads. A pipe carries the bytes as the renderer wrote them.
+//
+// Everywhere else the pty is kept, since a renderer can read the width it
+// should lay out to off it, and a configuration that doesn't name a width would
+// otherwise render at whatever width the renderer falls back to.
+func rendersThroughAPipe() bool {
+	return runtime.GOOS == "windows" || os.Getenv(renderWithoutPtyEnvVar) != ""
+}
+
+// pipedRender feeds the diff renderer the command's output through a pipe.
+//
+// A stdin filter becomes a command of our own here, because git only invokes
+// the one named by GIT_PAGER when it thinks it is talking to a terminal, so
+// with a pipe the filter would never run. An external diff renderer is git's
+// own business, named in the environment and run by git per file, so with one
+// the command runs alone.
+func (gui *Gui) pipedRender(spec renderSpec) (startRender, onCloseRender) {
+	if spec.stdinFilter == "" {
+		return gui.plainRender(spec)
+	}
+
+	view := spec.view
+	cmd := spec.cmd
+
+	var pipe io.ReadCloser
+	start := func() (tasks.Cmd, io.Reader) {
+		// See the matching call in ptyRender for why this happens here.
+		view.SetContentWidth(spec.width)
+
+		// The filter runs in a plain shell, without lazygit's shell functions
+		// sourced, since that is the shell git would have run it in. It is
+		// handed git's environment for the same reason: as git's child it
+		// would have inherited exactly that.
+		pipeline, reader, err := gui.os.StartPipeline(
+			gui.os.Cmd.NewFromCmd(cmd).DontLog(),
+			gui.os.Cmd.NewShell(spec.stdinFilter, "").SetEnviron(cmd.Env).DontLog(),
+		)
+		if err != nil {
+			gui.c.Log.Error(err)
+			// The command has been started and stopped again by now, so it
+			// can't be run a second time without the renderer. Show what went
+			// wrong where the diff would have been.
+			return tasks.ExecCmd{Cmd: cmd}, strings.NewReader(err.Error())
+		}
+		pipe = reader
+		return pipeline, reader
+	}
+
+	onClose := func() {
+		// Closing the reader brings the pipeline down. The renderer's next write
+		// fails, so it exits, and git's write into the pipe the renderer was
+		// reading fails in turn.
+		if pipe != nil {
+			pipe.Close()
+			pipe = nil
+		}
+	}
+
+	return start, onClose
+}
+
+// setColumnsEnvVar tells a command how wide the view its output goes into is.
+// git reads COLUMNS in preference to the size of the terminal it is talking to,
+// and lays the diffstat graph out to it; a diff renderer with no terminal to
+// ask may read it too (difftastic and diff-so-fancy do, delta does not). A
+// command told nothing renders for 80 columns.
+func setColumnsEnvVar(cmd *exec.Cmd, width int) {
+	cmd.Env = append(cmd.Env, fmt.Sprintf("COLUMNS=%d", width))
+}
+
+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_")
+}

pkg/gui/pty.go

@@ -1,20 +1,14 @@
 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,156 +47,61 @@ 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()
-
-	// 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 pty after layout so that it gets the correct size
-	gui.afterLayout(func() error {
-		// Need to get the width and the pager command again because the layout might have
-		// changed the size of the view
-		width = view.InnerWidth()
-		pager := gui.stateAccessor.GetDiffRendererConfigManager().GetStdinFilterCommand(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)
-
-		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
-		}
-
-		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
-}
-
-// withPtyGitConfig returns args with extra git configuration for commands
-// that render into a pty. On Windows, such a command is terminated at an
-// arbitrary point of its execution when its task stops: tearing down the
-// pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the default
-// handler, which just calls ExitProcess. git's automatic index refresh
-// (diff.autoRefreshIndex, on by default) takes index.lock at the end of a
-// diff against the worktree to write back refreshed stat information —
-// GIT_OPTIONAL_LOCKS does not cover this lock — and a termination landing
-// in that window leaves a stale index.lock behind that the next git command
-// chokes on. So don't let pty-rendered commands refresh the index;
-// lazygit's foreground `git status` refreshes, which never run in a pty,
-// keep the stat cache fresh instead.
+// 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.
 //
-// On Unix a stopped pty child gets SIGTERM, and git's signal handlers remove
-// its lock files, so the refresh can stay enabled there and keep healing
-// stale stat info.
-func withPtyGitConfig(args []string, goos string) []string {
-	if goos != "windows" {
-		return args
-	}
-	// Most pty commands are direct git invocations, but the user-configured
-	// ones can be arbitrary command lines (e.g. a branchLogCmd wrapping git
-	// in `sh -c`), and injecting git flags into those would corrupt them.
-	// Only direct git invocations get the config; that loses nothing, since
-	// the wrapped commands are log commands, which never take the index
-	// lock. (For direct invocations other than worktree diffs the config is
-	// simply a no-op.)
-	base := strings.TrimSuffix(strings.ToLower(filepath.Base(args[0])), ".exe")
-	if base != "git" {
-		return args
-	}
-	result := make([]string, 0, len(args)+2)
-	result = append(result, args[0])
-	result = append(result, "-c", "diff.autoRefreshIndex=false")
-	return append(result, args[1:]...)
-}
+// 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
 
-func removeExistingTermEnvVars(env []string) []string {
-	return lo.Filter(env, func(envVar string, _ int) bool {
-		return !isTermEnvVar(envVar)
-	})
-}
+	// 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)
 
-// 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_")
+	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
+	}
+
+	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()
+	}
+
+	return start, onClose
 }

pkg/gui/pty_test.go

@@ -1,29 +0,0 @@
-package gui
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestWithPtyGitConfig(t *testing.T) {
-	args := []string{"git", "-C", "/repo", "diff", "--color=always"}
-
-	assert.Equal(t,
-		[]string{"git", "-c", "diff.autoRefreshIndex=false", "-C", "/repo", "diff", "--color=always"},
-		withPtyGitConfig(args, "windows"))
-
-	assert.Equal(t, args, withPtyGitConfig(args, "linux"))
-	assert.Equal(t, args, withPtyGitConfig(args, "darwin"))
-
-	// A user-configured command that wraps git in a shell must not have git
-	// flags injected into it.
-	shellArgs := []string{"sh", "-c", "git log --graph {{branchName}} -- | sed -e s/x/y/"}
-	assert.Equal(t, shellArgs, withPtyGitConfig(shellArgs, "windows"))
-
-	// The guard recognizes git regardless of case and extension.
-	exeArgs := []string{"GIT.EXE", "diff"}
-	assert.Equal(t,
-		[]string{"GIT.EXE", "-c", "diff.autoRefreshIndex=false", "diff"},
-		withPtyGitConfig(exeArgs, "windows"))
-}

pkg/gui/tasks_adapter.go

@@ -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

pkg/gui/types/rendering.go

@@ -84,17 +84,17 @@ func NewRunCommandTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunCommandTask {
 	return &RunCommandTask{Cmd: cmd, Prefix: prefix}
 }
 
-type RunPtyTask struct {
+type RunDiffRendererTask struct {
 	Cmd    *exec.Cmd
 	Prefix string
 }
 
-func (t *RunPtyTask) IsUpdateTask() {}
+func (t *RunDiffRendererTask) IsUpdateTask() {}
 
-func NewRunPtyTask(cmd *exec.Cmd) *RunPtyTask {
-	return &RunPtyTask{Cmd: cmd}
+func NewRunDiffRendererTask(cmd *exec.Cmd) *RunDiffRendererTask {
+	return &RunDiffRendererTask{Cmd: cmd}
 }
 
-func NewRunPtyTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunPtyTask {
-	return &RunPtyTask{Cmd: cmd, Prefix: prefix}
+func NewRunDiffRendererTaskWithPrefix(cmd *exec.Cmd, prefix string) *RunDiffRendererTask {
+	return &RunDiffRendererTask{Cmd: cmd, Prefix: prefix}
 }

pkg/integration/tests/diff/render_through_a_pipe.go

@@ -0,0 +1,49 @@
+package diff
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var RenderThroughAPipe = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "A stdin filter renders the diff when it is fed through a pipe rather than run in a pty",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	// This is how a render works on Windows, where a pty can't carry a
+	// renderer's output faithfully. Ask for it here so that the path is
+	// covered on the platforms the integration tests do run on.
+	ExtraEnvVars: map[string]string{"LAZYGIT_RENDER_WITHOUT_PTY": "1"},
+	SetupConfig: func(cfg *config.AppConfig) {
+		// Reports the width it was given, then passes the diff through. git
+		// only runs a filter of its own when it talks to a terminal, so the
+		// filter running at all says the pipeline was built here.
+		cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{
+			{Command: `echo "rendered at {{width}} columns"; cat`},
+		}
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("file1", "one\n")
+		shell.Commit("one")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("one").IsSelected(),
+			)
+
+		t.Views().Main().
+			// The width reaches the filter on its command line, since with no
+			// terminal it has nowhere to read it from.
+			Content(MatchesRegexp(`rendered at \d+ columns`)).
+			ContainsLines(
+				Equals("diff --git a/file1 b/file1"),
+				Contains("new file mode"),
+				Contains("index "),
+				Equals("--- /dev/null"),
+				Equals("+++ b/file1"),
+				Equals("@@ -0,0 +1 @@"),
+				Equals("+one"),
+			)
+	},
+})

pkg/integration/tests/diff/stat_uses_the_view_width.go

@@ -0,0 +1,42 @@
+package diff
+
+import (
+	"strconv"
+	"strings"
+
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var StatUsesTheViewWidth = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "The diffstat of a commit is laid out to the width of the view showing it",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(cfg *config.AppConfig) {
+		// git's own diff, so that the render runs as a plain command rather
+		// than through a renderer.
+		cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{
+			{Type: "rawGit"},
+		}
+	},
+	SetupRepo: func(shell *Shell) {
+		// Enough added lines that git scales the graph to the width it has,
+		// rather than drawing one mark per line.
+		lines := make([]string, 200)
+		for i := range lines {
+			lines[i] = "line " + strconv.Itoa(i)
+		}
+		shell.CreateFileAndAdd("file1", strings.Join(lines, "\n")+"\n")
+		shell.Commit("add file1")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("add file1").IsSelected(),
+			)
+
+		t.Views().Main().
+			Content(MatchesRegexp(`(?m)^ file1 \| 200 \+{80,}$`))
+	},
+})

pkg/integration/tests/test_list.go

@@ -230,6 +230,8 @@ var tests = []*components.IntegrationTest{
 	diff.DiffNonStickyRange,
 	diff.IgnoreWhitespace,
 	diff.RenameSimilarityThresholdChange,
+	diff.RenderThroughAPipe,
+	diff.StatUsesTheViewWidth,
 	file.ClickArrowToCollapse,
 	file.CollapseExpand,
 	file.CopyMenu,