Query Kitty Protocol support

[hasezoey]

This PR refactors the way kitty protocol support is acquired by querying the terminal for both remote & local(file) support.
This should be more robust than check environment variables.

Also implements a fix for the race-condition of viuer removing the file before the terminal can finish reading it, leading to empty viu displays (in #79 this was fixed via i= and waiting for OK, but here it is solved via DSR, #79 (comment))

Depends on #85 and #87
Re #79
fixes #70
fixes #67
fixes #71

[atanunq]

It would be fantastic if we could reliably test remote support like that but some terminals don't respond to this type of sequence and it ends up displayed before the image's blocks. That's what happens on urxvt. However, seems like most modern emulators don't do that and we might be able to get away with it. Will check a few more tomorrow

[hasezoey]

and it ends up displayed before the image's blocks.

What exactly do you mean with this? Do you mean that the escape sequence gets echoed to the terminal's output? (FYI: i know that kitty somehow echoes back the second part of the escape sequence(^[[?62;52;c) sometimes, i do not understand why, even though it is literally recommended to be done by kitty itself)
If that is the case, then to my knowledge, that is a wrong implementation of basic escape sequence handling and we shouldnt really special case that.
In any case, this extra handling, if necessary, should be able to be put in another PR instead of blocking this one.

Kitty 0.42.2

[atanunq]

Precisely, before the block characters are displayed. I mentioned it here because before this PR such escape sequences were sent only if well-known env vars are present.

[hasezoey]

Precisely, before the block characters are displayed.

I still dont quite understand what you mean. Do you mean this?

(the ^[[?62;52;c before the image)

[atanunq]

hm, no, but this is also not ideal. I'll send you a picture once I am back on my PC.

[hasezoey]

I think i have found the problem with the screenshot:
We write to the stdout, but dont set raw mode, so characters have a chance of being printed.
Term::read_key activates raw mode, but this happens after we already send it to stdout. It does not seem to be possible to fix this with console::Term itself, but a workaround is using crossterm::terminal::enable_raw_mode (and explicitly disable it again).
Though from what i can tell, we would need to wrap every escape code + read in that (not just for kitty)

And while we are at it, why not just completely use crossterm at that point?

I would like to do that in another PR, or should it be part of this PR?

[atanunq]

I'm sorry if this is taking too long to be merged but I'd prefer to keep master publishable. That's also why I went with this PR, instead of #87. If we start using local printing we should fix it at the same time.

This is what I see in urxvt:

Good points about raw mode, it's probably the same reason. I spent a good amount of time trying to get it to work with raw mode, using alternate screens (viuer is used in tmux, vim plugins, etc. I'm afraid to use that buffer) or other hacks to hide the output. Unless you manage to get it to work without putting too much effort in it, we can give it a shot as-is and revisit if it becomes an issue.

[hasezoey]

Unless you manage to get it to work without putting too much effort in it, we can give it a shot as-is and revisit if it becomes an issue.

My testing patch was:

diff --git a/src/printer/kitty.rs b/src/printer/kitty.rs
index 511e475..576c797 100644
--- a/src/printer/kitty.rs
+++ b/src/printer/kitty.rs
@@ -118,6 +118,7 @@ fn wait_for_dsr(stdout: &mut impl Write, stdin: &impl ReadKey) -> ViuResult {
 
 /// Query the terminal whether it can display inline images (the kitty image protocol base-line)
 fn has_remote_support(stdin: &impl ReadKey, stdout: &mut impl Write) -> ViuResult {
+    let _ = crossterm::terminal::enable_raw_mode();
     // send the query
     write!(
         stdout,
@@ -156,6 +157,8 @@ fn has_remote_support(stdin: &impl ReadKey, stdout: &mut impl Write) -> ViuResul
         }
     }
 
+    let _ = crossterm::terminal::disable_raw_mode();
+
     // The Graphics query response
     let expected = [
         Key::UnknownEscSeq(['_'].into()),

Could you test that with urxvt on your system? (though i dont know if it would fix this issue as that is output by us, not a key input from the terminal)

If we go with that, i would like to clean it up a bit (like not changing the raw mode if it is already enabled, so that we dont accidentally disable it if it is activated outside of viuer) and make it potentially a drop-guard (ie disable raw mode on drop) and put it basically everywhere we output escape sequences and expect responses.

[hasezoey]

I have now tried rxvt-unicode, and even with the patch mentioned above, this is not fixed. I dont know why this terminal does not read & discards it instead of printing it, so i dont know how this could be fixed aside from a terminal blacklist.

[atanunq]

Thank you!!