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

[FrameworkBundle] Fix a bug where a color tag will be shown when passing an antislash #25308

Conversation

Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Dec 4, 2017

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

You can see in the reproducer when running bin/console debug:container that there an error in the ouput (like in the issue) when using a class with \ in the service name.

This PR fix this wrong output. (even if that feels more developer thingy when there are xml everywhere ;)

@@ -226,6 +226,9 @@ protected function describeContainerServices(ContainerBuilder $builder, array $o
$rawOutput = isset($options['raw_text']) && $options['raw_text'];
foreach ($this->sortServiceIds($serviceIds) as $serviceId) {
$definition = $this->resolveServiceDefinition($builder, $serviceId);
if ('\\' === substr($serviceId, -1)) {
$serviceId = substr($serviceId, 0, -1);
Copy link
Member

Choose a reason for hiding this comment

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

But this means that the displayed service ID will be wrong, doesn't it?

Copy link
Contributor Author

@Simperfit Simperfit Dec 4, 2017

Choose a reason for hiding this comment

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

Yeah, you are right, we should escape without removing it. I was not thinking about that, it feels strange to have that kind of service, maybe we should do something before displaying it ?

Copy link
Member

Choose a reason for hiding this comment

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

I think you need to take a look at the OutputFormatter::escape() method

Copy link
Contributor Author

@Simperfit Simperfit Dec 4, 2017

Choose a reason for hiding this comment

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

Thanks @xabbuh that's what I needed!

@Simperfit Simperfit force-pushed the bugfix/fix-a-bug-where-an-antislash-will-break-the-ouput branch from 987fbae to a9dbec8 Compare December 4, 2017 16:18
@Simperfit Simperfit force-pushed the bugfix/fix-a-bug-where-an-antislash-will-break-the-ouput branch from a9dbec8 to 890edf7 Compare December 4, 2017 16:41
@nicolas-grekas
Copy link
Member

I'm removing the photo from the description, it's a bit weird to see your face when reviewing the PR :)
For which branch is the bug fix? This targets master right now, not sure it's correct.

@Simperfit Simperfit changed the base branch from master to 3.3 December 4, 2017 16:51
@Simperfit
Copy link
Contributor Author

Simperfit commented Dec 4, 2017

@nicolas-grekas it's for 3.3 since the bug does not exist before.

@fabpot
Copy link
Member

fabpot commented Dec 4, 2017

Thank you @Simperfit.

@fabpot fabpot merged commit 890edf7 into symfony:3.3 Dec 4, 2017
fabpot added a commit that referenced this pull request Dec 4, 2017
…n when passing an antislash (Simperfit)

This PR was merged into the 3.3 branch.

Discussion
----------

[FrameworkBundle] Fix a bug where a color tag will be shown when passing an antislash

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

You can see in the [reproducer](Simperfit/symfony-reproducer@e6509ff) when running `bin/console debug:container` that there an error in the ouput (like in the issue) when using a class with `\` in the service name.

This PR fix this wrong output. (even if that feels more developer thingy when there are xml everywhere ;)

Commits
-------

890edf7 [FrameworkBundle] Fix a bug where a color tag will be shown when passing an antislash
This was referenced Dec 4, 2017
@Simperfit Simperfit deleted the bugfix/fix-a-bug-where-an-antislash-will-break-the-ouput branch December 5, 2017 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants