[DependencyInjection] Fix named args support in ChildDefinition #22981
Conversation
ec191ba to
70105d1
Compare
|
I would also update the exception message. "$index must be an integer." is not really right anymore. |
5881fea to
9245400
Compare
|
@xabbuh updated, |
|
@dunglas I meant the one in |
9245400 to
91985fb
Compare
|
@xabbuh updated |
|
The exception message in
is also not correct anymore in case an argument exists as a string. Then you can't actually access the argument with the given range. |
|
Seems false alarm, I'll remove confusing comment to avoid indexing and leading others to wrong place ;) |
|
Despite my false alarm in #22979 ;) still doesn't work with PR applied locally: Service description is quite trivial <service id="sulu_admin.js_config_pool" class="%sulu_admin.js_config_pool.class%"/> |
|
@yukoff named args must start with a |
|
My previous comment is not actual as of sulu/sulu#3379 - works now. @nicolas-grekas not quite sure I got to where you're referencing... ;) |
|
I'm referencing the error message you pasted just above: it looks legit to me. |
|
Oh... Anyway it seems not actual any more, thanks! |
|
Thank you @dunglas. |
…ition (dunglas) This PR was squashed before being merged into the 3.3 branch (closes #22981). Discussion ---------- [DependencyInjection] Fix named args support in ChildDefinition | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Following @Tobion's review of #21383. Commits ------- 1ab3e41 [DependencyInjection] Fix named args support in ChildDefinition
Following @Tobion's review of #21383.