🐛 0.17.0 seems much slower then 0.16.5

[svanharmelen]

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.

[tautropfli]

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.

[svanharmelen]

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!

[tautropfli]

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

[dandavison]

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

[tautropfli]

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

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

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.

[dandavison]

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

[tautropfli]

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

[tautropfli]

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

[dandavison]

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.

[tautropfli]

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.

[owzim]

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

[bezhermoso]

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

[tautropfli]

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