Skip to content
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

Improve backwards compatibility for command events #1403

Merged
merged 2 commits into from Dec 1, 2020

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Nov 29, 2020

Pull Request

Problem

The custom command event (e.g. 'command:example') was being emit-ed from the wrong object to be useful and for backwards compatibility with Commander v4 and earlier.

See: #1402

Solution

The command events used to be how Commander triggered the action handler, and custom listeners were somewhat of a free bonus. Now the parsing is rather different and it requires extra code to approximate the old behaviour.

There have never been explicit tests for the events. I have added some tests so at least the behaviour is more stable going forward.

(From code inspection, there may be some minor differences in edge cases from historical behaviour which I won't be attempting to match, like what event name is generated for aliases).

ChangeLog

@shadowspawn shadowspawn marked this pull request as ready for review November 29, 2020 03:17
Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

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

LGTM

@shadowspawn
Copy link
Collaborator Author

Thanks @abetomo 👓

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Dec 1, 2020
@shadowspawn shadowspawn added this to the v7.0.0 milestone Dec 1, 2020
@shadowspawn shadowspawn merged commit bd538aa into tj:release/7.x Dec 1, 2020
@shadowspawn shadowspawn deleted the feature/fix-emit branch December 1, 2020 04:34
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants