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

[WebProfilerBundle] Fixed error from unset twig variable#18457

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

Conversation

@simonsargeant
Copy link
Contributor

QA
Branch?2.8
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

Minor bug, fixes error from twig variable which was removed in 2.8
Here was where it was originally sethttps://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig#L69

Replaced with expected class name

@javiereguiluz
Copy link
Member

@simonsargeant thanks for fixing this. It looks good to me 👍

However, there is a big problem with the code we use for this:

{%if'n/a'!=collector.debug %}    ...{%endif %}

collector.debug is a boolean value and we're comparing it with a string (n/a). In this casen/a is transformed totrue and the comparison isif true != collector.debug. That's why we never see this information in the toolbar.

Should we fix that in this PR or create a new PR?

@stof
Copy link
Member

stof commentedApr 6, 2016

@javiereguiluz do we actually need the color here ?

@javiereguiluz
Copy link
Member

@stof I don't have a strong opinion about this. But the color may be consistent with other colored blocks in that panel and it doesn't hurt:

ColorNo Color
colorno_color

@simonsargeant
Copy link
ContributorAuthor

@javiereguiluz I think this is now expected behavior as collector.debug could be boolean or 'n/a'https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/DataCollector/ConfigDataCollector.php#L69. Does not show the debug status if 'n/a', shows activated or deactivated if true or false.

@javiereguiluz
Copy link
Member

@simonsargeant yes, the collector can betrue,false or'n/a' ... but the problem is in the condition:

Envcollector.debug'n/a' != collector.debugResult
devtruefalseYou don't see this in the toolbar
prodfalsetrueYou see this in the toolbar ... but there is no toolbar because we are inprod
-n/afalseYou don't see this in the toolbar ... but I've never faced this condition

So the only scenario where this istrue is in theprod environment, where the toolbar is not displayed

@simonsargeant
Copy link
ContributorAuthor

@javiereguiluz Sorry, I meant I had updated the PR so the behaviour is now:

collector.debug'n/a' is not same as collector.debugResult
truetrueShown in toolbar (green)
falsetrueShown in toolbar (red)
'n/a'falseNot shown in toolbar

@javiereguiluz
Copy link
Member

👍 Thanks@simonsargeant

@stof
Copy link
Member

stof commentedApr 7, 2016

👍

@fabpot
Copy link
Member

Thank you@simonsargeant.

fabpot added a commit that referenced this pull requestApr 7, 2016
…simonsargeant)This PR was squashed before being merged into the 2.8 branch (closes#18457).Discussion----------[WebProfilerBundle] Fixed error from unset twig variable| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aMinor bug, fixes error from twig variable which was removed in 2.8Here was where it was originally sethttps://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig#L69Replaced with expected class nameCommits-------3e2c4c9 [WebProfilerBundle] Fixed error from unset twig variable
@fabpotfabpot closed thisApr 7, 2016
This was referencedApr 29, 2016
ostrolucky pushed a commit to ostrolucky/symfony that referenced this pull requestMar 25, 2018
…iable (simonsargeant)This PR was squashed before being merged into the 2.8 branch (closessymfony#18457).Discussion----------[WebProfilerBundle] Fixed error from unset twig variable| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aMinor bug, fixes error from twig variable which was removed in 2.8Here was where it was originally sethttps://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig#L69Replaced with expected class nameCommits-------3e2c4c9 [WebProfilerBundle] Fixed error from unset twig variable
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

@simonsargeant@javiereguiluz@stof@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp