fix: fall back to /dev/tty when stdin/stdout is not a tty

[aschey]

Resolves #919
Resolves #396
Supersedes #743

Previous attempts to read from /dev/tty had issues because of problems with polling /dev/tty on MacOS. This has been solved for a while with the use-dev-tty feature, so now it should be safe to fall back to using /dev/tty if stdin/stdout is not a terminal everywhere except for the mio event reader on MacOS.

This PR standardizes the use of /dev/tty for both input and output by splitting the tty_fd function into tty_fd_in and tty_fd_out so both can handle the appropriate fallback logic for stdin and stdout respectively. The only place that needs special care is the MacOS event reader when use-dev-tty is not enabled.

The mio event reader for MacOS has been modified to return an explicit error message when used with piped input to inform users that use-dev-tty is required for such use cases. The error message was previously getting swallowed, but will now be returned to the caller.

Finally, the logic to read the cursor position was always writing to stdout, causing an error when stdout was not a tty. This has been changed to use the new tty_fd_out function.

This was tested on Mac, Linux, and WSL. Windows should be unaffected.

[LoricAndre]

This might fix an issue I have in skim-rs/skim#653
Any idea when it could be merged ?

[aschey]

Hey @LoricAndre, I'm not sure when this will be reviewed. If you want to give this branch a try, it would be helpful to see if it does fix your issue since the original problem that skim was facing (#396) is one of the main reasons I made this change.

[joshka]

Changing from processing input from stdin to always using dev/tty seems like it would fundamentally change the behavior of apps. I'm not certain that this is desired in all cases. Can you talk a bit about the impact of the change and perhaps discuss where this might not be desired? A good way to think about this is put yourself in the perspective of an app developer who sees this change. What does it mean for their app? What does it mean if they hit a problem caused by this change?

[aschey]

Input processing currently uses the tty_fd method which only reads from stdin if it's a tty anyway, so it shouldn't have any noticeable effects.

The only scenario I can think of where explicitly reading from stdin instead of /dev/tty would be preferred is if you wanted to pipe some data into your app and have it parsed as input sequences, which seems odd.

It wouldn't be too difficult to have another method or some conditional logic that prefers stdin over /dev/tty that we only use for input reading, I just couldn't think of a good reason for it. If someone knows of a scenario where this can be problematic then I'm happy to change it.

[joshka]

My question is not so much about whether the use case is rational, but more about whether by choosing this approach is a one way door. I.e. this change makes it impossible to use stdin instead of tty as the input. An example of where you might want to do this is nesting an inner TUI app inside an outer one and using the stdin / out as the means to control the inner tty from the outer.

[aschey]

An example of where you might want to do this is nesting an inner TUI app inside an outer one and using the stdin / out as the means to control the inner tty from the outer.

Fair point. I suppose it doesn't hurt to prefer using stdin/stdout if isatty is true. I'll refactor the tty_fd_in and tty_fd_out methods to work that way and test it out.

[aschey]

Actually, I don't think it matters here. If you're piping input from a parent process, isatty will be false anyway, so there isn't a good way to determine this automatically. There have been requests to allow customizing the input source (#728, #941) which is probably the way to deal with these kinds of corner cases. That's out of scope for this fix though I think.

[LoricAndre]

@aschey I can confirm this fixes the issue I'm having (which is somewhat similar to #396, I am rewriting skim to use ratatui and I'm having issue with the zsh widgets.

[aschey]

Would it be better if I made a separate PR just for reading the cursor position from /dev/tty? That would require copy/pasting something very similar to this. I think it would be better to have reusable methods for determining the usage of /dev/tty, but I can go with the simpler change if that would be easier to review.

[joshka]

Would it be better if I made a separate PR just for reading the cursor position from /dev/tty? That would require copy/pasting something very similar to this. I think it would be better to have reusable methods for determining the usage of /dev/tty, but I can go with the simpler change if that would be easier to review.

I think I'd like to see some narrative about the design of the changes in addition to the PR here. Capture the previous conversation / concerns, document how you expect this will work etc. Fall backs are often problematic from a design perspective as they're often invisible to the calling code, which makes them confusing for devs ("this worked the other day, what's different here") Having something to point at that describes the reason in a way that works for the dev and the app users seems like a good idea here.

[donhk]

I just ran into this issue in mac, any idea when this will be merged? :)

[aschey]

I think I'd like to see some narrative about the design of the changes in addition to the PR here

Maybe it would help to use some other well-known libraries for comparison. Here is a list of other examples and how they handle I/O:

For input:

• termwiz - Provides different methods for finer control, but the default new method always opens a tty
• termion - Implements an interface for any io::Read-er
• termina - Prefers stdin and falls back to tty
• bubbletea - Prefers stdin and falls back to tty
• tcell - Opens a tty by default unless stdin is explicitly requested
• textual - always reads from stdin. The only way to handle piped stdin is for the app developer to do it themselves.

For output:

• termwiz - Same logic as input
• termion - Provides different methods for stdout/custom file descriptor
• termina - Same logic as input. Only checks if stdin is a tty rather than checking stdout separately, which seems like a small bug.
• bubbletea - always uses stdout unless manually overridden (redirected output doesn't work without the manual override)
• tcell - same logic as input
• textual - always writes to stderr. Presumably requires the same workaround as input handling.

The changes here should be transparent and not break any existing code. Not that I can promise anything with 100% certainty; there are plenty of weird edge cases with terminals. I do think it would be beneficial to make an API that gives the user more control over the I/O sources, but that will require design discussion, probable breaking changes, etc. The fallback approach is already used in other popular libraries and I think it's probably the best way to make things "just work" without any breaking changes.

The goal of this PR is to unblock people who want to make fzf-like apps. ex: myfile=$(fzf). Right now, writing to stdout when performing actions like querying the cursor position will always time out when doing this. It can easily be noticed when trying it with atuin which uses Ratatui/Crossterm. mycmd=$(atuin search -i) will fail with Error: The cursor position could not be read within a normal duration.

[aschey]

I will admit I don't have a super great reason for preferring the fallback method other than the fact that I've seen it done before by people that seem to know terminal mechanics well, so I opened a discussion on the bubbletea repo to see if I could hear more about their rationale for the design.

[joshka]

The problem with introducing a fallback is that it breaks anyone's code which previously assumed that things would fail in a certain way. Obligatory XKCD:

[aschey]

I got a reply on my discussion that mostly confirms my suspicions - the fallback method is used to avoid opening an extra file descriptor when it's not necessary to do so.

Crossterm already uses a fallback almost everywhere, just not when reading the cursor position, so that's the only real change here. If you want, we could just open a tty every time at the cost of opening extra file descriptors instead of reusing stdin/stdout. That's a pretty minor overhead.

[tugtugtug]

Can we please get some clear direction from the code maintainers? @TimonPost

The function seems broken in its current state. If the library will never support the fallback method as fixed in this PR, the documentation should clearly state that the function will take 2 seconds to timeout if called with a piped stdout, i.e. it should at least mention this known bug/limitation.

If backward compatibility is a concern, a feature flag could be introduced to allow the fallback behavior, provided it is properly documented.

[joshka]

Taking a bit more of a look at each of the other terminal libraries (just the input side)

• Termwiz
  • allows specifically setting input reader. When that's set, does not fallback, but instead returns an error at runtime.
• termion - has size for both stdin / any file descriptor, but you have to choose one or the other manually
• termina - has a fallback at construction time to /dev/tty if stdin is not a terminal
• bubbletea - fallback only for the default input setting (allows a custom input)
• tcell - uses /dev/tty only if you don't pass in something else
• textual - uses shutil.get_terminal_size, which uses environment variables and stdout only https://docs.python.org/3/library/shutil.html#querying-the-size-of-the-output-terminal

So I'd say that the common factor is that being explicit about things rather than falling back.
If I tell a terminal library to use stdout, it should use stdout. If I tell it to use a non-terminal file descriptor that's hooked up, it should use that.
Falling back to using /dev/tty when it gets to the window size call doesn't seem right.

Now that's not to say I don't think that using /dev/tty should be possible. I think it definitely should be possible to choose that the input comes from /dev/tty. But it should be an active application choice to do so rather than something that crossterm does without any notification to the app.

Does that pushback make sense? It's worth thinking about situations where a terminal app might be hooked up to a non terminal file descriptor for whatever reason, where the size information can be provided by that means regardless of it not being a terminal.

Crossterm already uses a fallback almost everywhere, just not when reading the cursor position, so that's the only real change here. If you want, we could just open a tty every time at the cost of opening extra file descriptors instead of reusing stdin/stdout. That's a pretty minor overhead.

This is actually a reasonable argument - what are the current places crossterm does fallbacks to /dev/tty that are comparable?

Can we please get some clear direction from the code maintainers

Technically, I'm a maintainer, but I'm responding with my user hat on rather than as a maintainer here.
If there's a compelling argument that I'm really wrong on my perspective, I can push this forward.

[tugtugtug]

Hi @joshka,

I understand the need for explicitness, but is the current position API (or any other cursor API) anywhere close to being explicit about how it works? If not, are you suggesting a completely new API or a set of APIs that would allow explicit control? As a user, I find this API unreliable, or rather unusable, as it’s unclear when it will work, leading to a 2-second delay whenever it fails. This seems inconsistent with your philosophy.

Again, while a more explicit API design would be great, it just seems awkward in this case, as also the Windows implementation doesn’t seem to align with a more explicit approach. So, are you suggesting a Unix-specific variant that requires an explicit parameter or a more wide-spread re-design?

[aschey]

This is actually a reasonable argument - what are the current places crossterm does fallbacks to /dev/tty that are comparable?

Every other place that needs to read from the tty uses a fallback.

• The tty_fd method (used for the input reader and to enable raw mode)
• query_keyboard_enhancement_flags_raw
• window_size

The change here unifies the logic for the latter two scenarios and makes the cursor position implementation behave the same way.

I agree that it would be good to allow this to be more customizable, but @tugtugtug is correct in that it will require some additional refactoring to expose these options everywhere they need to be used, as well as require platform-specific considerations (CONIN$/CONOUT$ on Windows vs /dev/tty on Unix). Crossterm uses a lot of free functions enable_raw_mode/poll, etc. These methods would need to have an overload or be refactored to use a builder pattern to make something like this work.

[joshka]

I understand the need for explicitness, but is the current position API (or any other cursor API) anywhere close to being explicit about how it works? If not, are you suggesting a completely new API or a set of APIs that would allow explicit control? As a user, I find this API unreliable, or rather unusable, as it’s unclear when it will work, leading to a 2-second delay whenever it fails. This seems inconsistent with your philosophy.

Again, while a more explicit API design would be great, it just seems awkward in this case, as also the Windows implementation doesn’t seem to align with a more explicit approach. So, are you suggesting a Unix-specific variant that requires an explicit parameter or a more wide-spread re-design?

I just want to clarify my involvement here. I'm a maintainer of the Ratatui crate mainly;I have maintainer access on the crossterm repo mainly because I volunteered to help Timon get some long outstanding issues and PRs reviewed and merged. That doesn't make me the expert / gatekeeper on what goes on with these things. It's highly likely that there are many users that are significantly more knowledgeable about this library and the nuances than I am. So I want to make sure that anything that I'm saying in terms of my position on things is coming from the perspective of a user mostly.

As a user of any API, I expect that when something fails I should get a reasonable failure message that helps me understand the failure. It's ok to create methods that do the right thing when that failure occurs, but care has to be taken to make sure that the original failure is available when that fallback is undesirable. Failling back to or explicitly opting in to using /dev/tty seems like a logical thing to want. But it also seems like there are use cases where this would not be ideal.

Right now, we're in the situation where this doesn't have a fallback, so that's the existing expected behavior of any application that uses these methods. Changing that so that what was previously an error is now successfully doing something different is a breaking change for some use cases. Introducing a breaking change has a few options:

1. ignore it and say this is the way now (this is what this PR does)
2. make it configurable / explicit (e.g. some static config that tells how this works)
3. add a new method which makes it possible to run the previous behavior
4. make an additive change only (i.e. don't break existing code, make new code able to work)
5. there's probably hybrid approaches here (e.g. use Option<> to choose whether things are explicit or defaulted (e.g. what bubbletea does)

Each of these approaches has a certain amount of complexity. I don't like option 1 very much at all, so choosing to do anything else would be my preference. With that said, I'm not an expert on the choices made internally to the library already, so if a fallback is more consistent, then that makes a bunch of sense.

The use cases I'm interested in are ptys, things like tuis that are hooked up to an ssh connection using russh, driving tuis to test them using piped input, ...

Every other place that needs to read from the tty uses a fallback.

Thanks @aschey, I'll give the PR another solid read in light of that sometime this week. I think you might be right that this approach is the correct one given the existing behavior. (I.e. that this PR doesn't make things worse).

As a related aside. There was a discovery in #996 that macOS can actually use kqueue and mio against /dev/ttys000 (where 000 is the terminal attached to the process), just not /dev/tty. So something around that might be worth considering for the fallback instead of /dev/tty.

Aside 2: Given that there's been no complaints about the new rustix implementation of things, it could be worth considering dropping the libc code altogether to simplify the amount of code in this area. When this was implemented we kept both because the change was fundamentally changing some of the deep internals. This might be worth considering as a future change (noting here as you're sort of in that code area).

[LoricAndre]

Any news about this ? I understand that this can be complicated to review, but this is actually a real issue in some contexts, like zsh widgets as mentioned above.

[aschey]

termina handles this correctly if anyone needs this functionality. There isn't a published ratatui backend for it, but you can mostly copy it from helix's implementation.