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

POC adding of href link to the dump call location #31446

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ktherage
Copy link

commented May 9, 2019

Q A
Branch? master
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? not tested yet
Fixed tickets #30830
License MIT
Doc PR

see #30830

@ktherage ktherage force-pushed the ktherage:poc-var-dump branch from 5ff617f to c7cc20a May 9, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone May 9, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented May 21, 2019

The implementation relies on a statefull service. This should be avoided.
If we need dumping some context, it should be added to the Data object to me.
Then, dumpers could read it and do things appropriately.
See also SourceContextProvider which has more intensive logic for extracting the stack trace.

@ktherage ktherage force-pushed the ktherage:poc-var-dump branch from 2572ddd to 4037141 May 24, 2019

@ktherage

This comment has been minimized.

Copy link
Author

commented May 24, 2019

@nicolas-grekas Thank you for the feed back. I've changed the implementation.

Do you have an idea on how can i insulate this new feature and set her only for CliDumper?

Because currently, without forcing file and line to null in getDump method of VarDumperTestTrait it has impact on all other tests and so maybe on functionnalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.