Fix Open3 documentation: clarify redirection options and blocking behavior #24
Conversation
lib/open3.rb
Outdated
| # in the call to Process.spawn; | ||
| # see {Execution Options}[https://docs.ruby-lang.org/en/master/Process.html#module-Process-label-Execution+Options]. | ||
| # | ||
| # Note: The redirection options :in and [:out, :err] will be ignored |
There was a problem hiding this comment.
Q: :in and [:out, :err] is it meant to be different from how the other methods are documented?
There was a problem hiding this comment.
I thought that was a little weird, too.
It must be because [:out, :err] is actually what's used in the code of that method:
https://github.com/ruby/open3/pull/24/files#diff-22509663fa03dbb4fc6fa062734ed1eed523f8d125e604da6e303d8102036bceR535-R539
So if you specified that key exactly, it will be silently ignored, which was the theme of these updates.
I just tested what happens if you pass :err (or :out) specifically, then you get an error that the FD was specified twice.
We probably don't need to document that [:out, :err] is specifically silently ignored because that seems like an implementation detail that could be changed.
Maybe instead we should change the language to say something more like "just don't specify any of the redirection handles"?
I suppose we could also change the code to warn about them being passed (and ignored) (so that it isn't silent)?
What do you think?
There was a problem hiding this comment.
Since changing the behaviour (even improving it) is a longer step, I'd settle for documentation that guided towards the current right thing to do, even if that is a little detailed.
There was a problem hiding this comment.
I made the message more general, what do you think about this?
rwstauner
force-pushed
the
docs
branch
2 times, most recently
from
122ca07 to
06a57e3
Compare
1. Fixed incorrect documentation in popen3, popen2, and popen2e that claimed these methods always wait for child processes to exit. Actually, they only wait when used with a block. Without a block, they return immediately and the caller must wait manually. 2. Updated pipeline method examples to clarify that when used with a block, the methods automatically wait for all processes to exit. Removed misleading code that showed manual thread joining in the block form.
Each Open3 method that sets up pipes now documents which redirection options will be ignored. This clarifies that when methods create pipes for stdin, stdout, or stderr, the corresponding redirection options in the execution options hash will have no effect.
Replace specific implementation details about which redirection options are ignored with a more general user-friendly message. This avoids exposing internal implementation details and provides clearer guidance to users.
|
I forgot this was still open. Any other feedback about the changes? |
Summary
This PR fixes documentation issues in the Open3 module:
🤖 Generated with Claude Code