Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

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

Sign up
Appearance settings

[DI] Allow for invokable event listeners#25275

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromro0NL:eventlistener
Jan 19, 2018
Merged

[DI] Allow for invokable event listeners#25275

fabpot merged 1 commit intosymfony:masterfromro0NL:eventlistener
Jan 19, 2018

Conversation

@ro0NL
Copy link
Contributor

@ro0NLro0NL commentedDec 2, 2017
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#...
LicenseMIT
Doc PRsymfony/symfony-docs#...

Inspired by#24637 /#25259. This adds invokable support for event listeners :)

Some\Foo:tags:[{ name: kernel.event_listener, event: kernel.request }]
class Foo {publicfunction__invoke(GetResponseEvent$event) { }}

jvasseur, dmaicher, dunglas, yceruto, sstok, theofidry, Toflar, and artyuum reacted with thumbs up emoji
Copy link
Member

@dunglasdunglas left a comment
edited
Loading

Choose a reason for hiding this comment

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

❤️

@Simperfit
Copy link
Contributor

I will use this one !

$event['method'] ='on'.preg_replace_callback(array(
'/(?<=\b)[a-z]/i',
'/[^a-z0-9]/i',
),function ($matches) {returnstrtoupper($matches[0]); },$event['event']);
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know there is such magic default logic.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

true, yet convenient enough to keep IMHO, or at least didnt want to spoil this feat. along with a deprecation.

publicfunctiontestInvokableEventListener()
{
$container =newContainerBuilder();
$container->register('foo','stdClass')->addTag('kernel.event_listener',array('event' =>'foo.bar'));
Copy link
Contributor

Choose a reason for hiding this comment

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

\stdClass::class

),function ($matches) {returnstrtoupper($matches[0]); },$event['event']);
$event['method'] =preg_replace('/[^a-z0-9]/i','',$event['method']);

if (null !== ($class =$container->getDefinition($id)->getClass()) && !method_exists($class,$event['method']) &&method_exists($class,'__invoke')) {

Choose a reason for hiding this comment

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

we should use $container->getReflectionClass() instead
this will fix the current logic that misses resource tracking

@nicolas-grekasnicolas-grekas added this to the4.1 milestoneDec 4, 2017
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 29, 2017
edited
Loading

I'm "-0": I don't see the benefit of this. Naming methods helps the reader, and is not a strong requirement.

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedDec 29, 2017
edited
Loading

Naming methods helps the reader

naming classes too :)

this allows me, the class author, to write IMHO pure event listeners and let users configure as usual. That is win-win.

@fabpot
Copy link
Member

Thank you@ro0NL.

@fabpotfabpot merged commitfa5b7eb intosymfony:masterJan 19, 2018
fabpot added a commit that referenced this pull requestJan 19, 2018
This PR was merged into the 4.1-dev branch.Discussion----------[DI] Allow for invokable event listeners| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->Inspired by#24637 /#25259. This adds invokable support for event listeners :)```yamlSome\Foo:    tags: [{ name: kernel.event_listener, event: kernel.request }]``````phpclass Foo {    public function __invoke(GetResponseEvent $event) { }}```Commits-------fa5b7eb [DI] Allow for invokable event listeners
@ro0NLro0NL deleted the eventlistener branchJanuary 28, 2018 14:53
@fabpotfabpot mentioned this pull requestMay 7, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@dunglasdunglasdunglas approved these changes

@xabbuhxabbuhxabbuh approved these changes

@chalasrchalasrchalasr approved these changes

+4 more reviewers

@TobionTobionTobion left review comments

@TaluuTaluuTaluu approved these changes

@SimperfitSimperfitSimperfit approved these changes

@20uf20uf20uf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

11 participants

@ro0NL@Simperfit@nicolas-grekas@fabpot@dunglas@Taluu@Tobion@xabbuh@20uf@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp