AnonymouX47 / term-image

Display images in the terminal with python
https://term-image.readthedocs.io
MIT License
206 stars 9 forks source link

Strange "phantom keystrokes" on startup of app that uses Term-Image under hterm #81

Closed danschwarz closed 1 year ago

danschwarz commented 1 year ago

Some weird behavior to report - not sure if it started with the synchronized-output feature or if it was there previously with the earlier UrwidImage work. Now, on startup of the toot app under certain terminals, sometimes, I get phantom keystrokes - the app is misinterpreting terminal commands as text typed by a user.

here's an example - this is under hterm.js (unknown version, probably fairly old). Immediately on startup of the app, it is as if the user typed "r" to reply to a status, then you see the ffff/ffff/ffff;rgb:2222 ... command in the edittext box.

So perhaps there's some ANSI escape intialization sequence that hterm is misinterpreting as keystrokes? This doesn't happen on every startup, but fairly often, so it may be a question of timing.

Screenshot 2023-03-06 094542

I haven't seen this happen in Windows Terminal or Kitty.

danschwarz commented 1 year ago

Apologies; the culprit is xterm.js not hterm. It's whatever version of xterm.js is bundled with Google Cloud Shell's [Theia](https://theia-ide.org/) v1.34 editor environment. I tried to find out what version of xterm.js it is by examining the Javascript source, but it's not clear from the code.

I wonder if the ANSI escape sequence above is something to do with palette-setup?

AnonymouX47 commented 1 year ago

First off, sorry for the late reply.

The issue is actually not a bug, it's a response timeout issue. A couple of queries (OSC 10, OSC 11 and CSI c) are sent to the terminal emulator to determine the default foreground and background colors (used for blending semi-transparent parts of images to produce better results for text-based render styles). How quickly a terminal emulator responds to queries is out of the control of the application, once it sends a query, the best it can do is to wait for the response and it can't exactly wait forever.

As it seems, the terminal emulator does not respond before the default timeout elapses. The response is then sent later (after the function that should read it has returned) and that's what is seen written to the screen (though, ideally, it shouldn't have been written to the screen since urwid turns ECHO off). Because the response comes as input via the TTY, urwid reads it and that is why it seems as though "r" is pressed ("r" is in the response).

The solution is to increase the timeout using set_query_timeout(). Also, you could disable terminal queries using disable_queries() but this will disable certain features. See Terminal Queries.

I'll suggest you add CLI option(s) to toot for this purpose i.e to set the timeout and to disable queries.

I think I should include this in the FAQs 🤔.

By the way, I tried using the terminal on https://theia-ide.org/ (the online version), seemed like python wasn't installed and couldn't figure out a way to install it.

AnonymouX47 commented 1 year ago

Most graphics-capable terminal emulators are quick to respond to queries. Non-graphics-cable terminal emulators usually have higher response times but of all the terminal emulators I've tested locally, 0.1 seconds (the default) was always enough for a full-screen terminal window at any rational zoom level (on a 2304×1296 display). The same can't be said for sessions over a network though, those will usually require a higher timeout.

AnonymouX47 commented 1 year ago

Thinking about this (i.e the use of terminal queries and the possible side effects) now... it doesn't seem so obvious from the docs, even though it's in there 🤔. I'm really sorry about that.

Currently, it's mentioned in:

... but the issue is there are a few cases that are implicit, one of them is your case here. There is no method docstring where in I could note the use of terminal queries for these features, though they are listed here.

Is there any other way you think I could make it more obvious, particularly for the implicit cases?

danschwarz commented 1 year ago

Thank you for the very detailed explanation. Makes total sense. I knew about the terminal request response timeouts but I thought this issue might have been related to a missing ANSI sequence handler on the part of xterm.js rather than a timeout. I need to give the xterm.js authors more credit, and Google’s cloud VM performance less credit :)

If these are really the only places that you are waiting on responses from the terminal, then I can safely increase the timeout value to, say, 0.5sec without impacting overall app performance much.

On Wed, Mar 8, 2023 at 3:04 AM Toluwaleke Ogundipe @.***> wrote:

Thinking about this (i.e the use of terminal queries and the possible side effects) now... it doesn't seem so obvious from the docs, even though it's in there 🤔. I'm really sorry about that.

Currently, it's mentioned in:

  • the FAQs (yet to push the changes though).
  • the docstring of every function or method (or of a parameter) that performs an operation involving a query.

... but the issue is there are a few cases that are implicit, one of them is your case here. There is no method docstring where in I could note the use of terminal queries, though these implicit features are listed here https://term-image.readthedocs.io/en/latest/reference/utils.html#list-of-features-that-use-terminal-queries .

Is there any other way you think I could make it more obvious, particularly for the implicit cases?

— Reply to this email directly, view it on GitHub https://github.com/AnonymouX47/term-image/issues/81#issuecomment-1459692691, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY4FJUUOIB4OFYV4BCYP4TW3A4PXANCNFSM6AAAAAAVR5JSPE . You are receiving this because you authored the thread.Message ID: @.***>

AnonymouX47 commented 1 year ago

I need to give the xterm.js authors more credit, and Google’s cloud VM performance less credit :)

Can't be too sure though, the performance of xterm.js might be the culprit here... after all, I wouldn't expect a terminal emulator written in TypeScript and being executed by a browser to be so performant (compared to the usual ones written in Rust, C and other compiled languages). Though that is not the fault of the authors, it's just an inherent characteristic of the language and it's mode of translation.

I'm not into web development, so i might be wrong but I don't think the Cloud VM isn't responsible for executing xterm.js, the browser is.

If these are really the only places that you are waiting on responses from the terminal

... at the moment. I don't know what the future holds :grin:.

I can safely increase the timeout value to, say, 0.5sec without impacting overall app performance much.

Well... quite true. After all, the query is performed only once and the result is cached (same if it fails).

AnonymouX47 commented 1 year ago

By the way, if any tips concerning my question

Is there any other way you think I could make it more obvious, particularly for the implicit cases?

in https://github.com/AnonymouX47/term-image/issues/81#issuecomment-1459692691a above crosses your mind, please let me know.

Thanks 🙇🏾‍♂️