[Console] Allow commands to provide a default name for compile time registration #23887

chalasr · 2017-08-14T15:15:04Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#23796
License	MIT
Doc PR	symfony/symfony-docs#8147

nicolas-grekas

Would it make sense to use psr4+this to register existing commands?

nicolas-grekas · 2017-08-14T15:41:43Z

src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php

-            $commandName = $tags[0]['command'];
+            if (!isset($tags[0]['command'])) {
+                $defaultNameProvider = array($class, 'getDefaultName');
+                $commandName = $defaultNameProvider();


$class::getDefaultName()

chalasr · 2017-08-14T18:28:09Z

@nicolas-grekas That would require to make them autowired, right?

nicolas-grekas · 2017-08-14T18:41:27Z

Correct, bad idea :)

ro0NL · 2017-08-16T09:51:52Z

src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php

@@ -55,7 +56,7 @@ public function process(ContainerBuilder $container)

            $commandId = 'console.command.'.strtolower(str_replace('\\', '_', $class));

-            if (!isset($tags[0]['command'])) {
+            if (!isset($tags[0]['command']) && !$r->implementsInterface(DefaultNameProviderInterface::class)) {


Should we deprecate this behavior?

That would mean deprecating non-lazy command services, deserves a dedicated discussion I think

Hm right. Could application as a service be a next step? So we can simply wire via add method call.

We should stop here IMHO. We don't do changes for the sake of it, so that if we force people to move on, the benefit must be clear.

Sure. Just saying we can then cleanly wire commands, the command loader etc. The benefit would be its more flexible and allows injecting the application elsewhere... that may be useful.

nicolas-grekas · 2017-08-16T16:02:12Z

src/Symfony/Component/Console/Command/DefaultNameProviderInterface.php

+interface DefaultNameProviderInterface
+{
+    /**
+     * @return string The name to which register the implementing Command


The name to use by default for calling the Command

…egistration

nicolas-grekas · 2017-08-21T08:42:31Z

What about making it easier to define the default name of a command, by asking people to just fill in a
protected static $defaultName?

Here is a commit doing this to see what it could look like: 05c922a

Note that this also "fixes" an issue with inheritance, where the return value of getDefaultName() can be provided by the parent class, which usually is not desired. This also means we could just remove the interface btw.

ogizanagi · 2017-08-21T08:53:00Z

Note that this also "fixes" an issue with inheritance, where the return value of getDefaultName() can be provided by the parent class, which usually is not desired. This also means we could just remove the interface btw.

This is enough to convince me 👍

nicolas-grekas · 2017-08-21T11:25:56Z

src/Symfony/Component/Console/Command/Command.php

@@ -29,6 +29,8 @@
 */
 class Command
 {
+    protected static $defaultName;


a docblock here to help people discover how this can be leveraged, and all good on my side

ro0NL · 2017-08-21T16:30:13Z

src/Symfony/Component/Console/DependencyInjection/AddConsoleCommandPass.php

@@ -55,7 +55,7 @@ public function process(ContainerBuilder $container)

            $commandId = 'console.command.'.strtolower(str_replace('\\', '_', $class));

-            if (!isset($tags[0]['command'])) {
+            if (!isset($tags[0]['command']) && !$commandName = $class::getDefaultName()) {


&& null === $commandName = ... ?

so, what should happen when $commandName === '' ? Registering the empty string is a no-go to me. As is LGTM.

an empty string would give the expected unexpected result :) "0" could be a valid name. Anyway... this is nitpicking; fine asis for me 👍

Supporting the 0 command name will be asked by @Tobion anyway 😂, so let's support it now.

ro0NL · 2017-08-21T16:31:18Z

src/Symfony/Component/Console/Command/Command.php

@@ -46,6 +48,17 @@ class Command
    private $helperSet;

    /**
+     * @return string|null The name to use by default for calling the Command or null when no default name is set
+     */
+    public static function getDefaultName()


chalasr · 2017-08-22T12:17:51Z

@nicolas-grekas applied

fabpot · 2017-08-24T21:10:45Z

Thank you @chalasr.

… compile time registration (chalasr, nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [Console] Allow commands to provide a default name for compile time registration | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23796 | License | MIT | Doc PR | symfony/symfony-docs#8147 Commits ------- eda7d42 [Console] Add protected static $defaultName to set the default name of a Command 5d9ae6b [Console] Allow commands to provide a default name for compile time registration

mvrhov · 2017-09-30T08:47:19Z

Just in case I'm missing something.
Defining the name in the variable and having command registered as service shouldn't create command even in 3.4?
I'm asking this because if I put die in a constructor of a command registered as a service then everything dies and this means that the command is not lazily loaded.

chalasr · 2017-10-06T14:55:27Z

@mvrhov Just tried and no issue on my side. Defining this property and registering the command as a console.command tagged service (nothing to configure with the new default config) is enough to make it loaded lazily, meaning that you can execute any other command without loading this one, except for list (or just bin/console) because it requires to load all commands.
Please open an issue with reproducer if you think there is a bug.

mvrhov · 2017-10-06T17:06:52Z

Oh, I understood that it doesn't need to load them anymore no matter what. I've run the app/console directly when I tried that.

stof · 2017-10-17T11:28:59Z

running just app/console actually runs the list command

mvrhov · 2017-10-17T12:00:59Z

@stof: Yeah I know. Sorry for not being clear enough.
So I'm going to be now. Then this works. :)

Due to some logic in Symfony "Command" the default name of extended commands is not used in the extending command. So we need to copy all of them. See doctrine/migrations#839 And symfony/symfony#23887

This PR was merged into the 1.8-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT This commit is about of a little optimisation to avoid initialisation of all command services on each executing `bin/console` command. Since Symfony 3.4 console commands can be lazy thanks to [a static name property](symfony/symfony#23887). This change does not break BC because the BC layer is already [in the base command's constructor](https://github.com/symfony/symfony/pull/23887/files#diff-8e80595663798837db1b033187835535R75). Commits ------- 91c9918 Make console commands lazy

REF: symfony/symfony#23887

chalasr added this to the 3.4 milestone Aug 14, 2017

chalasr requested a review from nicolas-grekas August 14, 2017 15:15

carsonbot added Status: Needs Review Console Feature labels Aug 14, 2017

nicolas-grekas reviewed Aug 14, 2017

View reviewed changes

chalasr force-pushed the console-default-name branch from c7f9137 to ba4be99 Compare August 14, 2017 17:22

nicolas-grekas approved these changes Aug 14, 2017

View reviewed changes

ogizanagi approved these changes Aug 14, 2017

View reviewed changes

chalasr force-pushed the console-default-name branch 2 times, most recently from 2658662 to bd71187 Compare August 16, 2017 08:01

ro0NL reviewed Aug 16, 2017

View reviewed changes

nicolas-grekas reviewed Aug 16, 2017

View reviewed changes

[Console] Allow commands to provide a default name for compile time r…

5d9ae6b

…egistration

chalasr force-pushed the console-default-name branch from bd71187 to 5d9ae6b Compare August 16, 2017 19:45

chalasr force-pushed the console-default-name branch from 106f353 to 7784b1b Compare August 21, 2017 11:22

nicolas-grekas reviewed Aug 21, 2017

View reviewed changes

ro0NL reviewed Aug 21, 2017

View reviewed changes

chalasr force-pushed the console-default-name branch 6 times, most recently from e431d33 to 96411e8 Compare August 22, 2017 11:34

chalasr force-pushed the console-default-name branch from 3a54ff8 to eda7d42 Compare August 24, 2017 14:44

Tobion approved these changes Aug 24, 2017

View reviewed changes

fabpot approved these changes Aug 24, 2017

View reviewed changes

fabpot merged commit eda7d42 into symfony:3.4 Aug 24, 2017

chalasr deleted the console-default-name branch August 24, 2017 21:11

ogizanagi mentioned this pull request Aug 29, 2017

[Form] Add debug:form command #23694

Merged

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

nicolas-grekas mentioned this pull request Nov 28, 2017

[DependencyInjection] Autoconfiguration for more complex cases #25184

Closed

alcaeus mentioned this pull request Dec 1, 2017

Add default command name to DI config doctrine/DoctrineCacheBundle#127

Closed

ogizanagi mentioned this pull request Mar 19, 2018

Lazy-load commands algolia/search-bundle#218

Merged

ro0NL mentioned this pull request Nov 26, 2018

[RFC][Framework-Bundle] Make an Application available as a container service #29325

Closed

This was referenced Jul 15, 2019

Lazy console commands doctrine/migrations#839

Merged

Lazy console commands doctrine/DoctrineMigrationsBundle#275

Merged

mbrodala mentioned this pull request Mar 23, 2020

Doctrine 2.2.x compatibility andreaswolf/typo3-ext-migrations#1

Merged

vyacheslav-startsev mentioned this pull request Jul 8, 2020

Lazy console commands api-platform/core#3635

Merged

ERuban mentioned this pull request Jul 16, 2020

Make console commands lazy Sylius/Sylius#11657

Merged

kefzce mentioned this pull request Sep 21, 2020

Make command lazily loaded getsentry/sentry-symfony#373

Merged

ERuban mentioned this pull request Oct 2, 2020

[EasySsm][EasyTest] Make console commands lazy eonx-com/easy-monorepo#381

Merged

o5 added a commit to slimapi/slimapi that referenced this pull request Feb 12, 2021

Validation: Make console command lazy

448e701

REF: symfony/symfony#23887

TomasVotruba mentioned this pull request Sep 14, 2022

[Symfony53] Add CommandDescriptionToPropertyRector rectorphp/rector-symfony#245

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Allow commands to provide a default name for compile time registration #23887

[Console] Allow commands to provide a default name for compile time registration #23887

chalasr commented Aug 14, 2017

nicolas-grekas left a comment

nicolas-grekas Aug 14, 2017

chalasr Aug 14, 2017

chalasr commented Aug 14, 2017

nicolas-grekas commented Aug 14, 2017

ro0NL Aug 16, 2017

chalasr Aug 16, 2017

ro0NL Aug 16, 2017 •

edited

Loading

nicolas-grekas Aug 16, 2017 •

edited

Loading

ro0NL Aug 16, 2017

nicolas-grekas Aug 16, 2017

chalasr Aug 16, 2017

nicolas-grekas commented Aug 21, 2017 •

edited

Loading

ogizanagi commented Aug 21, 2017

nicolas-grekas Aug 21, 2017

ro0NL Aug 21, 2017

nicolas-grekas Aug 21, 2017

ro0NL Aug 21, 2017

fabpot Aug 23, 2017

ro0NL Aug 21, 2017

chalasr commented Aug 22, 2017

fabpot commented Aug 24, 2017

mvrhov commented Sep 30, 2017

chalasr commented Oct 6, 2017

mvrhov commented Oct 6, 2017

stof commented Oct 17, 2017

mvrhov commented Oct 17, 2017

[Console] Allow commands to provide a default name for compile time registration #23887

[Console] Allow commands to provide a default name for compile time registration #23887

Conversation

chalasr commented Aug 14, 2017

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Aug 14, 2017

nicolas-grekas commented Aug 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Aug 16, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Aug 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Aug 21, 2017 • edited Loading

ogizanagi commented Aug 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Aug 22, 2017

fabpot commented Aug 24, 2017

mvrhov commented Sep 30, 2017

chalasr commented Oct 6, 2017

mvrhov commented Oct 6, 2017

stof commented Oct 17, 2017

mvrhov commented Oct 17, 2017

ro0NL Aug 16, 2017 •

edited

Loading

nicolas-grekas Aug 16, 2017 •

edited

Loading

nicolas-grekas commented Aug 21, 2017 •

edited

Loading