NotificationsYou must be signed in to change notification settings
Fork9.6k
Star30.4k

[Console] Simplify using invokable commands when the component is used standalone#60394

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

Jump to bottom

Open

HypeMC wants to merge1 commit intosymfony:7.4

base:7.4

Choose a base branch

fromHypeMC:simplify-invokable-commands-when-standalone

Open

[Console] Simplify using invokable commands when the component is used standalone#60394

HypeMC wants to merge1 commit intosymfony:7.4fromHypeMC:simplify-invokable-commands-when-standalone

Conversation

Copy link

Member

HypeMC commentedMay 9, 2025

Q	A
Branch?	7.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	-
License	MIT

Inspired by#60389 (comment) .

This PR enables using invokable command when the component is used standalone:

#[AsCommand(name:'app:example')]class ExampleCommandimplements SignalableCommandInterface{publicfunction__invoke(InputInterface$input,OutputInterface$output):int    {// ...return Command::SUCCESS;    }publicfunctiongetSubscribedSignals():array    {return [\SIGINT, \SIGTERM];    }publicfunctionhandleSignal(int$signal,int|false$previousExitCode =0):int|false    {// handle signalreturn0;    }}$application =newApplication();$application->addInvokable(newExampleCommand());$application->run();

HypeMC requested a review fromchalasr as acode owner

May 9, 2025 21:07

carsonbot added Status: Needs Review Console Feature labels

May 9, 2025

carsonbot added this to the7.3 milestone

May 9, 2025

Copy link

Contributor

OskarStark commentedMay 10, 2025

So we really need the extra method addInvokable() or could we handle this automatically?

Copy link

Member

xabbuh commentedMay 10, 2025

We cannot widen the argument type inadd() as that would be a BC break for child classes overriding the method.

nicolas-grekas reviewed

May 10, 2025

View reviewed changes

src/Symfony/Component/Console/Application.php OutdatedShow resolvedHide resolved

nicolas-grekas reviewed

May 10, 2025

View reviewed changes

src/Symfony/Component/Console/Tests/ApplicationTest.phpShow resolvedHide resolved

HypeMC force-pushed thesimplify-invokable-commands-when-standalone branch frome07ce18 to5cf2a0dCompare

May 10, 2025 08:33

Copy link

Contributor

OskarStark commentedMay 10, 2025

We could remove it and handle it internally, couldn't we?

Do we need to adjust addCommands() method as well?

Copy link

Member

chalasr commentedMay 11, 2025

Cool, thanks for working on this.

We could remove it and handle it internally, couldn't we?

@OskarStark If you mean removing the type declaration then it's a BC break too unfortunately, seehttps://3v4l.org/5dhmc.

I'd prefer the invokable concept not to leak into public API though. What about addingApplication::addCommand(callable|Command $command): Command instead? It would be great if that new method could provide a way to make the command lazy also.

HypeMC force-pushed thesimplify-invokable-commands-when-standalone branch from5cf2a0d to758b85eCompare

May 13, 2025 22:48

[Console] Simplify using invokable commands when the component is use…

2a19b9b

…d standalone

HypeMC force-pushed thesimplify-invokable-commands-when-standalone branch from758b85e to2a19b9bCompare

May 13, 2025 23:18

Copy link

MemberAuthor

HypeMC commentedMay 13, 2025

I'd prefer the invokable concept not to leak into public API though. What about addingApplication::addCommand(callable|Command $command): Command instead? It would be great if that new method could provide a way to make the command lazy also.

@chalasr I've made the changes you requested, please let me know what you think.