-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] Add support for streams as input #10934
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
Conversation
array('file', '/dev/tty', 'w'), | ||
array('file', '/dev/tty', 'w'), | ||
); | ||
} elseif ($this->ptyMode && Process::isPtySupported()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just if
as the previous if
returns
Updated, addressed @stof comments |
* @param bool $blocking Whether to use blocking calls or not. | ||
* @param bool $close Whether to close pipes if they've reached EOF. | ||
* | ||
* @return string An array of read data indexed by their fd. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is string correct? the description tells otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romainneutron Can you fix this before I merge?
PR has been updated, comments have been addressed |
👍 |
Love that 👍 |
👍 I think the PipesInterface should be tagged as |
@romainneutron Can you tag all internal classes to make it clear that BC rules do no apply to these classes? |
Classes have been tagged, PR has been rebased |
Thanks @romainneutron. |
@romainneutron this should be documented (you omitted the "Doc PR" line in your summary btw) |
@stof I've opened an issue for this, see symfony/symfony-docs#3955 |
Process::setStdin
now accepts a stream as input.I've also split windows and unix pipes management.