search

dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
21.82k stars 364 forks source link

Try to obtain default dark/light state from the terminal #1493

Closed aykevl closed 1 year ago

aykevl commented 1 year ago

Use the termbg crate to try to read the current terminal background theme (light or dark) and use it as the default if no --light or --dark was specified (or light=true in .gitconfig).

This doesn't work in all cases: delta is frequently used with a pipe, in which case it's not really possible to obtain the background color using OSC 11. The fallback is the environment variable COLORFGBG which is supported by most terminals, but it doesn't immediately update when the system theme changes (it only applies to newly opened terminals). Still, I think this is better than assuming a dark background.

See https://github.com/dandavison/delta/issues/447 for discussion.

This PR is not ready yet. There are two problems:

  1. termbg doesn't check whether stdin/stderr are connected to a terminal. This is fixed in https://github.com/dalance/termbg/pull/19 but isn't merged yet.
  2. Lots of tests fail with this PR. I think I'm going to need a bit of help here. The issue is that the tests also try to read from the current terminal, while they shouldn't. Honestly the only place that should be querying the terminal should be somewhere in main, certainly not in a test. One way to fix this would be to provide some sort of mock terminal background read function, but my Rust knowledge is rather limited and I'm not sure how this is best implemented.
aykevl commented 1 year ago

I have been investigating this problem some more and it looks like it's fundamentally impossible to request the background color from the terminal when the output is piped to less.

The main call here is "OSC 11". I don't know where it is documented, but this is one place that describes it. To be able to read the response from the terminal, two flags need to be disabled: ECHO and ICANON (see man tcsetattr). This avoids showing the response in the terminal and makes it possible to read it (without waiting for a newline first). The termbg crate sets the terminal to raw mode, this is a bit overkill but also removes those two flags (among many others). The problem here is that less also changes the terminal state: it changes these flags manually but basically it also sets it to raw mode, including ECHO and ICANON. So what happens is that there is essentially a race condition between termbg and less. For example, termbg sets the terminal to raw mode, then less sets it to raw mode, and then termbg disables raw mode (making less unusable). I've been thinking about a solution to this, but I don't see a way to avoid the race condition entirely.

What would work, is reading the COLORFGBG environment variable. This is set by a number of terminals, apparently including Konsole, iTerm2, and the built-in macOS terminal. The problem with that one is that it doesn't respond to changes in the color scheme, but it will be correct for newly created terminals. Perhaps that would be better than assuming a dark background in all cases?

aykevl commented 1 year ago

Closing, I don't think this is the right approach.