Skip to content
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

Jump to bottom

[process] provide a restart method. #5456

Closed
wants to merge 6 commits into from
Closed

[process] provide a restart method. #5456

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8252085
[process] provide a restart method.
boombatower Sep 7, 2012
7dc25a9
Add newline before return.
boombatower Sep 7, 2012
286131b
Add tests.
boombatower Sep 7, 2012
34d821f
Update $callback documentation.
boombatower Sep 7, 2012
72c9793
Set properties to null instead of unset()'ing.
boombatower Sep 7, 2012
76aedb6
Update test to be more robust.
boombatower Sep 7, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/Symfony/Component/Process/Process.php
View file
Open in desktop
Expand Up @@ -143,6 +143,21 @@ public function __destruct()
$this->stop();
}

public function __clone()
{
unset($this->exitcode);
Copy link
Contributor

Choose a reason for hiding this comment

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

unset() accepts multi values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but is that preferred? and do you want them on one line or wrapped for each variable.

<?php
unset($var1, $var2);
unset(
  $var1,
  $var2
);

Or some variation.

unset($this->fallbackExitcode);
unset($this->processInformation);
unset($this->stdout);
unset($this->stderr);
unset($this->pipes);
unset($this->process);
unset($this->fileHandles);
unset($this->readBytes);
Copy link
Member

Choose a reason for hiding this comment

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

you should not unset the properties of the class but only reset the values in the object only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what you mean. Since the default state is non-set, unset() returns them to that state. Anything else like setting equal to null would not be equivalent.

Copy link
Member

Choose a reason for hiding this comment

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

unset($this->stdout) does not unset the value of the property. It unsets the property itself. Try to do a property_exists call after that.

If you try reading one of these properties before writing in it again, you will have an E_STRICT error. And if you write first, you will end with a public properties as properties added on the fly are public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good to know and seems somewhat non-intuitive. So what is the proper solution then to "reset the value" setting to null is the closest it seems.

Copy link
Member

Choose a reason for hiding this comment

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

it is the way to do it.


$this->status = self::STATUS_READY;
}

/**
* Runs the process.
*
Expand Down Expand Up @@ -300,6 +315,29 @@ public function start($callback = null)
$this->updateStatus();
}

/**
* Restarts the process by cloning and invoking start().
*
* @param Closure|string|array $callback A PHP callback to run whenever there is some
Copy link
Member

Choose a reason for hiding this comment

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

@param callable $callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should the stop method documentation be updated to the same format? I just used what they had.

Copy link
Member

Choose a reason for hiding this comment

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

yes, it should

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the documentation for restart() and placed the rest in separate request since there were more than just the one.

* output available on STDOUT or STDERR
*
* @return Process The new process.
*
* @throws \RuntimeException When process can't be launch or is stopped
* @throws \RuntimeException When process is already running
* @see start()
*/
public function restart($callback = null)
{
if ($this->isRunning()) {
throw new \RuntimeException('Process is already running');
}

$process = clone $this;
$process->start($callback);
return $process;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing new line before return.

}

/**
* Waits for the process to terminate.
*
Expand Down