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

[EventDispatcher] Try first if the event is Stopped#18426

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
nicolas-grekas merged 1 commit intosymfony:2.3fromlyrixx:event-stopped
Apr 4, 2016

Conversation

@lyrixx
Copy link
Member

QA
Branch?2.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

If you trigger 2 events with the same instance of Event and if a listener in the first dispatch stop the propagation, then the very first listener of the second dispatch is called. It should not IMHO.

@unkind
Copy link
Contributor

Does it make sense to dispatch stopped event? Raising exception here seems more appropriate.

@lyrixxlyrixx closed thisApr 3, 2016
@lyrixxlyrixx deleted the event-stopped branchApril 3, 2016 22:16
@lyrixxlyrixx restored the event-stopped branchApril 3, 2016 22:16
@lyrixxlyrixx reopened thisApr 3, 2016
@lyrixx
Copy link
MemberAuthor

Does it make sense to dispatch stopped event? Raising exception here seems more appropriate.

I don't think so, because it will make userland code harder to write.

@stof
Copy link
Member

stof commentedApr 4, 2016

@lyrixx I think such case should add a log message in the traceable event dispatcher, as it could also be a mistake in the userland code (people not knowing that stopping the the propagation of the first event will impact the next event because they reused the same event object)

@lyrixx
Copy link
MemberAuthor

@stof Good idea. I added it.

@nicolas-grekas
Copy link
Member

👍

@unkind
Copy link
Contributor

I don't think so, because it will make userland code harder to write.

How does error reporting make harder to write?

@javiereguiluz
Copy link
Member

👍

@lyrixx
Copy link
MemberAuthor

How does error reporting make harder to write?

there is not error IMHO. see this for instance:https://github.com/symfony/symfony/pull/11882/files#diff-6d1e8dc0d2c0e7c4ab206d0efbb71516R161

@nicolas-grekas
Copy link
Member

Thank you@lyrixx.

@nicolas-grekasnicolas-grekas merged commita30e166 intosymfony:2.3Apr 4, 2016
nicolas-grekas added a commit that referenced this pull requestApr 4, 2016
This PR was merged into the 2.3 branch.Discussion----------[EventDispatcher] Try first if the event is Stopped| Q             | A| ------------- | ---| Branch?       | 2.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -If you trigger 2 events with the same instance of Event and if a listener in the first dispatch stop the propagation, then the very first listener of the second dispatch is called. It should not IMHO.Commits-------a30e166 [EventDispatcher] Try first if the event is Stopped
This was referencedApr 29, 2016
@lyrixxlyrixx deleted the event-stopped branchNovember 10, 2016 09:22
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lyrixx@unkind@stof@nicolas-grekas@javiereguiluz@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp