Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[DependencyInjection] Support @> as a shorthand for !service_closure in YamlFileLoader #59257

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 6, 2025
Merged

[DependencyInjection] Support @> as a shorthand for !service_closure in YamlFileLoader #59257

merged 1 commit into from
Jan 6, 2025

Conversation

chx
Copy link
Contributor

@chx chx commented Dec 18, 2024 •
edited by GromNaN
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #59255
License MIT

@carsonbot carsonbot added this to the 7.3 milestone Dec 18, 2024
GromNaN
GromNaN reviewed Dec 18, 2024
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

I appreciate the idea, but I don't like this new symbol.

  • It creates an extra way of doing the same thing.
  • It's not expressive: impossible to search for these special characters in the doc to understand what they do.
  • It looks like the symbol @> in Postgres that is a totally different feature.

...ny/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_short_service_closure.yml Outdated Show resolved Hide resolved
@GromNaN GromNaN added Yaml and removed Bug labels Dec 18, 2024
@carsonbot carsonbot changed the title Support @> as a shorthand for !service_closure in YamlFileLoader (Iss… [Yaml] Support @> as a shorthand for !service_closure in YamlFileLoader (Iss… Dec 18, 2024
@GromNaN GromNaN changed the title [Yaml] Support @> as a shorthand for !service_closure in YamlFileLoader (Iss… [Yaml] Support @> as a shorthand for !service_closure in YamlFileLoader Dec 18, 2024
nicolas-grekas
nicolas-grekas reviewed Dec 26, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

works for me
but let's add test cases with @>?bar (and make it work)

src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php Outdated Show resolved Hide resolved
@wouterj
Copy link
Member

wouterj commented Dec 27, 2024

I'm also not so sure if we need this shortcut. The @, @? and %...% Yaml conventions were introduced in a time where we didn't have proper tag support for Yaml. The tags are the official Yaml syntax to define custom data types (e.g. type a specific string or object). With good tag support in the Yaml component, we can use the official features instead of using our own conventions now. This remove magic and makes it easier to understand.

@nicolas-grekas
Copy link
Member

Yet I would not like to have to use !reference foo instead of @foo, so that these shortcuts are welcome IMHO.

@carsonbot carsonbot changed the title [Yaml] Support @> as a shorthand for !service_closure in YamlFileLoader [DependencyInjection] Support @> as a shorthand for !service_closure in YamlFileLoader Dec 31, 2024
GromNaN
GromNaN approved these changes Jan 2, 2025
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Ok, if that is convenient.

nicolas-grekas
nicolas-grekas approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I fixed the remaining comments and added a line to the changelog of the component.

@fabpot
Copy link
Member

fabpot commented Jan 6, 2025

Thank you @chx.

@fabpot fabpot merged commit a36b981 into symfony:7.3 Jan 6, 2025
11 checks passed
@fabpot fabpot mentioned this pull request Jan 6, 2025
Closed
@OskarStark OskarStark changed the title [DependencyInjection] Support @> as a shorthand for !service_closure in YamlFileLoader [DependencyInjection] Support @> as a shorthand for !service_closure in YamlFileLoader Jan 9, 2025
@chx chx deleted the 7.3 branch February 4, 2025 02:45
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@GromNaN GromNaN GromNaN approved these changes

Assignees
No one assigned
Labels
DependencyInjection Feature Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Support @> as a shorthand for !service_closure in YamlFileLoader
8 participants
@chx @wouterj @nicolas-grekas @fabpot @GromNaN @OskarStark @xabbuh @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.