search

jesseduffield / lazygit

simple terminal UI for git commands
MIT License
52.65k stars 1.84k forks source link

Let custom pagers detect lazygit #3419

Closed bash closed 7 months ago

bash commented 7 months ago

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)
  • 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. 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.

stefanhaller commented 7 months ago

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?

bash commented 7 months ago

Would you be up for making a PR?

Sure thing! I just opened #3420

georgeguimaraes commented 7 months ago

Nice work here! I didn't know we could use delta inside lazygit. Do you know if we can disable side by side in delta only in lazygit? my delta config in git enables side by side by default, but I don't think it's a good visualization inside lazygit.

bash commented 7 months ago

Do you know if we can disable side by side in delta only in lazygit?

I think "Features" might help here

[delta "lazygit"]
side-by-side = false

and then in your lazygit config add --features +lazygit to the delta flags.

I haven't tested this though :)

georgeguimaraes commented 7 months ago

Tks! Got it working, but I had to do it in the other way around:

[core]
   pager = DELTA_FEATURES=+side-by-side delta

Sorry to hijack the issue here, but maybe you can shed some light on this issue I got here: https://github.com/jesseduffield/lazygit/issues/3362#issuecomment-2018905965