-
Notifications
You must be signed in to change notification settings - Fork 1
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
Use nix::select
directly to fix MacOS support
#2
Conversation
On MacOS, `poll` and `kqueue` are not supported for tty[1]. So let's ditch mio completely and directly call `select` ourselves. It's not a lot of work anyway. This fixes MacOS support. [1]: tokio-rs/mio#1377
I tested this to work on both Linux and Mac. |
I'm trying to ponder the downsides of using the old As I understand it, this would fail if the application using xterm-query has already opened many files, because of the file selector limit of select ? Using |
wezterm's filedescriptor crate seems to have built a "poll" wrapper with compatibility for linux, mac, and windows: https://docs.rs/filedescriptor/latest/filedescriptor/index.html#polling |
The whole issue is that Mac doesn't support poll or kqueue for tty (it's in the first line of the pr description 😉). AFAIK the difference between poll and select is that the former is more efficient than the latter, for repeatedly calling. In here, we only need to call once and even if we didn't, we've only one FD so it wouldn't be so inefficient to re-initialising the fdset all the time. I can't find any information about how they differ in terms of the number of open FDs in the process. Could you point me to some resource? To summarise, I think only select usage keeps things simple here but if there are good reasons to avoid it for this very simple case, I can update the pr to use select only for Mac. |
https://beesbuzz.biz/code/5739-The-problem-with-select-vs-poll I'm not sure of the exact consequences of this problem. Wezterm seems to have adopted the logic of using select only for mac: https://docs.rs/filedescriptor/latest/filedescriptor/index.html#polling |
NM, I found it. I'll update the pr as per your suggestion. |
Closing this in favor of #4, that uses |
On MacOS,
poll
andkqueue
are not supported for tty. So let's ditch mio completely and directly callselect
ourselves. It's not a lot of work anyway.This fixes MacOS support.