Enable using commands containing underscores #1365
Conversation
There was a problem hiding this comment.
I added some comments with a few thoughts.
If @noplanman agrees, those should be changed before merging.
In regards of the BC-breaking change, I think that should be ok.
We could eventually write something into the log if there is a case, that has changed bevavior, so the user has a clue, if something is wrong all of the sudden.
I think that would be pretty smart. Log output should be removed in a later version though.
| protected function classNameToCommandName(string $class): string | ||
| { | ||
| if (!preg_match('/^(.+)Command$/', $class, $matches)) { | ||
| return ''; |
There was a problem hiding this comment.
Returning an empty string if no command is found, is not good style. Should return null instead.
There was a problem hiding this comment.
Returning an empty string if no command is found, is not good style.
I usually agree, except the cases when an empty string is not a valid value (like this case).
To check if the return value is valid we'd need to check if it's not null OR is an empty string: by avoiding null we simply have to check for empty strings.
|
Thanks @mlocati 🎉 😃 |
Summary
As explained in #1364, it's currently impossible to define commands containing underscores.
This PR enables it.
For example:
FoobarCommandclass defines the/foobarcommandFooBarCommandclass defines the/foo_barcommand (this is a BC break)PS: closes #1364