Feature #18655
closedCopy `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core.
Description
Extracted from [Feature #18566].
The decision was made to consider the methods from io-wait
and io-nonblock
one by one.
I think wait_readable
and wait_writeable
should be fairly non-controversial. They're quite essential to use IO#read_nonblock
and IO#write_nonblock
effectively.
Proposed patch: https://github.com/ruby/ruby/pull/5694
NB: if we only merge some methods, then io/wait
must test which methods it needs to define or not. For now I use a if RUBY_VERSION >= "3.2"
check in extconf.rb
, but there might be a better approach.
Updated by byroot (Jean Boussier) over 2 years ago
- Related to Feature #18566: Merge `io-wait` and `io-nonblock` gems into core IO added
Updated by Eregon (Benoit Daloze) over 2 years ago
Copying a general comment (https://github.com/ruby/ruby/pull/5694#issuecomment-1081700929) on the PR here:
I agree with @ioquatix (Samuel Williams) that it doesn't make sense to make IO#wait_readable and IO#wait_writable core, but IO#wait not, because of their close relation. And similarly it makes no sense to me to IO#leave wait_priority in the gem (that was just badly documented and that can be fixed easily).
The 3 IO#wait_* methods should be together and must stay consistent with one another, and also implementation-wise it's super weird if not in the same place. It's a similar argument for IO#wait.
The controversial methods in https://bugs.ruby-lang.org/issues/18566#note-16 seem ready? and nread.
Those seem fine to leave in the gem (very rarely used, and not so directly related to IO#wait*).
Updated by larskanis (Lars Kanis) over 2 years ago
IMHO the problem with IO#wait is that its return value not well defined. Here is the issue: https://github.com/ruby/io-wait/issues/13
Updated by Eregon (Benoit Daloze) over 2 years ago
@larskanis (Lars Kanis) That's a good point (it seems a bug).
And IO#wait
also evolved more than IO#wait_{readable,writable,priority}
IIRC.
It doesn't seem a very stable interface yet, and it's hard to use (STDOUT.wait(IO::READABLE | IO::WRITABLE)
doesn't work as expected, it hangs)
All of these 4 methods end up in rb_io_wait()
I think, but with a fair amount of code to wrap it.
So I think it'd be fair to leave IO#wait out of core for now then based on these reasons.
I think it'd make sense to include wait_priority
, so the 3 wait_*
with simple semantics are kept together.
Updated by ioquatix (Samuel Williams) over 2 years ago
I'm fine with that.
Updated by ioquatix (Samuel Williams) over 2 years ago
I've made a PR to move wait
, wait_readable
, wait_writable
, wait_priority
to io.c
with some minor compatibility and documentation improvements.
Updated by ioquatix (Samuel Williams) over 2 years ago
- Subject changed from Merge `IO#wait_readable` and `IO#wait_writable` into core to Copy `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core.
Updated by matz (Yukihiro Matsumoto) over 2 years ago
If @nobu (Nobuyoshi Nakada) and @akr (Akira Tanaka) are OK, I am OK.
Matz.
Updated by nobu (Nobuyoshi Nakada) over 2 years ago
ioquatix (Samuel Williams) wrote in #note-6:
I've made a PR to move
wait
,wait_readable
,wait_writable
,wait_priority
toio.c
with some minor compatibility and documentation improvements.
This mixes the fix for compatibility (https://github.com/ruby/io-wait/issues/13), which is irrelevant to this PR.
Updated by ioquatix (Samuel Williams) over 2 years ago
- Status changed from Open to Closed
This is now completed.
Updated by matz (Yukihiro Matsumoto) over 2 years ago
After the discussion, we will drop wait_priority
, but others are accepted.
Matz.