Skip to content

[Process] Added support for stdout and stderr flush (Issue #7884) #8288

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[Process] Added support for stdout and stderr flush (Issue #7884) #8288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e5fbdf
[Process] Added support for stdout and stderr flush (Issue #7884)
Jun 16, 2013
9f5aef9
[Process] New tests for flush methods
Jun 16, 2013
321cbc1
[Process] [Tests] Fixing faulty tests for stdout and stderr flush met…
Jun 16, 2013
20e67f3
[Process] Removed unnecesary flush + get methods keeping flush only
Aug 9, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/Symfony/Component/Process/Process.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,19 @@ public function getIncrementalOutput()
return $latest;
}

/**
* Clears the process output.
*
* @return Process
*/
public function flushOutput()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot shouldn't a flush method send all the buffers content and clear the buffer afterwards, intead of only clearing it? (This is what php does when flushing a buffer, isn't it?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send? We don't send the buffers anyway. It's up to you to get the buffer and do whatever you want with them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me say it in another way. I would not expect that a method called flushxxx() just clears the buffer. If so, this method should be named e.g. CleanBuffer() (to be consitent with php naming)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this context, I don't what it can do on top of removing the data from the buffer. See http://www.catb.org/jargon/html/F/flush.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken from your link

2. [Unix/C] To force buffered I/O to disk, as with an fflush(3) call. This is not an abort or deletion as in sense 1, but a demand for early completion!

Thats what I meant.. It is not delete. Flush is more like a stop now but process everything which is already there.

Since this is not the case here, flush is not the correct term in this situation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the whole thing ;) Actually, I could link to definition 1, which is exactly what we do here, but I won't. I do understand what you mean, except that it does not apply here as the Process class never output/send the buffers, so there are no ambiguities here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that, given functionality of ob_flush(), which sends and cleans the buffer, the methods should be named differently. See #9407.

{
$this->stdout = '';
$this->incrementalOutputOffset = 0;

return $this;
}

/**
* Returns the current error output of the process (STDERR).
*
Expand Down Expand Up @@ -565,6 +578,19 @@ public function getIncrementalErrorOutput()
return $latest;
}

/**
* Clears the process output.
*
* @return Process
*/
public function flushErrorOutput()
{
$this->stderr = '';
$this->incrementalErrorOutputOffset = 0;

return $this;
}

/**
* Returns the exit code returned by the process.
*
Expand Down
18 changes: 18 additions & 0 deletions src/Symfony/Component/Process/Tests/AbstractProcessTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,15 @@ public function testGetIncrementalErrorOutput()
}
}

public function testFlushErrorOutput()
{
$p = new Process(sprintf('php -r %s', escapeshellarg('$n = 0; while ($n < 3) { file_put_contents(\'php://stderr\', \'ERROR\'); $n++; }')));

$p->run();
$p->flushErrorOutput();
$this->assertEmpty($p->getErrorOutput());
}

public function testGetOutput()
{
$p = new Process(sprintf('php -r %s', escapeshellarg('$n=0;while ($n<3) {echo \' foo \';$n++;}')));
Expand All @@ -175,6 +184,15 @@ public function testGetIncrementalOutput()
}
}

public function testFlushOutput()
{
$p = new Process(sprintf('php -r %s', escapeshellarg('$n=0;while ($n<3) {echo \' foo \';$n++;}')));

$p->run();
$p->flushOutput();
$this->assertEmpty($p->getOutput());
}

public function testExitCodeCommandFailed()
{
if (defined('PHP_WINDOWS_VERSION_BUILD')) {
Expand Down