Project

General

Profile

Bug #17094

PTY methods with blocks

Added by soutaro (Soutaro Matsumoto) 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-dev:50941]

Description

Some methods on PTY yield one array if a block is given, but the RDoc says it passes two arguments to the block.

https://github.com/ruby/ruby/blob/master/ext/pty/pty.c#L529

return rb_ensure(rb_yield, assoc, pty_close_pty, assoc);

https://github.com/ruby/ruby/blob/master/ext/pty/pty.c#L467

 *   PTY.open {|master_io, slave_file| ... } => block value

I'd like to propose to fix the implementation. However, it would make more sense to fix the docs because of potential incompatibilities.

Updated by nobu (Nobuyoshi Nakada) 2 months ago

As it can be incompatible only when passing a lambda, I don't think it is a serious problem.

Updated by soutaro (Soutaro Matsumoto) about 2 months ago

Runtime testing of RBS uncovered this issue.

https://github.com/ruby/rbs/pull/346#issuecomment-665817340

The RDoc implies the type of (*String) { (IO, IO) -> void } -> void, but the implementation is (*String) { ([IO, IO]) -> void } -> void. And runtime type checking detected the issue.

Maybe we can fix the RBS runtime type checking then.

#3

Updated by jeremyevans (Jeremy Evans) about 1 month ago

  • Status changed from Open to Closed

Applied in changeset git|9e25eb308d4fae9a10e120c2b4601916cc38336c.


Update PTY.open documentation to document it yields a single argument [ci skip]

For a regular block, accepting two arguments is fine as the array
will be autosplatted. However, a lambda that accepts two arguments
will not work.

We could change the implementation to yield two arguments instead
of an array with a single argument, but that would be less backwards
compatible.

I'm only changing the call-seq to be precise, other examples pass
a literal block that accepts two arguments, and I left those alone
as that will be the most common usage.

Fixes [Bug #17094]

#4

Updated by sawa (Tsuyoshi Sawada) about 1 month ago

  • Description updated (diff)

Also available in: Atom PDF