8000 [make:listener] Match event name against active events class/id by maelanleborgne · Pull Request #1579 · symfony/maker-bundle · GitHub
[go: up one dir, main page]

Skip to content

[make:listener] Match event name against active events class/id #1579

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 2 commits into from
Aug 29, 2024

Conversation

maelanleborgne
Copy link
Contributor

Closes #1410

Description

The goal of this PR is to simplify the usage of the make:listener command by adding the following features to the event input :

  • Matching of the event name to existing events FQCN.
  • Typo detection (using levenshtein distance).

Example

Matching on class short name :

image

Matching on class short name when multiple events match :

image

Typo detection :

image

@94noni
Copy link
Contributor
94noni commented Jul 30, 2024

I like the idea and will indeed simplify such maker, and behavior can be duplicated on others as well I think
but have no opinion on the implementation

Copy link
Collaborator
@jrushlow jrushlow left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @maelanleborgne - a couple of minor comments below...

Comment on lines 112 to 113
$suggestionList = [];
foreach ($eventIdAndFQCNList as $eventSuggestion) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add spacing for readability between logic statements (e.g. if's) to be consistent with other code in the bundle.

Suggested change
$suggestionList = [];
foreach ($eventIdAndFQCNList as $eventSuggestion) {
$suggestionList = [];
foreach ($eventIdAndFQCNList as $eventSuggestion) {

We need to do this for the other if statements below as well.

if (1 === \count($suggestionList)) {
$question = new ConfirmationQuestion(sprintf('<fg=green>Did you mean</> <fg=yellow>"%s"</> <fg=green>?</>', $suggestionList[0]), false);
$event = $io->askQuestion($question) ? $suggestionList[0] : $event;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can eliminate this else condition by returning early inside the if conditional. Then just have the statements within the else called after the if.

@jrushlow jrushlow added Feature New Feature Status: Needs Work Additional work is needed labels Jul 31, 2024
@jrushlow jrushlow added this to the v1.61.0 milestone Jul 31, 2024
@maelanleborgne maelanleborgne force-pushed the feature/listener-class-match branch from 8fe0c1e to 86f6dd6 Compare August 12, 2024 07:43
Copy link
Collaborator
@jrushlow jrushlow left a comment
8000

Choose a reason for hiding this comment

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

Thank you @maelanleborgne

@jrushlow jrushlow added Status: Reviewed Has been reviewed by a maintainer Related Tests Pass and removed Status: Needs Work Additional work is needed labels Aug 29, 2024
@jrushlow jrushlow merged commit ba37c4f into symfony:main Aug 29, 2024
8 checks passed
@jrushlow jrushlow mentioned this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Related Tests Pass Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Make:Listener] Improve event name matching
3 participants
0