-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Add normalization to tag options #9489
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<?xml version="1.0" ?> | ||
|
||
<container xmlns="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"> | ||
<services> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please be consistent with indentation |
||
<service id="foo" class="BarClass"> | ||
<tag name="foo_tag" | ||
some-option="cat" | ||
some_option="ciz" | ||
other-option="lorem" | ||
an_other-option="ipsum" | ||
/> | ||
</service> | ||
</services> | ||
</container> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -231,6 +231,29 @@ public function testLoadServices() | |
$this->assertFalse($aliases['another_alias_for_foo']->isPublic()); | ||
} | ||
|
||
public function testParsesTags() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. testParseTags makes more sense if we try to read the method name, check test method below for consistency |
||
{ | ||
$container = new ContainerBuilder(); | ||
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); | ||
$loader->load('services10.xml'); | ||
|
||
$services = $container->findTaggedServiceIds('foo_tag'); | ||
$this->assertCount(1, $services); | ||
|
||
foreach ($services as $id => $tagAttributes) { | ||
foreach ($tagAttributes as $attributes) { | ||
$this->assertArrayHasKey('other_option', $attributes); | ||
$this->assertEquals('lorem', $attributes['other_option']); | ||
$this->assertArrayHasKey('other-option', $attributes, 'unnormalized tag attributes should not be removed'); | ||
|
||
$this->assertEquals('ciz', $attributes['some_option'], 'no overriding should be done when normalizing'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the note should land on the next line |
||
$this->assertEquals('cat', $attributes['some-option']); | ||
|
||
$this->assertArrayNotHasKey('an_other_option', $attributes, 'normalization should not be done when an underscore is already found'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so it does not normalize other - 's ? this sounds weird There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is consistent with the way the configuration processing works: it you are mixing dashes and underscores, it assumes that it is not a case where the dash is here because of the difference between the XML and YAML conventions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah 👶 I read it also above for BC purposes thanks @stof |
||
} | ||
} | ||
} | ||
|
||
public function testConvertDomElementToArray() | ||
{ | ||
$doc = new \DOMDocument("1.0"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?xml version="1.0" ?> | ||
|
||
<container xmlns="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"> | ||
<services> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think indentation needs to be here 4 spaces |
||
<service id="foo" class="BarClass" /> | ||
</services> | ||
</container> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess that piece of logic goes together with the one below