8000 [DI] Strip New Lines and Whitespace for multi-line XML Parameters for Class Initialization by kylecannon · Pull Request #6123 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Strip New Lines and Whitespace for multi-line XML Parameters for Class Initialization #6123

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

[DI] Strip New Lines and Whitespace for multi-line XML Parameters for Class Initialization #6123

wants to merge 4 commits into from

Conversation

kylecannon
Copy link

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

The main purpose of this patch is when formatting your parameters in an XML configuration to use multiple lines due to the long length.

Example:

<parameters>
    <parameter key="kc_media.credential.factory.aws.class">KC\Bundle\MediaBundle\DependencyInjection\Factory\Credential\AwsCredentialFactory</parameter>
</parameters>

May be reformatted into the following in PHPStorm:

<parameters>
    <parameter key="kc_media.credential.factory.aws.class">
        KC\Bundle\MediaBundle\DependencyInjection\Factory\Credential\AwsCredentialFactory
    </parameter>
</parameters>

After reformatting, you would get the following error:

ReflectionException: Class
KC\Bundle\MediaBundle\DependencyInjection\Factory\Credential\AwsCredentialFactory
does not exist

By stripping the new lines and trimming the white space will allow the proper initialization of the class .

Koc
Koc reviewed Nov 26, 2012
View reviewed changes
src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -759,8 +759,9 @@ private function createService(Definition $definition, $id)

$service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments);
} else {
$r = new \ReflectionClass($parameterBag->resolveValue($definition->getClass()));

$classDefinition = preg_replace('~[\r\n]+~', '', trim($parameterBag->resolveValue($definition->getClass())));
Copy link
Contributor

Choose a reason for hiding this comment

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

is trim not enouth?

Copy link
Author

Choose a reason for hiding this comment

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

You're correct. I'll change the commit to using just trim instead.

fabpot
fabpot reviewed Nov 28, 2012
View reviewed changes
src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -759,8 +759,9 @@ private function createService(Definition $definition, $id)

$service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments);
} else {
$r = new \ReflectionClass($parameterBag->resolveValue($definition->getClass()));

$classDefinition = trim($parameterBag->resolveValue($definition->getClass()));
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to create a temp variable.

@fabpot
Copy link
Member
fabpot commented Nov 28, 2012

A similar discussion happened some time ago, and we decided against it. Here; you propose to trim only the class name, but why just this one and not some other ones? This looks quite arbitrary, no?

@fabpot
Copy link
Member
fabpot commented Dec 5, 2012

Any feedback regarding my above concern?

@kylecannon
Copy link
Author

To be honest that was the only issue that was throwing an Exception when I had reformatted my XML so I found the underlying issue and patched it so I could continue to develop.

Another possible idea would be if there was a filter we can pass in the parameter to have it automatically trim a value?

Example:

<parameters>
    <parameter key="kc_media.credential.factory.aws.class">
        KC\Bundle\MediaBundle\DependencyInjection\Factory\Credential\AwsCredentialFactory
    </parameter>
</parameters>

To:

<parameters>
    <parameter key="kc_media.credential.factory.aws.class" filter="trim">
        KC\Bundle\MediaBundle\DependencyInjection\Factory\Credential\AwsCredentialFactory
    </parameter>
</parameters>

Ultimately though I believe that the patch I did was necessary due to having a problem calling a class with line breaks or whitespace around it that is unintentional.

@fabpot
Copy link
Member
fabpot commented Dec 11, 2012

I'm closing this PR as it introduces a very specific behavior. Moreover, this fix would not be enough as we are using the class name in many more places where this hack should be propagated too.

@fabpot fabpot closed this Dec 11, 2012
@gnugat gnugat mentioned this pull request Aug 4, 2013
Closed
@jakzal jakzal mentioned this pull request Dec 27, 2014
Closed
Sign up for free to 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.
3 participants
@kylecannon @fabpot @Koc
0