Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle terminal interrogation failure better #1469

Closed
dankamongmen opened this issue Mar 27, 2021 · 47 comments
Closed

handle terminal interrogation failure better #1469

dankamongmen opened this issue Mar 27, 2021 · 47 comments
Assignees
Labels
enhancement New feature or request

Comments

@dankamongmen
Copy link
Owner

Currently, if we interrogate a terminal, and don't get a response, the program will simply hang. Hence the requirement that the user explicitly call some function (e.g. notcurses_check_pixel_support()) before we begin interrogation.

I hate this so fucking much. I have no idea whether user input can break up the terminal response, but it wouldn't surprise me at all (and if it doesn't break the response, we'll at best lose said input). Hate hate hate hate hate.

I've been unwilling to put in a timeout, because on a loaded system, a slow reply is totally normal, and what timeout would we use, and even worse would be pushing that timeout question to the user, because they know nothing. I stand by this decision, because I am a dinosaur and a curmudgeon and everyone's too busy these days. Let the computer cogitate, if that's what it needs to do.

I do think, however, that we need at least print a diagnostic after a certain time goes by without a response, so the user isn't just left with no idea what's up. I think this can be dumped to stderr; it'll be jarring and disrupt any drawn screen, but hey, you've been sitting around for five (or whatver) stimulus-free seconds already, and in all likelihood you'll be sitting there until time ends or the power fails.

So yeah, make a concession to reality, and after say 5 seconds without a response from the terminal, print a diagnostic to stderr about "N seconds without a response from the terminal....". Maybe include the TERM value in this output for the inevitable bug reports.

Better yet would be a method to break user input up from terminal responses, but I doubt that this is possible =[. Maybe we ought take unrecognized input and queue it up as user input, but then we're just forwards-incompatible with new, wacky returns from tomorrow's terminals.

Fuck the world, save yourself.

@dankamongmen
Copy link
Owner Author

i think assuming that the terminal communicates back to us via escape codes is reasonable. so if we're not in an escape code, punt that over to the general input queue. this ought take care of e.g. input piped in.

@dankamongmen
Copy link
Owner Author

note that in #1506 we stopped attempting terminal interrogation when TIOCGSIZE doesn't return pixel information, which has proven a pretty good heuristic thus far.

@dankamongmen
Copy link
Owner Author

can't we just open /dev/tty as a new file descriptor and talk to that directly? wouldn't that at least fix the piped-input case?

@dnkl
Copy link
Contributor

dnkl commented Apr 18, 2021

can't we just open /dev/tty as a new file descriptor and talk to that directly?

I'm not familiar enough with notcurses to say if this is a problem or not, but unless you are sure stdin/stdout is a tty you need to open, and use /dev/tty. Of course, it will still be intermingled with user input, if stdin is the tty.

i think assuming that the terminal communicates back to us via escape codes is reasonable.

FWIW, I'm not aware of any query replies that aren't escapes. In fact, I'm not aware of anything, besides user input, that isn't escapes. Foot never sends anything that isn't an escape, except for user input.

A common trick to avoid waiting forever for replies to queries that aren't supported on all terminals is to send another query right after the first one. The last one should be one that you are sure (as sure as one can be) is supported on all terminals (Send Device Attributes, \E[c, is a common choice). This way, you'll get at least something returned. You'll have to parse the response to determine if you got one, or two replies.

@dankamongmen
Copy link
Owner Author

A common trick to avoid waiting forever for replies to queries that aren't supported on all terminals is to send another query right after the first one. The last one should be one that you are sure (as sure as one can be) is supported on all terminals (Send Device Attributes, \E[c, is a common choice). This way, you'll get at least something returned. You'll have to parse the response to determine if you got one, or two replies.

if there is a truly general such code, this is a very elegant way to handle the problem; i love it! <3 <3 <3 <3 <3 <3

@dnkl
Copy link
Contributor

dnkl commented Apr 18, 2021

if there is a truly general such code

Let me put it this way; I'm not aware of any terminals that do not support \E[c.

@dankamongmen
Copy link
Owner Author

so i have one last concern before implementing this sweet technique: how does this interact with someone redirecting stdin? the particular case i'm worrying about is when a large source is sent to stdin (someone holding down on a key might presumably have the same result, read on).

even if i don't have an internal thread reading off input, and instead am just checking input when either (a) i've just written something expecting a result, or (b) the user requests it, let's say there's a 2GB file being sent to stdin. i would need to buffer all of that in the (a) case, since the user hasn't asked for it yet, and it's going to come before my expected answer from the terminal.

of course, if stdin is redirected to a file, it presumably is not getting input from the tty like normal. so perhaps i can read responses exclusively on the actual terminal fd (i.e. the one to which i wrote the query), and it ought never see redirected stdin. regular stdin (i.e. from a tty) would still need to be buffered in this case, and indeed, it might be broken across both stdin and the termfd...we'll need experiment with that.

@dankamongmen
Copy link
Owner Author

and this is of course predicated on successful implementation of #1692, which i intend to handle very soon.

@dnkl
Copy link
Contributor

dnkl commented May 28, 2021

You should be reading terminal query responses directly from the TTY, i.e. not stdin. Simply because you'll never get any replies if you read from a re-directed stdin.

But, you're right, if stdin is not re-directed, there's a chance you'll be reading user input before the reply arrives.

Fun fact: the terminal itself has more or less the same problem when pasting text from the clipboard. And yes, in this case, foot buffers everything else until the paste is complete, then it drains the buffered writes.

So, it's very unlikely you'll get 2GB user input on the TTY. A re-directed stdin, yes, but you're not reading TTY replies from that. However, you can get fairly large amounts of data if the user pastes a large amount of text.

If stdin is not re-directed, then it and your term FD refers to the same underlying TTY FD. And, the stdin FILE * stream may be buffered by the OS already. Not sure how you've set things up? So you absolutely need to be careful how you read from them.

@dankamongmen
Copy link
Owner Author

it would be good to go ahead and implement this ASAP, since it'll allow us to run on both mainbranch and graphicsbranch alacritty.

@dankamongmen
Copy link
Owner Author

You should be reading terminal query responses directly from the TTY, i.e. not stdin. Simply because you'll never get any replies if you read from a re-directed stdin.

I'm never reading from stdin; i'm worried about when it's not redirected, user input being interpreted as terminal responses. but so long as terminal responses are always escaped, and arrive atomically (not safe to assume with old terminals, but hopefully safe now--i'm not getting into any ESCDELAY business), they ought be distinguishable. i think we're good to go here!

@dnkl
Copy link
Contributor

dnkl commented Jun 6, 2021

but so long as terminal responses are always escaped, and arrive atomically (not safe to assume with old terminals, but hopefully safe now--i'm not getting into any ESCDELAY business), they ought be distinguishable

I don't think you can assume that. You can probably assume that a single ESC is just that, but not that you'll have the entire response atomically. I think it's theoretically possible to get a single ESC in one read, even though it's part of a longer response.

@dankamongmen
Copy link
Owner Author

but so long as terminal responses are always escaped, and arrive atomically (not safe to assume with old terminals, but hopefully safe now--i'm not getting into any ESCDELAY business), they ought be distinguishable

I don't think you can assume that. You can probably assume that a single ESC is just that, but not that you'll have the entire response atomically. I think it's theoretically possible to get a single ESC in one read, even though it's part of a longer response.

ugh, that leads to the whole ESCDELAY situation that NCURSES has -- how long do you wait, following receipt of an \e, before handing it all off to the user as regular input?

so far as i can tell, options include

  • not allowing the user to make use of escape (unacceptable)
  • some timeout (unacceptable, with more steps)
  • assuming atomic receipt of escape sequences

there are some guarantees of atomicity of sufficiently small write(2)s to a pipe, but i don't think they apply to this case, and i'd be hesitant to rely on them anyway. obviously there's no hope of atomicity if the reply is written across multiple write()s, but i'd hope that doesn't happen. what are your thoughts?

@dankamongmen
Copy link
Owner Author

i'm thinking that, rather than send DA and then conditionally send 2x(XTSMGRAPHICS+DA), we ought just send 2xXTSMGRAPHICS+DA at the beginning. it cuts two round-trip times and several bytes, we need the same parsing logic either way (pump that lemma!), and it makes our behavior more deterministic. furthermore, i'd say we probably ought toss this out immediately upon starting, so the reply is ideally available by the time we've initialized, minimizing latency to quiescence.

@dankamongmen
Copy link
Owner Author

i'm thinking that, rather than send DA and then conditionally send 2x(XTSMGRAPHICS+DA), we ought just send 2xXTSMGRAPHICS+DA at the beginning. it cuts two round-trip times and several bytes, we need the same parsing logic either way (pump that lemma!), and it makes our behavior more deterministic. furthermore, i'd say we probably ought toss this out immediately upon starting, so the reply is ideally available by the time we've initialized, minimizing latency to quiescence.

shit, if we assume we're always going to get a reply from DA, we can eliminate the entire notcurses_check_pixel_support() fecaoma, which would make me truly happy. ok, let's go for it!

@dankamongmen
Copy link
Owner Author

so to do this we'd just make sixel support conditional on successful parse of the second XTSMGRAPHICS, or better yet both of them. in that case, there's no need to trigger it off any particular DA response. yes, i love this!

@dnkl
Copy link
Contributor

dnkl commented Jun 6, 2021

I think that any write, regardless how small, may be split up if the pipe is already almost full? That's where I think the theoretical problem lies.

obviously there's no hope of atomicity if the reply is written across multiple write()s, but i'd hope that doesn't happen

I do think that may happen. Not sure how common. In foot, query replies are usually send in a single write(), but not always. For example, OSC-52 (clipboard data) is split up. DA and XTGRAPHICS responses are sent in single writes. But more importantly, alt-prefixed user input is split up, so that the ESC is sent first, and the character next.

I don't mind updating foot, but I think that if one terminal send split up responses, then others are too.

@dankamongmen
Copy link
Owner Author

I think that any write, regardless how small, may be split up if the pipe is already almost full? That's where I think the theoretical problem lies.

absolutely, but if i throw this off immediately (and i only need it at startup, though that's not true for cursor location queries), it "ought" be among my first inputs. i agree there are fundamental races here, but there seem to be at any selection of timeout (i'm suggesting a choice of 0).

@dankamongmen
Copy link
Owner Author

here's approximately what i'd like to see (though kitty's left curly braces don't seem to be matching its curly extender, who knows whether it's a font thing or not)

2021-06-13-150754_808x1417_scrot

@dankamongmen
Copy link
Owner Author

man, that is one big, grotesque state machine!

@dankamongmen
Copy link
Owner Author

confirmed WezTerm is here to party, good work @wez

2021-06-13-151827_960x1057_scrot

@dnkl
Copy link
Contributor

dnkl commented Jun 13, 2021

hey @dnkl , fix your horizontal 1/8ths, i would not hit it

You're on the stable 1.7.2 release? I believe this has already been fixed, if you run latest master:
foot-notcurses-info

(note: foot doesn't render any brace extenders itself, but with this particular font they still look good :))

@dankamongmen
Copy link
Owner Author

hey @dnkl , fix your horizontal 1/8ths, i would not hit it

You're on the stable 1.7.2 release? I believe this has already been fixed, if you run latest master:
foot-notcurses-info

what kind of christmas-in-london-ass-font is this lol =D

@dankamongmen
Copy link
Owner Author

alright, that was a slog, but i think we're ready to merge the initial phase of work -- unified single query, from which we opportunistically extract sixel parameters. this allowed us to kill off alacritty_sixel_hack, nice.

dankamongmen added a commit that referenced this issue Jun 13, 2021
@dankamongmen
Copy link
Owner Author

@dnkl if i'm in a control sequence that we know to be split up in such a fashion, i can explicitly wait. what i don't want to do is wait following arbitrary control-sequence-looking-things that I don't recognize before passing them up to the user. Theoretically, I ought not be getting control sequences I don't recognize, since they're only coming in response to actions I take (save bracketed paste maybe?). i'm handling alt-prefixed characters fine right now, though i can't claim to be doing so perfectly under rough conditions....

alright, anyway, this issue has gone on for a long time. we know how we're proceeding now. next thing is to do opportunistic certain terminal detection using these results -- i.e., let them override TERM for purposes of heuristics. not yet sure whether we want to do so for terminfo; we'll see.

@dnkl
Copy link
Contributor

dnkl commented Jun 14, 2021

what kind of christmas-in-london-ass-font is this lol =D

Fantasque Sans Mono, my goto font when I need a vector font :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants