Skip to content
This repository has been archived by the owner on Jan 6, 2020. It is now read-only.

Remove decoration from forwarded exception #317

Merged
merged 1 commit into from
May 11, 2017
Merged

Remove decoration from forwarded exception #317

merged 1 commit into from
May 11, 2017

Conversation

ro0NL
Copy link
Contributor

@ro0NL ro0NL commented May 3, 2017

See symfony/symfony#22618

before
image

after
image

Copy link

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👍

@@ -290,7 +290,7 @@ protected static function executeCommand(Event $event, $consoleDir, $cmd, $timeo
$process = new Process($php.($phpArgs ? ' '.$phpArgs : '').' '.$console.' '.$cmd, null, null, null, $timeout);
$process->run(function ($type, $buffer) use ($event) { $event->getIO()->write($buffer, false); });
if (!$process->isSuccessful()) {
throw new \RuntimeException(sprintf("An error occurred when executing the \"%s\" command:\n\n%s\n\n%s.", escapeshellarg($cmd), $process->getOutput(), $process->getErrorOutput()));
throw new \RuntimeException(sprintf("An error occurred when executing the \"%s\" command:\n\n%s\n\n%s.", escapeshellarg($cmd), self::removeDecoration($process->getOutput()), self::removeDecoration($process->getErrorOutput())));
Copy link
Contributor

Choose a reason for hiding this comment

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

having the dot at the end of the message (alone on its own line) after the command output looks quite weird actually. I would remove it.
The command output is very likely to include sentences anyway.

@ro0NL
Copy link
Contributor Author

ro0NL commented May 3, 2017

Trailing dot removed.

@ro0NL
Copy link
Contributor Author

ro0NL commented May 3, 2017

What about removing the escapeshellarg as well? It seems to cause double escaping + we're putting it between double quotes as well; removing it....

before

An error occurred when executing the "'assets:install --symlink --relative '\''web'\'''" command:

after

An error occurred when executing the "assets:install --symlink --relative 'web'" command:

Am i missing something?

@fabpot
Copy link
Member

fabpot commented May 11, 2017

Thank you @ro0NL.

@fabpot fabpot merged commit d5b9737 into sensiolabs:master May 11, 2017
fabpot added a commit that referenced this pull request May 11, 2017
This PR was merged into the 5.0.x-dev branch.

Discussion
----------

Remove decoration from forwarded exception

See symfony/symfony#22618

before
![image](https://cloud.githubusercontent.com/assets/1047696/25667089/bed3c6a2-3022-11e7-9557-baa9e8a5a71c.png)

after
![image](https://cloud.githubusercontent.com/assets/1047696/25667328/7935c46e-3023-11e7-8562-ab2a077dd487.png)

Commits
-------

d5b9737 remove decoration from forwarded exception
@ro0NL ro0NL deleted the decoration branch May 11, 2017 16:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants