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

Add a formatter to compactly print stack traces #78

Closed
wants to merge 1 commit into from
Closed

Add a formatter to compactly print stack traces #78

wants to merge 1 commit into from

Conversation

Erikvv
Copy link

@Erikvv Erikvv commented Oct 11, 2017

The available formatters are very verbose. This one produces a similar format to the native PHP exception handler.

It could use some more tests.
The type of the exception is not printed, it is not available in the event.

Solves #32 if you use this formatter

PHP version is bumped to 7.0 to use the coalesce operator and more type hints

Produces a similar format to the native PHP exception handler
@zf2timo
Copy link

zf2timo commented Oct 12, 2017

You changed the php constrained in the composer.json, so the .travis.yml also have to be updated.

@froschdesign
Copy link
Member

froschdesign commented Oct 12, 2017

@Erikvv and @zf2timo

PHP version is bumped to 7.0 to use the coalesce operator and more type hints

You changed the php constrained in the composer.json, so the .travis.yml also have to be updated.

Please open a new PR for these changes!

$event['timestamp'] = $event['timestamp']->format($this->getDateTimeFormat());
}

$output = '[' . $event['timestamp'] . '] ' . $event['priorityName'] . ' ('
Copy link
Member

Choose a reason for hiding this comment

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

These lines are very hard to read. Please use sprintf.

if (! empty($event['extra']['trace'])) {
$output .= "Stack trace:\n";
foreach ($event['extra']['trace'] as $i => $traceLine) {
$output .= "#$i " . $this->formatTraceLine($traceLine) . "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Same here. (Don't use "#$i ".)

}

/**
* {@inheritDoc}
Copy link
Member

Choose a reason for hiding this comment

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

}

/**
* {@inheritDoc}
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@@ -0,0 +1,164 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
Copy link
Member

Choose a reason for hiding this comment

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

Please replace the header with this:

/**
 * @see       https://github.com/zendframework/zend-log for the canonical source repository
 * @copyright Copyright (c) 2015-2017 Zend Technologies USA Inc. (http://www.zend.com)
 * @license   https://github.com/zendframework/zend-log/blob/master/LICENSE.md New BSD License
 */

@@ -0,0 +1,63 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
Copy link
Member

Choose a reason for hiding this comment

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

Please replace the header with this:

/**
 * @see       https://github.com/zendframework/zend-log for the canonical source repository
 * @copyright Copyright (c) 2015-2017 Zend Technologies USA Inc. (http://www.zend.com)
 * @license   https://github.com/zendframework/zend-log/blob/master/LICENSE.md New BSD License
 */

@Erikvv
Copy link
Author

Erikvv commented Oct 13, 2017

Bit of a misunderstanding, turns out our company will not be using this library in the near future.

I'm closing the PR.

@Erikvv Erikvv closed this Oct 13, 2017
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.

3 participants