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

[VarDumper] Display fully qualified title #33486

Merged
merged 2 commits into from Sep 8, 2019

Conversation

@pavinthan
Copy link
Contributor

commented Sep 6, 2019

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

We can see the objects with namespace that help us to navigate to the file easily.

Before: These are diffrent Collection class
Screen Shot 2019-09-06 at 1 02 37 PM

Now: we can see the diffrent
Screen Shot 2019-09-06 at 1 02 20 PM

@pavinthan pavinthan changed the title Display fully qualified title [VarDumper] - Display fully qualified title Sep 6, 2019

@pavinthan pavinthan marked this pull request as ready for review Sep 6, 2019

@pavinthan pavinthan changed the title [VarDumper] - Display fully qualified title [VarDumper] Display fully qualified title Sep 6, 2019

@fancyweb

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

You can already see the FQCN when you move your mouse cursor over the class short name (thanks to the title HTML attr), so not sure it's really needed. Most of the time you know the class you are dumping.

@pavinthan

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

@fancyweb Yes We can able to see the title, could you able to copy that? ( you may need to inspect the element to copy that ) and sometime we are not sure about class we dumping ( eg: think about external libraries ), please advice me feather.

Thanks.

@pavinthan pavinthan changed the title [VarDumper] Display fully qualified title [VarDumper] New feature - Display fully qualified title Sep 6, 2019

@pavinthan pavinthan changed the title [VarDumper] New feature - Display fully qualified title [VarDumper] Improve Display fully qualified title Sep 6, 2019

@pavinthan pavinthan changed the title [VarDumper] Improve Display fully qualified title [VarDumper] Display fully qualified title Sep 6, 2019

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 6, 2019

@nicolas-grekas
Copy link
Member

left a comment

Thank you for the proposal.

I pushed some changes on your fork: the full class is now displayed only for the root object. When an object is nested, we display an ellipsed class instead. I think that's a better compromise so that the output if not crippled with class names, while still allowing copy/paste.

Here is a screenshot (the green class name is unrelated but shows how we display strings when we know they represent a class name, for comparison):

image

@fabpot
fabpot approved these changes Sep 8, 2019
@fabpot

This comment has been minimized.

Copy link
Member

commented Sep 8, 2019

Thank you @pavinthan.

fabpot added a commit that referenced this pull request Sep 8, 2019
feature #33486 [VarDumper] Display fully qualified title (pavinthan, …
…nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[VarDumper] Display fully qualified title

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

We can see the objects with namespace that help us to navigate to the file easily.

Before: These are diffrent Collection class
<img width="203" alt="Screen Shot 2019-09-06 at 1 02 37 PM" src="https://user-images.githubusercontent.com/13897936/64410319-663f0500-d0a8-11e9-98d5-743e2ccf2737.png">

Now: we can see the diffrent
<img width="376" alt="Screen Shot 2019-09-06 at 1 02 20 PM" src="https://user-images.githubusercontent.com/13897936/64410304-60e1ba80-d0a8-11e9-9cb1-f88c0f8c3de9.png">

Commits
-------

a8252a2 [VarDumper] display ellipsed FQCN for nested classes
84682ea [VarDumper] Display fully qualified title

@fabpot fabpot merged commit a8252a2 into symfony:4.4 Sep 8, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@pavinthan

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

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