🐛 0.17.0 seems much slower then 0.16.5 about delta HOT 13 CLOSED

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!

from delta.

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 :)

from delta.

Hi @svanharmelen 👋 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.

from delta.

I would be keen to know what @dandavison thinks, but I would like to keep this issue open and investigate a bit further :)

from delta.

@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.

from delta.

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.

from delta.

As of the latest lazygit release https://github.com/jesseduffield/lazygit/releases/tag/v0.42.0, no workarounds are needed anymore :)

from delta.

@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.

from delta.

@dandavison terminal-colorsaurus already has mechanisms for detecting if a terminal will answer or not:

• The DA1 trick ¹ 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.

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 :)

from delta.

@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.

from delta.

[...] 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()
 }

from delta.

As of jesseduffield/lazygit#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.

from delta.

foss code being slower than in the last version must be triggering, given the current events surrounding the xz compression lib 😄

from delta.