-
-
Couldn't load subscription status.
- Fork 5.3k
[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 | - |
components/process.rst Outdated
| 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? |
components/process.rst Outdated
| | ||
| 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 components/process.rst Outdated
| 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 components/process.rst Outdated
| 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! |
| 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 |
components/process.rst Outdated
| | ||
| 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...) |
components/process.rst Outdated
| | ||
| .. 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. components/process.rst Outdated
| 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. |