Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Var-Dumper] added feature to set default nodes collapsed#18148

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
MGDSoft wants to merge11 commits intosymfony:masterfromMGDSoft:ticket_16741
Closed

[Var-Dumper] added feature to set default nodes collapsed#18148

MGDSoft wants to merge11 commits intosymfony:masterfromMGDSoft:ticket_16741

Conversation

@MGDSoft
Copy link
Contributor

QA
Branchmaster
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#16741
LicenseMIT
Doc PR

modify js htmldumper and add options variable to configure js

Code to test it

$array = ['level 1','level 1',            ['level 2','level 2'],            [['level 3'],['level 3']],        ];$cloner =newVarCloner();$dumper =newHtmlDumper();        VarDumper::setHandler(function ($var)use ($cloner,$dumper) {$dumper->dump($cloner->cloneVar($var));        });// default (set to 1)dump($array);// all collapsed$dumper->setJsProperties(array('collapsedByDefaultNodesHigherThan' =>0 ));dump($array);$dumper->setJsProperties(array('collapsedByDefaultNodesHigherThan' =>3 ));dump($array);

@MGDSoft
Copy link
ContributorAuthor

I dont know why travis fails for php 5.6 && 7 and Its fine for 5.5 && hhvm. my code is 80% js

@nicolas-grekas
Copy link
Member

I understand the need but the current interface (method) is really tailored to one single use case.
What if now I want to expand key foo.bar.baz? Then should we add a new method for each way of collapsing/expanding we can think of? Is there a more generic solution?

@MGDSoft
Copy link
ContributorAuthor

I think it is the best generic solution.
If u want replace foo.bar.baz the steps are

  • add in array jsPropertiesDefault default Value like 'fooBarBaz' => 'hi'
  • in js where used replace hardcode value to var fooVar = options.fooBarBaz
  • To override default values call setJsProperties(['fooBarBaz' => 'bye!'])

@MGDSoft
Copy link
ContributorAuthor

ping@nicolas-grekas

@nicolas-grekas
Copy link
Member

I think its worth it. Here are a few suggestions:

I'd renamesetJsProperties() tosetDisplayOptions().
I'd add a third argument todump(), called$displayOptions, that overwrites the default options when provided (and thus keep only one$this->displayOptions property, that holds the default config).
I'd then allow more display options. Dump limiting is controlled by a few more options, amongst them:

  1. maxItems: max total number of items to dump
  2. maxDepth: well, max depth
  3. maxItemsPerDepth: max number of items per depth level
  4. maxStringLength: max sumber of chars for strings
  5. maxStringWidth: max number of characters per line for strings

1. and5. are not applicable, but2.,3. and4. could also exist to control default collapsing of elements in the browser. I'd keep the same wording (maxDepth instead of collapsedByDefaultNodesHigherThan, etc.).

WDYT?

@MGDSoft
Copy link
ContributorAuthor

Ey Nicolas thx for your reply!.

  • I think displayOptions var instead of (propertiesJs && propertiesJsDefault ) its perfect
  • Add displayOptions to HtmlDumper->dump() its ok and we can also add to VarDumper::dump() because it can be used for CliDumper (in the future?), isn't it?
  • maxItemsPerDepth and maxStringLength its ok, but maxDepth I think is ambiguos. It would be maxDepthByDefault or something like that.

I have a another question. Sorry if its a dumb question (is my 1º pullrequest here ...)
If I break a test like my pullrequest62c6ab5 (build:https://travis-ci.org/symfony/symfony/builds/115623557 deps=high)
I think this test is looking for a bc breaks, because its executing in branch 3.0, and in my local machine all test are passed.

What is the process to correct or warn of this problem?
Thanks!

@nicolas-grekas
Copy link
Member

add to VarDumper::dump() because it can be used for CliDumper (in the future?)

This doesn't apply to CliDumper, because we are talking about collapsing options, which doesn't exist on the cli. We already handle these kind of limits (max depth) at the state extraction level, they are not exposed by the dump functions but could be one day (see#17290).

but maxDepth I think is ambiguos

that's the way it is now and changing it would be a bc break, no way to change :)

If I break a test

We'll look at them once the feature is ready, but the fix usually is to raise the lowest required versions in the composer.json file of the failing components (at least for the deps=low matrix line).

$line ='<script>
Sfdump = window.Sfdump || (function (doc) {
var defaultOptions ='.json_encode($this->jsPropertiesDefault).';

Choose a reason for hiding this comment

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

since the actual max depth is provided as argument to the Sfdump function, there is no need to put the default value here (and thus neither no need to handle any merging (extend))

@nicolas-grekas
Copy link
Member

Status: needs work

@fabpot
Copy link
Member

What's the status of this PR?

@MGDSoft
Copy link
ContributorAuthor

currently working, I was on vacation

@MGDSoft
Copy link
ContributorAuthor

Hi,

If I use a single var to displayOptions, I break a some test like TwigDumpExtension.
What should we do to fix this problem?

I think it would be good, add a parameter in TwigDumpExtension to change displayOptions, but it can have a random number of arguments.

I am stuck :/

@MGDSoft
Copy link
ContributorAuthor

The option maxItemsPerDepth require modify allsamp elements toul because each row must be delimited by a element likeli. WDYT?

modify js htmldumper and add attr in root node
modify js htmldumper and add attr in root node
modify js htmldumper and add options to configure js
modify js htmldumper and add options to configure js
modify js htmldumper and add options to configure js
@nicolas-grekas
Copy link
Member

Let's do maxItemsPerDepth later in an other PR then. I can't look at this right now but will definitely try it soon!

@nicolas-grekas
Copy link
Member

Continued in#18948
Thank you@MGDSoft !

fabpot added a commit that referenced this pull requestJun 28, 2016
…ions (MGDSoft, nicolas-grekas)This PR was merged into the 3.2-dev branch.Discussion----------[VarDumper] Add maxDepth & maxStringLength display options| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#16741| License       | MIT| Doc PR        | -Takes over#18148 to add display options to html dumps.Status: needs workCommits-------998ff33 [VarDumper] Tweak display options implementation58eb665 [VarDumper] Add maxDepth & maxStringLength display options
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MGDSoft@nicolas-grekas@fabpot@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp