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.

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

Improve support for anonymous classes #28218

Merged
merged 1 commit into from Aug 24, 2018

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Aug 17, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Before:
image

After:
image

Same in other places, e.g. Console failures.

@@ -753,12 +753,20 @@ protected function doRenderException(\Exception $e, OutputInterface $output)
do {
$message = trim($e->getMessage());
if ('' === $message || OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
$title = sprintf(' [%s%s] ', \get_class($e), 0 !== ($code = $e->getCode()) ? ' ('.$code.')' : '');
$class = \get_class($e);
$class = 'c' === $class[0] && 0 === strpos($class, "class@anonymous\0") ? get_parent_class($class).'@anonymous' : $class;
Copy link
Contributor

@Kocal Kocal Aug 17, 2018

Choose a reason for hiding this comment

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

I'm curious about \0, I know it represents a null byte, but it's the first time I see it in PHP code.
Does it improve perf or something like that? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot print a null char using null as this would result in an empty string 😃

Copy link
Member Author

Choose a reason for hiding this comment

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

That's desired, see #28218 (comment) (or maybe you mean something else... :))

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Aug 17, 2018 via email

@Kocal
Copy link
Contributor

Kocal commented Aug 17, 2018

Oh okay, thanks 👍

@nicolas-grekas nicolas-grekas force-pushed the anonymous branch 3 times, most recently from da351cd to 6380b7d Compare August 18, 2018 20:45
@@ -142,6 +142,12 @@ public function getMessage()

public function setMessage($message)
{
if (false !== strpos($message, "class@anonymous\0")) {
$message = preg_replace_callback('/class@anonymous\x00.*?\.php0x?[0-9a-fA-F]++/', function ($m) {
return \class_exists($m[0], false) ? get_parent_class($m[0]).'@anonymous' : $m[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

technically this can occur for anon. clases without a parent class.. do we care? Same for setClass.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes we do, and this is handled: the result will be just @anonymous, as the VarDumper component already does.

@@ -518,21 +524,25 @@ public function handleException($exception, array $error = null)
$handlerException = null;

if (($this->loggedErrors & $type) || $exception instanceof FatalThrowableError) {
if (false !== strpos($message = $exception->getMessage(), "class@anonymous\0")) {
($message = new FlattenException())->setMessage($message);
Copy link
Member

Choose a reason for hiding this comment

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

naming the temporary variable $message here looks weird, as it does not contain the message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed by making FlattenException fluent.

@nicolas-grekas nicolas-grekas merged commit e41ced2 into symfony:master Aug 24, 2018
nicolas-grekas added a commit that referenced this pull request Aug 24, 2018
This PR was merged into the 4.2-dev branch.

Discussion
----------

Improve support for anonymous classes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Before:
![image](https://user-images.githubusercontent.com/243674/44276933-5c522600-a249-11e8-8bcc-6d55c9fa5a5e.png)

After:
![image](https://user-images.githubusercontent.com/243674/44276912-4cd2dd00-a249-11e8-943f-b6b0d0eb8908.png)

Same in other places, e.g. Console failures.

Commits
-------

e41ced2 Improve support for anonymous classes
@nicolas-grekas nicolas-grekas deleted the anonymous branch August 24, 2018 13:46
public function setStatusCode($code)
{
$this->statusCode = $code;

return $this;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need fluent interface here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need it. It still makes it easier to use the class.

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
This was referenced Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants