Closed svanharmelen closed 2 months ago
Hi @svanharmelen :wave: Thank you for your bug report!
The culprit here is auto-dark-light detection introduced in #1615 (by me, sorry πΌπΌ)
From a quick glance the issue seems to be that lazygit pretends to be a terminal, but doesn't respond to escape sequences which causes the long delay.
You can work around this by passing either the --light
or --dark
flags to delta which disables automatic detection.
Hi @bash π
Thanks for the update/info! Adding --dark
indeed "fixed" the problem.
I'm good with closing this issue as it seems clear what is happening and a related PR is already open. But I'll leave that up to you, also fine to keep it open until a fix is actually merged.
Thanks!
I would be keen to know what @dandavison thinks, but I would like to keep this issue open and investigate a bit further :)
@bash what do you think about modifying terminal-colorsaurus
to accept a deadline parameter: if it cannot obtain a response from the terminal/tty within this time budget then it returns an error. Then, in delta we set that so that either the auto-detection is imperceptible, or we guess dark
.
The trouble with slow response times is obviously that people have no idea that it's related to color detection. Wrong colors on the other hand will hopefully send them to --help
or the README or manual where they'll find --light
and --dark
.
@dandavison terminal-colorsaurus
already has mechanisms for detecting if a terminal will answer or not:
TERM
is dumb
, do nothing (This originates from NCURSES)I wonder how many other applications such as layzgit are out there that run a process in a pty but neither support DA1 nor have an appropriate TERM
set.
My current goal is to write up an issue for layzgit and ask if they'd be up to setting TERM=dumb
(or a different mechanism that let's us detect such a situation)
and hope that there aren't that many terminals / apps out there that fit that category :)
[^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.
@bash, OK I see, what I was thinking was
From a quick glance the issue seems to be that lazygit pretends to be a terminal, but doesn't respond to escape sequences which causes the long delay.
could we make it so that this is guaranteed never to happen in delta, i.e. it will always guess dark
after a few milliseconds? (not sure the appropriate value yet). I know lazygit is very popular, and there are other popular ways of executing delta in non-standard contexts. For the ssh case, we could make the timeout configurable in delta settings; for me what's critical is that if auto-detection is going to be the default that it should always be imperceptible.
[...] if auto-detection is going to be the default that it should always be imperceptible
@dandavison I absolutely agree with this goal.
The issue with timing out is that we can't "cancel" our request so terminals that support querying the colors will still respond some time in the future.
This means that whoever reads from the terminal next (in delta's case the pager) receives the terminal's response leading to unexpected effects.
Here's what happens in my that case with less:
You can easily reproduce this by setting the timeout to zero and running:
GIT_PAGER=target/debug/delta git show HEAD
diff --git a/src/options/theme.rs b/src/options/theme.rs
index f53a082..355dcb2 100644
--- a/src/options/theme.rs
+++ b/src/options/theme.rs
@@ -133,7 +133,10 @@ fn detect_light_mode() -> bool {
return value;
}
- color_scheme(QueryOptions::default())
+ let mut options = QueryOptions::default();
+ options.timeout = std::time::Duration::ZERO;
+
+ color_scheme(options)
.map(|c| c.is_dark_on_light())
.unwrap_or_default()
}
@dandavison I see from the lazygit docs that delta is configured with --paging=never
.
Maybe we could use that as a hint to not also not detect colors in that case. I imagine that if delta is used in similar situations that --paging=never
is also passed.
This means that whoever reads from the terminal next (in delta's case the pager) receives the terminal's response leading to unexpected effects.
Ah-ha! I didn't think about that. Thanks.
Maybe we could use that as a hint to not also not detect colors in that case. I imagine that if delta is used in similar situations that --paging=never is also passed.
Interesting. I definitely see the thinking behind that. And you already have --detect-dark-light=always
for people who want --paging=never
with auto-detection. So the docs would be modified something like
auto
enables auto-detection unless either delta's output is being redirected, or the user has disabled paging (--paging=never
). The reason for the first exception is that terminal color detection encounters a race condition when output is redirected to a pager; the reason for the second exception in that if the user has disabled paging then this suggests that delta might be running in a TUI or other non-standard terminal environment.
As of https://github.com/jesseduffield/lazygit/pull/3420 (only on master, not released yet) this is now fixed from the side of lazygit.
With this change delta no longer tries to detect the terminal's theme when run inside lazygit.
foss code being slower than in the last version must be triggering, given the current events surrounding the xz compression lib :smile:
11;?[c
^^^ Putting the "weird" characters here for anyone like me who'll search for this issue in GitHub, hopefully they'll have an easier time tracking it down to this issue :)
As of the latest lazygit release https://github.com/jesseduffield/lazygit/releases/tag/v0.42.0, no workarounds are needed anymore :)
I'm using delta through layzgit and I noticed a fair bit of delay when rendering the diff. From almost instant to first getting the
loading...
from lazygit before actually seeing the diff.I also noticed a few weird looking chars in the diff, not sure if that is related:
I downgraded to 0.16.5 and things run smooth as butter again.