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

fix: standardize usage of /dev/tty where possible #957

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aschey
Copy link
Contributor

@aschey aschey commented Dec 15, 2024

Resolves #919
Resolves #396
Supersedes #743

Previous attempts to prefer /dev/tty over stdin 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 prefer /dev/tty everywhere except for the mio event reader on MacOS.

This PR prioritizes 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.

@aschey aschey requested a review from TimonPost as a code owner December 15, 2024 23:57
@aschey aschey changed the title fix: standardize usage of /dev/tty everywhere fix: standardize usage of /dev/tty where possible Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cursor::position() fails when piping stdout Always read user input from /dev/tty
1 participant