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

[DependencyInjection] Trim constants#8661

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
gnugat wants to merge3 commits intosymfony:masterfromgnugat:dic-trim-constants
Closed

[DependencyInjection] Trim constants#8661

gnugat wants to merge3 commits intosymfony:masterfromgnugat:dic-trim-constants

Conversation

@gnugat
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

When indenting constant parameters in XML configuration files, an exception is throwed.
This PR simply adds:

  • a test for simple constant support in XML configuration files
  • a test for indented constant support in XML configuration files (proving the issue)
  • a fix by trimming constants

Example to reproduce the bug

Configuration sample:

<?xml version="1.0" ?><containerxmlns="http://symfony.com/schema/dic/services"xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">  <parameters>    <parameterkey="simple_constant"type="constant">PHP_EOL</parameter>    <parameterkey="indented_constant"type="constant">        PHP_EOL    </parameter>  </parameters></container>

Code sample:

<?php$container =newContainerBuilder();$loader =newXmlFileLoader($container,newFileLocator(__DIR__);$loader->load('indented_constant.xml');$container->getParameterBag()->all();

Result:

constant(): Couldn't find constant         PHP_EOL

Notice the fact that the error message is not:

constant(): Couldn't find constant PHP_EOL

Loïc Chardonnet added3 commitsAugust 3, 2013 23:20
Created a test for simple constant support in XML configuration files.
Added a fixture with an indented constant. This currently throws anexception:"""constant(): Couldn't find constant        PHP_EOL"""This might be caused because the parameter value is not trimmed.
Fixed the exception throwed when indenting constants in XMLconfiguration files, by simply trimmed constants.
@wouterj
Copy link
Member

This is already proposed multiple times and refused. The reason (see#4463 ):

The rule is simple enough: All characters are significant, including whitespace.
-- fabpot

@fabpotfabpot closed thisAug 4, 2013
@gnugat
Copy link
ContributorAuthor

Then maybe the first commit, which adds a test for constant support, can still be useful?

I'd be curious to see cases where whitespaces are used for constant names.

@wouterj
Copy link
Member

Whitespaces for constant names are not common, but we can't make an exception for some values. See also#3646

@gnugat
Copy link
ContributorAuthor

@wouterj yes, I was looking at this PR, and also at its related issue#3644 and another PR#6123.

While I still don't understand why this behavior should be kept, I think I'm going to take another approach: I'll submit a PR to add a test on constants support and another one on the component documentation.

@wouterj
Copy link
Member

👍 for both of those PR you're going to create. I missed the test commit and we also don't have this documented.

@gnugatgnugat deleted the dic-trim-constants branchAugust 4, 2013 10:33
@stof
Copy link
Member

stof commentedAug 4, 2013

@gnugat People may want to define some parameters with whitespaces in it. If we are trimming the value, it becomes impossible

fabpot added a commit that referenced this pull requestAug 8, 2013
This PR was squashed before being merged into the master branch (closes#8663).Discussion----------[DependencyInjection] Test constants| Q             | A| ------------- | ---| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | **yes**| License       | MITAdded a test for constant support in XML configuration files.Related PR:#8661Commits-------9acedb7 [DependencyInjection] Test constants
garak pushed a commit to garak/symfony-docs that referenced this pull requestNov 2, 2013
In XML configuration, the value between` parameter` tags isn't trimmed,which can lead to unexpected behavior.Seesymfony/symfony#8661
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gnugat@wouterj@stof@fabpot

[8]ページ先頭

©2009-2025 Movatter.jp