OS X: Garbage in search area when launched through git hook

[zeenix]

We solved this issue for Linux through the addition of this line:

exec < /dev/tty

in the commit hook shell script but seems it does not help on the Mac OS X. :(

[zeenix]

Thanks to @kdheepak, for giving me some clues on the ratatui matrix:

I know exactly what this is 🙂 I was experiencing the same thing in taskwarrior-tui

https://github.com/kdheepak/taskwarrior-tui/issues/46

The short version is that the "garbage" you are seeing is the terminal sending back on stdin the terminal background color in RGB.

I haven't gone through your code exactly to see how it works (cool idea btw!) but if you try adding a sleep() to the prepare-commit-msg commit hook before your app starts, you should stop seeing this.

There is a workaround that I eventually landed on that prevented this from entirely happening even if I removed all sleep() in and outside taskwarrior-tui. But that was due to the way I was structuring my app and may not apply to you.

[zeenix]

I tried the sleep workaround but had no success. :(

[kdheepak]

I was able to fix the issue:

https://github.com/zeenix/gimoji/assets/1813121/6fa45669-2125-48bd-98de-51c26ffaf6a9

I was having the issue before my changes and I don't get the issue any more. Here's the patch:

diff --git a/.rustfmt.toml b/.rustfmt.toml
new file mode 100644
index 0000000..e69de29
diff --git a/src/main.rs b/src/main.rs
index 6aa8bbf..0f13919 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -105,9 +105,14 @@ fn main() -> Result<(), Box<dyn Error>> {
 fn select_emoji() -> Result<Option<String>, Box<dyn Error>> {
     let emojis = &emoji::EMOJIS;

-    let colors = match terminal_light::luma() {
-        Ok(luma) if luma > 0.6 => Colors::light(),
-        _ => Colors::dark(),
+    let colors = if let Ok(s) = std::env::var("GITMOJI_COLOR_SCHEME") {
+        match s.as_str() {
+            "light" => Colors::light(),
+            "dark" => Colors::dark(),
+            _ => Colors::dark(),
+        }
+    } else {
+        Colors::dark()
     };

     let mut terminal = Terminal::setup()?;

I added a empty .rustfmt.toml so that my global settings wouldn't clobber your project. You probably should add an empty .rustfmt.toml to your project too.

The issue is because of terminal_light.luma(). To get the background color, terminal_light writes to stdout but it needs to wait for stdin to respond. Some terminals can take longer to respond than others. terminal_light doesn't appear to wait for stdin, and returns immediately. Then you end up starting the TUI that reads stdin for key presses, and then the terminal sends the background color on stdin. The TUI reads those as if a user is typing those letters and it ends up as "garbage".

Your options are:

1. Use an environment variable to allow users to change color scheme. This is the simplest solution. Generally, if you use the named colors in ratatui::style::Color (like you are doing) you can use the same color scheme for both light and dark scheme. Alternatively, add a Colors::default() like below and use that by default.

    pub fn default() -> Self {
        Self {
            selected: Color::Yello,
            unselected: Color::DarkGray,
            border: Color::DarkGray,
        }
    }

The patch I made uses just light() and dark() and sets dark() as default:

    let colors = if let Ok(s) = std::env::var("GITMOJI_COLOR_SCHEME") {
        match s.as_str() {
            "light" => Colors::light(),
            "dark" => Colors::dark(),
            _ => Colors::dark(),
        }
    } else {
        Colors::dark()
    };

2. Submit an issue to terminal_light and ask them to implement a sleep or a poll for reading stdin. Preferably they would make it user configurable. In my opinion, it should wait indefinitely until it gets a response from stdin. However, different terminals have different ways of getting the background color (the non standard modern ones can respond immediately iirc), so maybe there's something they are doing based on that? I haven't looked at their implementation.

3. Implement your own ascii codes for requesting the background color from the terminal, polling stdin till you get a response and then reading stdin and converting that to ratatui's Color::Rgb.

I tend to prefer option 1 for my projects :)

Here's a reproducible test you can share with the terminal_light developers. Make your main function just these lines of code:

fn main() -> Result<(), Box<dyn Error>> {
  let colors = match terminal_light::luma() {
    Ok(luma) if luma > 0.6 => Colors::light(),
    _ => Colors::dark(),
  };
  std::thread::sleep(std::time::Duration::from_secs(10));
  Ok(())
}

And run git commit -a. Here's what I get:

They probably will have a better idea of how to solve this problem since this is not an area I'm familiar with.

[zeenix]

@kdheepak Thanks so much for looking into this and proposing multiple possible solutions in detail as well. :pray:

terminal_light doesn't appear to wait for stdin, and returns immediately.

Actually, according to its README, it does wait for 20ms. I thought of filing an issue to get it bumped up to a value that would work for iterm2 (the terminal I can reproduce this issue on) but as I mentioned, gimoji intends to be as fast as possible and 20ms is already too long IMO.

So while autodetection would be great, I think your suggestion about a env variable might be the best option here. I can also add cmdline params to specify the theme, add that to the generated git hook and maybe even make this parameters mandatory with -i option (that tells gimoji to install the git hook).

[zeenix]

terminal_light doesn't appear to wait for stdin, and returns immediately.

Actually, according to its README, it does wait for 20ms.

LOL and looking at the code, it seems the timeout is at 100ms. That's quite a long time. Wonder what's up with iterm2. :thinking: Is it possible that iterm2 just doesn't implement this xterm query and ignores the query?

[kdheepak]

Iterm2 responds very quickly:

Here's a python program that reads the background color and prints it out:

import sys
import os
import termios
import tty

def set_terminal_raw_mode():
    # Save the current terminal settings
    original_settings = termios.tcgetattr(sys.stdin)

    # Set the terminal to raw mode
    tty.setraw(sys.stdin)

    return original_settings

def restore_terminal_settings(original_settings):
    # Restore the original terminal settings
    termios.tcsetattr(sys.stdin, termios.TCSADRAIN, original_settings)

def query_background_color():
    # Save original terminal settings
    original_settings = set_terminal_raw_mode()

    try:
        sys.stdout.write("\033]11;?\a")
        sys.stdout.flush()
        response = os.read(sys.stdin.fileno(), 1024)

    finally:
        # Restore original terminal settings
        restore_terminal_settings(original_settings)

    return response.strip()

if __name__ == "__main__":
    background_color = query_background_color()
    print(f"The current background color is: {background_color}")

I think the problem is that terminal_light is not changing the termios settings to put the terminal into raw. They have to do that to read from stdin, if they don't it'll spit the response back to the terminal's stdin (which is typically mirrored to stdout) and you'll see it in your terminal. They will have to restore the terminal as back to the old settings if they do this.

Again, I haven't looked at their code so I'm just guessing as to what the problem is here :)

[kdheepak]

I looked through their code briefly and it seems they are doing exactly what I mentioned in the comment. I think the problem is in xterm_query, and it is not behaving correctly on MacOS OR not behaving as intended in ITerm2. You might want to open an issue there: https://github.com/Canop/xterm-query

[zeenix]

I looked through their code briefly and it seems they are doing exactly what I mentioned in the comment.

:+1:

I think the problem is in xterm_query, and it is not behaving correctly on MacOS OR not behaving as intended in ITerm2.

Ah ok, thanks again for investigating. xterm-query is from the same person so they can just move the issue. I'll inform them of this development.

[zeenix]

Looks like we don't have much choice but to remove the luma detection, in favor of defaulting to dark and providing a cli arg and env to override the default. :(