-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Process] Introduce InputStream and iterator for output #6424
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
nicolas-grekas
commented
Apr 1, 2016
Q | A |
---|---|
Doc fix? | no |
New docs? | yes |
Applies to | 3.1 |
Fixed tickets | - |
is closed. | ||
|
||
In order to write to a subprocess standard input while it is running, the component | ||
provides the :class:`Symfony\\Component\\Process\\InputStream` class. |
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.
documenting its usage is necessary
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.
I was about the say the same. A small practical example would help a lot.
I think it might be time to split the doc of the Process component into multiple chapters, an dhaving one such chapter dedicated to the way to deal with the input (with basic examples and more complex ones) |
…rocesses (nicolas-grekas) This PR was merged into the 3.1-dev branch. Discussion ---------- [Process] Add InputStream to seamlessly feed running processes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18262 | License | MIT | Doc PR | symfony/symfony-docs#6424 Look at the tests, beautiful, isn't it? Commits ------- 3d20b6c [Process] Add InputStream to seamlessly feed running processes
…rocesses (nicolas-grekas) This PR was merged into the 3.1-dev branch. Discussion ---------- [Process] Add InputStream to seamlessly feed running processes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18262 | License | MIT | Doc PR | symfony/symfony-docs#6424 Look at the tests, beautiful, isn't it? Commits ------- 3d20b6c [Process] Add InputStream to seamlessly feed running processes
I think @stof has a valid point here, the process page is already rather big compared to some others. |
Don't forget to also document "IteratorAggregate to stream output" from symfony/symfony#18414 |
… (nicolas-grekas) This PR was merged into the 3.1-dev branch. Discussion ---------- [Process] Implement IteratorAggregate to stream output | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#6424 Sibling to #18386 for iterating of the output streams. Commits ------- 5947f5d [Process] Implement IteratorAggregate to stream output
6a7eb39
to
0c8e0d0
Compare
PR updated, LGTM. Do you think it needs more content? |
|
||
The :method:`Symfony\\Component\\Process\\Process::clearOutput` method clears | ||
the contents of the output and | ||
:method:`Symfony\\Component\\Process\\Process::clearErrorOutput` clears | ||
the contents of the error output. | ||
|
||
You can also use the :class:`Symfony\\Component\\Process\\Process` class with the | ||
foreach construct to get the output while it is generated. By default, the loop waits | ||
for new outputs before going to the next iterations:: |
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.
output, iteration?
👍 |
0c8e0d0
to
cd14410
Compare
echo $process->getOutput(); | ||
|
||
The :method:`Symfony\\Component\\Process\\InputStream::write` method accepts scalars, | ||
stream resources or Traversable objects as argument. As shown is the above example, |
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.
As shown is the
-> As shown in the
cd14410
to
d2066c9
Compare
@@ -128,6 +143,46 @@ are done doing other stuff:: | |||
which means that your code will halt at this line until the external | |||
process is completed. | |||
|
|||
Feeding the standard input of a Process |
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.
Streaming
instead of Feeding
?
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.
"Feeding" sounds good to me.
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.
"Standard" and "Input" should be uppercased.
👍 @nicolas-grekas thanks for these docs! |
@@ -43,13 +43,28 @@ The ``getOutput()`` method always returns the whole content of the standard | |||
output of the command and ``getErrorOutput()`` the content of the error | |||
output. Alternatively, the :method:`Symfony\\Component\\Process\\Process::getIncrementalOutput` | |||
and :method:`Symfony\\Component\\Process\\Process::getIncrementalErrorOutput` | |||
methods returns the new outputs since the last call. | |||
methods return the new output since their last call. |
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.
We should make this fix in all affected lower branches too.
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.
see #6658
We need a |
|
||
Before a process is started, you can specify its standard input using either the | ||
:method:`Symfony\\Component\\Process\\Process::setInput` method or the 4th argument | ||
of the Process constructor. The provided input can be a string, a stream resource or |
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.
"of the constructor."
Hi @nicolas-grekas! Can you please finish this PR? Otherwise, just say it and we'll fix the minor issues while merging. |
d2066c9
to
6e32e52
Compare
Comments addressed hopefully (I'll let you guys split this into several pages...) |
@@ -68,6 +83,10 @@ with a non-zero code):: | |||
echo $e->getMessage(); | |||
} | |||
|
|||
.. versionadded:: 3.1 | |||
Streaming the input or the output of a process using iterators | |||
were added in Symfony 3.1 |
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.
I think we should split this.
Above the paragraph added above:
.. versionadded:: 3.1
Support for streaming the output using iterators was introduced
in Symfony 3.1.
After the "Streaming to the standard input of a Process" headline:
Support for streaming the input of a process was introduced in
Symfony 3.1.
@@ -128,6 +147,46 @@ are done doing other stuff:: | |||
which means that your code will halt at this line until the external | |||
process is completed. | |||
|
|||
Streaming to the standard input of a Process |
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.
Streaming to the Standard Input of a Process
👍 except of the minor comments above (but we can address them while merging) Status: Reviewed |
6e32e52
to
2795261
Compare
Thanks @nicolas-grekas! Fyi, I've rebased this PR in #6705 and merged that one. |