-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Fix support for inner collections when using <bind>
#50061
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
Conversation
zerustech
commented
Apr 19, 2023
Q | A |
---|---|
Branch? | 6.2 |
Bug fix? | yes |
New feature? | no |
Deprecations? | no |
Tickets | Fix #50046 |
License | MIT |
<bind>
@@ -276,6 +276,7 @@ | |||
<xsd:complexType name="bind" mixed="true"> | |||
<xsd:choice maxOccurs="unbounded"> | |||
<xsd:element name="bind" type="argument" minOccurs="0" maxOccurs="unbounded" /> | |||
<xsd:element name="argument" type="argument" minOccurs="0" maxOccurs="unbounded" /> |
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.
On closer review, I feel like I mislead you: maybe we can just set the type to "bind" on the line above and remove this one.
Yes, that would forbid XML files that use <argument>
nested inside <bind>
. But these were ignored, isn't it? In that case, I would definitely be fine "breaking" those files, proved they are really misleading, thus broken also at the moment.
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.
This will cause an XML validation error: since attribute key
was optional for argument
elements, but is required for bind
elements now, any XML file that contains an inner <bind>
element without a key
attribute will not be parsed successfully.
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.
Got it thanks! I updated the PR to add a new "bind_argument" complex type.
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.
Thanks, that works perfectly.
069c3b2
to
4634eeb
Compare
Thank you @zerustech. |
No problem. |