Repository: jesseduffield/lazygit (language: go)

Issue:
Let custom pagers detect lazygit
Hi folks 👋🏼 

### Background

I have recently implemented automatic color detection in [delta] as part of dandavison/delta#1615.
This involves sending an escape sequence (`OSC 10` / `OSC 11`) to the terminal and waiting for a response.
Delta uses the [`terminal-colorsaurus`] library for this.

There are a couple of mechanisms in place to detect terminals that don't support this escape sequence to prevent waiting forever. Quoting myself from https://github.com/dandavison/delta/issues/1664#issuecomment-2015355094:
> - The DA1 trick [^1] which works for most terminals emulators
> - If the environment variable `TERM` is `dumb`, do nothing (This originates from [NCURSES](https://invisible-island.net/ncurses/terminfo.src.html#tic-dumb))
> - Finally a 1 second timeout. This is intentionally quite high as to not bail out too early for high-latency situations such as when connected via SSH.

From what I can tell lazygit runs git (and delta) [inside a PTY][pty]. This PTY however does not respond to queries (such as `OSC 10` or `DA1`). It also does not identify itself via an environment variable such as `TERM` or similar means.

For [delta] this currently means that we hit the timeout which we are trying to prevent as it is the last resort mechanism.

### Feature Request

Would you be open to changing lazygit's PTY to identify itself somehow?

My preferred mechanism would be setting the environment variable `TERM=dumb` [^2] since `terminal-colorsaurus` (the library that delta uses) already detects this and other libraries / applications do too.

From some preliminary testing git still starts the pager when `TERM=dumb` which seems to be the primary reason for using a PTY in the first place.


[^1]: Colorsaurus sends two escape sequences: OSC 11 (the actual color querying sequence) followed by DA1 (which is supported by almost all terminals out there). Since terminals answer in the same order as the sequences were received we know that if we receive the answer to DA1 then the terminal does not support OSC 11 and can bail out early and avoid a long timeout.
[^2]: It might make sense in that case to also clear out `TERM_PROGRAM` and `TERM_PROGRAM_VERSION` if they exist.

[delta]: https://github.com/dandavison/delta
[`terminal-colorsaurus`]: https://github.com/bash/terminal-colorsaurus
[pty]: https://github.com/jesseduffield/lazygit/blob/e1c3ef66295bf949de5e7781c0a60ac7f79c47ce/pkg/gui/pty.go#L45



Discussion / hints:
Sounds related to #3190, #2621, #3405.

I have no objections to setting `TERM=dumb` for our PTY if that helps and doesn't have negative consequences (I can't tell, no experience with this kind of stuff).

Would you be up for making a PR?

Fix patch:
diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go
index 3f59373b71c..cf3176f7505 100644
--- a/pkg/gui/pty.go
+++ b/pkg/gui/pty.go
@@ -12,6 +12,7 @@ import (
 	"github.com/creack/pty"
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/samber/lo"
 )
 
 func (gui *Gui) desiredPtySize() *pty.Winsize {
@@ -54,6 +55,13 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 
 	cmdStr := strings.Join(cmd.Args, " ")
 
+	// This communicates to pagers 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)
@@ -87,3 +95,20 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 
 	return nil
 }
+
+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_")
+}


Test patch: