Skip to content

[DependencyInjection] Allow loading and dumping tags with an attribute named "name" #18802

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

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Aug 30, 2023

Not an easy one to fully understand. Friendly ping @stof as you participated to the issue 🙂 Thanks!

Fix #13613

@carsonbot carsonbot added this to the 6.4 milestone Aug 30, 2023
@alexandre-daubois alexandre-daubois changed the base branch from 6.4 to 5.4 August 30, 2023 09:40
@@ -515,6 +515,45 @@ To answer this, change the service declaration:
;
};

.. tip::

In XML and YAML format, you may provide the tag an attribute called ``name``.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this "feature".

  • Is the name attribute special here?
  • If it's special: what does that imply?
  • If it's not special: why mention this at all? If it's just like any other attribute, why mention this name attribute in particular?

Thanks!

Copy link
Member Author

@alexandre-daubois alexandre-daubois Aug 31, 2023

Choose a reason for hiding this comment

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

The name attribute is not special itself. Actually, this tip indicates that you can use an attribute named name, which wasn't possible before 5.1. Also, this shows that if you need to add a name attribute on a tag, you must use a "special" syntax. Otherwise, the name attribute will be interpreted as the tag name, not as an attribute of it.

By doing

MailerSendmailTransport:
    tags:
        - { name: 'app.mail_transport', alias: 'sendmail' }

You declare a tag with the app.mail_transport name and an attribute called alias. This is a bit misleading because alias and name keys are at the same level. If you actually need an attribute called name, you must do the following:

MailerSmtpTransport:
    arguments: ['%mailer_host%']
        tags:
            - app.mail_transport: { name: 'arbitrary-value', alias: 'smtp' }

It took me some time to fully understand what this PR was all about 😄

Copy link
Member

Choose a reason for hiding this comment

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

By default, the name XML attribute contains the tag name, and so cannot be used to set an attribute named name. As of 5.1, there is a way to use name as an attribute by passing the tag name in the body of the XML element instead.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your explanation. Now I can fully understand what's happening here. So, I've done some tweaks while merging to hopefully make it less confusing to readers: 6e6565b

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect! 🙂

@javiereguiluz javiereguiluz merged commit 8683ab5 into symfony:5.4 Aug 31, 2023
@javiereguiluz
Copy link
Member

Alex, thanks a lot for contributing this ... and congrats on completing the Symfony 5.1 milestone 🎉

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.

[DI] allow loading and dumping tags with an attribute named "name"
5 participants