-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect Dark/Light Mode from Terminal #1615
Conversation
Hi @bash, this looks really interesting! I don't think I quite have it working in my set up. It is correctly detecting my light-background terminal (i.e. I have removed all env vars and git config settings that would influence it, and on your branch it is correctly using GitHub theme for a light background whereas on master it's using the dark theme despite my terminal being light). However, when I comment out my alacritty color settings so that my terminal has alacritty's default dark background, it continues to use the GitHub theme. Sorry if I've messed up my testing which is possible. Do you have any concerns about added startup latency? (Do you have any timings?) This is the main/only thing I want us to be sure about regarding this work. Obviously delta ideally wants to always have imperceptible start up time, even on not so high-end machines. As a tangential comment / for follow up, I think that delta does a few disk I/O things synchronously at start up. I wonder whether there would be latency wins in doing more things concurrently. I haven't followed the technical details of terminal color detection closely. I did look into it at one point and couldn't quite see / was confused about how delta could do it given that it was accepting stdin from git and outputting stdout to less. In practice, does your comment
point to any meaningful limitations? To start with the basic/dumb question, I believe it still works even when delta's output is going to less, right? Can you give me a little bit of hand-holding regarding how this is working at a technical level? Is the point that you're writing to / reading from the tty before delta has started redirecting its output to the pager? |
If it's the case that terminal-colorsaurus is solving limitations of existing crates (e.g. termbg) such that color detection for delta/bat etc is feasible now when it was not before, would you consider opening a PR against bat also (would fix sharkdp/bat#1746) so that both projects benefit and reviewing expertise / problem ironing-out is shared? cc @eth-p @sharkdp. |
Thank you @dandavison for testing my changes :) Since opening this PR, I did some more testing and decided to change a few things:
Prompted by your comment, I decided to collect some measurements regarding terminal latency.
I will happily try to do that. Please note that I have learned a lot of these things over the past few weeks by reading man pages and forum answers, so take everything I say with a grain of salt :) Querying the terminal for its colors involves communicating with the controlling terminal. This is a bidirectional stream that can be acquired by opening I think this is where the confusion might come from: In the case where all standard streams point to the terminal, one can write to stdout and read the terminal's response from stdin. Where does the race with the pager come from then? Usually, delta is the one to start the pager after processing its settings / querying the terminal for its colors. In that case we don't get a race. The race occurs when the user manually pipes delta's output to a pager, e.g. by running
|
Hi @bash, thanks a lot for the explanation (and sorry for the high-latency conversation). I'm still finding this:
(Would you mind rebasing / merging |
* Remove support for Windows. * Remove preconditions from public API
2c2933c
to
72c5538
Compare
Hi @dandavison Sure thing, I have rebased the PR :) Regarding your setup: Are you using tmux by any chance? In that case tmux is the one to answer the color query. tmux in turn queries the terminal but iirc caches the result. So if the terminal changes its colors during runtime tmux will unfortunately not know about that. |
Ah-ha, that's right, I'm using tmux. Do you think they might be amenable to a PR allowing that caching to be optionally disabled? tmux isn't released often but I assume lots of people (like me) install it from source -- we have to for hyperlink support currently -- so having a patch to enable your terminal color detection would very useful. Also, I think the bat project would benefit from your work just as much as delta (and their architecture w.r.t. the interaction between the Rust app and the pager etc is essentially identical to delta's because delta copied what they do). sharkdp/bat#1746 is their issue that your work would seem to solve. |
I think they already react to color changes, but the only terminal that I know of with a mechanism to notify processes of color changes is iTerm 2 1. Here's the tmux issue for reference.
Thank you for the encouragement, I think I'll cook up a PR for bat then :) Footnotes
|
Oh, that's a mistake 👀 I decided to remove windows support at some point because the winapi calls unfortunately report incorrect colors for most users (details here). I have updated the original PR description with a remark. Colorsaurus returns |
@dandavison I see that the tests are failing on Windows, was this already the case before or did I break them? |
I've merged this, thanks a lot @bash for the work! (including all the clear explanations and code comments). (The Windows test failure isn't your fault; there's a race condition in some tests that access env vars but are running concurrently that hasn't been fixed yet) |
Did a quick check in Konsole and it appears to be working for me. |
Nice, thanks @aykevl -- it's really useful to have some extra confirmation on different setups for this sort of feature. |
thank you everyone |
`Delta` now automatically detects light/dark theme based on the terminal background color. This change was done in dandavison/delta#1615 and released in `0.17.0`. This means that the git config theme detection can be significantly simplified. Another benefit is that it now works across different systems which don't use `gsettings` (WSL, Windows, etc.).
`Delta` now automatically detects light/dark theme based on the terminal background color. This change was done in dandavison/delta#1615 and released in `0.17.0`. This means that the git config theme detection can be significantly simplified. Another benefit is that it now works across different systems which don't use `gsettings` (WSL, Windows, etc.).
Use the
terminal-colorsaurus
crate to detect light/dark mode if neither--light
nor--dark
is specified.I was unsatisfied with existing crates (such as
termbg
) and so I went down the rabbit hole of making my own crate for this. It usesOSC 10
/OSC 11
on Unixand winapi calls on Windows.Edit: I have since removed Windows support for the reasons outlined hereMy crate has a few advantages over existing crates that do this:
To avoid race conditions when piped into a pager as mentioned in a comment in this previous attempt color detection is disabled when stdout is attached to a pipe.