Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Console] enable describing commands in ways that make thelist command lazy#39851

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

Merged
nicolas-grekas merged 1 commit intosymfony:5.xfromnicolas-grekas:console-attr
Jan 20, 2021

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedJan 15, 2021•
edited
Loading

QA
Branch?5.x
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#33804
LicenseMIT
Doc PR-

This PR improves the way one can describe a command so that thelist command can be made lazy:

  • when provided using the$defaultName property or theconsole.command tag, the name of a command is now exploded using the| character. The first name in the list defines the name of the command, the other ones its aliases. When the first name is the empty string, the second name is used instead, and the command is declared as hidden.
  • a new$defaultDescription static property and a newdescription tag attribute allow for defining the commands' description while registering them.

Together, this is enough to make thelist command lazy, because this command only accesses each command's name, aliases, hidden-status, and description.

On the implementation side, this PR adds aLazyCommand class that proxies regular commands to make them lazy for the target purpose.

This PR will enable support for attributes for configuring a command name+description+etc.
e.g. using the concepts in#39804:
#[CommandAutoTag(name: 'foo:bar', desc: 'boo', hidden: true)]#

The attribute could very well split thehidden andaliases settings apart - while the underlying code and pre-PHP8 apps would use the compact form, because dealing with many static properties + methods would be a maintenance pain imho.

ro0NL, derrabus, and tkotosz reacted with thumbs up emojichalasr, helhum, and tkotosz reacted with heart emoji
@chalasr
Copy link
Member

Looks really nice, thank you.
I'm going to challenge the new convention for hidden commands as I had to stare at it for a while before getting what it means. I'm not sure that the command visibility should be defined through its name, at least it's not obvious that an empty string before the first separator should be the way to control it.
Could we use a special char in front of the name (. ?) or add another static prop+tag attr?

@nicolas-grekas
Copy link
MemberAuthor

Not hidden:
$defaultName = 'app:foo|foo'

Hidden:
$defaultName = '|app:foo|goo'

I don't think this is worth adding another static property.

@nicolas-grekasnicolas-grekas changed the title[Console] enable describing commands in ways that makes thelist command lazy[Console] enable describing commands in ways that make thelist command lazyJan 15, 2021
@javiereguiluz
Copy link
Member

I don't like much the proposed convention to define a command as hidden because it's not self-explanatory.

As an alternative, would it be possible to store the list of commands in the cache dir while "compiling" the app? Same as we do with routes. Thanks.

dkarlovi, sstok, and Amunak reacted with thumbs up emoji

@nicolas-grekas
Copy link
MemberAuthor

IMHO, there is no need to optimize for setting the "hidden" flag, as that's not a mainstream feature. It's readable enough as is.

would it be possible to store the list of commands in the cache dir while "compiling" the app? Same as we do with routes. Thanks.

Sorry, I don't get what you mean, can you please describe your idea a bit more? What's the DX that goes with this?

@noniagriconomie
Copy link
Contributor

nice to have this lazy feature to be added :)

my 2cts, why not.app:foo|foo for hidden? as a hidden Unix file for example

that way:

  • the| for separating command name first and aliases next
  • the. to hide the command
natewiebe13, alister, 7ochem, tkotosz, and GromNaN reacted with thumbs up emoji

@jderusse
Copy link
Member

Instead of adding static property for everything, what's about making the methodconfigure static?.

class AppCommandextends Commandimplements StaticCommandDescribedInterface{publicstaticfunctionconfigure():CommandDescription  {returnnewCommandDescription('app:test')      ->setDefinition()      ->setHelp()      ->addInput(...)    ;  }}

class that can't be static and need internal service to resolve could implement another interface (that is incompatible with the previous)

class AppCommandextends Commandimplements InstanceCommandDescribedInterface{publicfunctionconfigure():CommandDescription  {returnnewCommandDescription('app:test')      ->setDefinition()      ->setHelp($this->getHint())      ->addInput(...)    ;  }}
javiereguiluz, jkufner, and wouterj reacted with thumbs up emoji

@nicolas-grekas
Copy link
MemberAuthor

@jderusse that would kill the possibility to make the command configuration sensitive to constructor argument. Aka that could kill a lot of flexibility in the class. Yes, we don't use/need that in typical Symfony DI, but the console component is very much used standalone. We should be sure to not degrade it's DX when used outside of a typical Symfony stack.

From a SOLID pov, static methods are just bad.

@nicolas-grekas
Copy link
MemberAuthor

why not.app:foo|foo for hidden

because it's not better than the current proposal?

@jderusse
Copy link
Member

@jderusse that would kill the possibility to make the command configuration sensitive to constructor argument. Aka that could kill a lot of flexibility in the class. Yes, we don't use/need that in typical Symfony DI, but the console component is very much used standalone. We should be sure to not degrade it's DX when used outside of a typical Symfony stack.

From a SOLID pov, static methods are just bad.

Isn't it addressed by the second part of my comment?

@nicolas-grekas
Copy link
MemberAuthor

@jderusse to not break BC, we should rather make static version opt-in. I don't know where this could lead, in terms of BC impact for example. Of course, feel free to give it a try, as I did here with my proposal.

@natewiebe13
Copy link

why not.app:foo|foo for hidden

because it's not better than the current proposal?

I think it's more clear as to what the intention is. Prefixing with a dot is common convention for hiding a file/directory in Linux and Mac OS. Whereas using the separator as a prefix, it isn't clear without first reading documentation that it's hiding the command instead of just a typo.

@stof
Copy link
Member

@natewiebe13 but this reserves 2 chars (. and|) which cannot be used anymore in command names.| is fine as a reserved char is fine, as no one would use that char in their command name:| is a special char in all shells, and so using the command would require escaping it all the time.
But. has no special meaning and could totally be used in command names (even though that's not the recommended Symfony naming convention).

nicolas-grekas and ro0NL reacted with thumbs up emoji

@stof
Copy link
Member

Isn't it addressed by the second part of my comment?

but that forbids lazy-loading these commands then, while the use case for relying in the constructor arguments in the configuration is much more likely to be related to configuring options or arguments than the description.

@natewiebe13
Copy link

natewiebe13 commentedJan 15, 2021•
edited
Loading

@natewiebe13 but this reserves 2 chars (. and|) which cannot be used anymore in command names.| is fine as a reserved char is fine, as no one would use that char in their command name:| is a special char in all shells, and so using the command would require escaping it all the time.
But. has no special meaning and could totally be used in command names (even though that's not the recommended Symfony naming convention).

But wouldn't that just be for the first char in a command name? I wouldn't expect any periods in the command name to cause issues, just if the name wasprefixed by a period.

Edit
For example, I'd still think.app.do-something would still be valid asapp.do-something. It would just be hidden.

@stof
Copy link
Member

@nicolas-grekas there is still the issue ofCommand::isEnabled() though. It impactslist as well.

@nicolas-grekas
Copy link
MemberAuthor

there is still the issue of Command::isEnabled() though. It impacts list as well.

Isn't this covered? See the implementation ofisEnabled(). Otherwise, I may have missed what you meant.

@nicolas-grekas
Copy link
MemberAuthor

I wouldn't expect any periods in the command name to cause issues, just if the name was prefixed by a period.

Until one will report a BC break :) This happens all the time when we do such assumptions.

Honestly, the leading pipe is fine, it's as readable. Everything needs to be taught anyway.

@stof
Copy link
Member

Isn't this covered? See the implementation ofisEnabled(). Otherwise, I may have missed what you meant.

disabled commands are excluded bylist

@ro0NL
Copy link
Contributor

@stof pipe char may be quoted 😅 im fine with it, otherwisestatic $defaultAliases & co

reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull requestFeb 4, 2021
With the Symfony PRsymfony/symfony#39851new depndency injection console.command tag properties aresupported, which allow to inject the description andthe hidden state into console commands.In order to maintain a consistent console.command DI tag syntaxbetween Symfony and TYPO3 core, we now add forward compatibilityfor this change.Releases: master, 10.4Resolves: #93425Related: #93174Change-Id: Ie5dac65deaede2099f2d337466295bd2815ce918Reviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67639Tested-by: TYPO3com <noreply@typo3.com>Tested-by: Helmut Hummel <typo3@helhum.io>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Benjamin Franzke <bfr@qbus.de>Reviewed-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Benjamin Franzke <bfr@qbus.de>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull requestFeb 4, 2021
With the Symfony PRsymfony/symfony#39851new depndency injection console.command tag properties aresupported, which allow to inject the description andthe hidden state into console commands.In order to maintain a consistent console.command DI tag syntaxbetween Symfony and TYPO3 core, we now add forward compatibilityfor this change.Releases: master, 10.4Resolves: #93425Related: #93174Change-Id: Ie5dac65deaede2099f2d337466295bd2815ce918Reviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67639Tested-by: TYPO3com <noreply@typo3.com>Tested-by: Helmut Hummel <typo3@helhum.io>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Benjamin Franzke <bfr@qbus.de>Reviewed-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Benjamin Franzke <bfr@qbus.de>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull requestFeb 4, 2021
With the Symfony PRsymfony/symfony#39851new depndency injection console.command tag properties aresupported, which allow to inject the description andthe hidden state into console commands.In order to maintain a consistent console.command DI tag syntaxbetween Symfony and TYPO3 core, we now add forward compatibilityfor this change.Releases: master, 10.4Resolves: #93425Related: #93174Change-Id: Ie5dac65deaede2099f2d337466295bd2815ce918Reviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67600Tested-by: TYPO3com <noreply@typo3.com>Tested-by: Benjamin Franzke <bfr@qbus.de>Reviewed-by: Benjamin Franzke <bfr@qbus.de>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull requestFeb 4, 2021
With the Symfony PRsymfony/symfony#39851new depndency injection console.command tag properties aresupported, which allow to inject the description andthe hidden state into console commands.In order to maintain a consistent console.command DI tag syntaxbetween Symfony and TYPO3 core, we now add forward compatibilityfor this change.Releases: master, 10.4Resolves: #93425Related: #93174Change-Id: Ie5dac65deaede2099f2d337466295bd2815ce918Reviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67600Tested-by: TYPO3com <noreply@typo3.com>Tested-by: Benjamin Franzke <bfr@qbus.de>Reviewed-by: Benjamin Franzke <bfr@qbus.de>
bnf added a commit to bnf/typo3 that referenced this pull requestFeb 4, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624d
bnf added a commit to bnf/typo3 that referenced this pull requestFeb 4, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624d
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/backend that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/extensionmanager that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/impexp that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/install that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/lowlevel that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/redirects that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/scheduler that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3IncTeam pushed a commit to TYPO3-CMS/workspaces that referenced this pull requestFeb 9, 2021
Based on the configuration syntax of the Symfony Consolefeaturesymfony/symfony#39851…but implemented differently, using a registry patternrather then a lazy-object pattern (like symfony does).Main motiviation for the registry pattern is following:Symfony LazyCommand wrappers add quite some complexity onlyfor the sake of the list command, we already got lazycommands (in terms of execution) as our CommandRegistryimplements the ConfigurationLoaderInterface that has beenintroduced by 2017 to add support for lazy commands.Now, that means we already got a registry for lazy commands,so it is logical to add lazy description handling there as well.We want to assure that the command list will never instantiateany commands. This is in constrast to the Symfony core LazyCommandapproach, where legacy commands, that do not provide a compile timedescription, would still be instantiated during console command list.Also commands that return false in `isEnabled()` are now listed.That means enabled state is only evaluated during runtime.Therefore the special `dumpautoload` command is transformed into alowlevel command in order to be hidden dependending on being runin composer-mode or not.Releases: masterResolves: #93174Change-Id: Ifa68404cc81c64a335be30f2263a7eb17de0624dReviewed-on:https://review.typo3.org/c/Packages/TYPO3.CMS/+/67635Tested-by: TYPO3com <noreply@typo3.com>Tested-by: core-ci <typo3@b13.com>Tested-by: Benni Mack <benni@typo3.org>Tested-by: Helmut Hummel <typo3@helhum.io>Reviewed-by: Benni Mack <benni@typo3.org>Reviewed-by: Helmut Hummel <typo3@helhum.io>
chalasr added a commit that referenced this pull requestFeb 19, 2021
… commands on PHP 8 (nicolas-grekas)This PR was merged into the 5.3-dev branch.Discussion----------[Console] Add `ConsoleCommand` attribute for declaring commands on PHP 8| Q             | A| ------------- | ---| Branch?       | 5.x| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Builds on#39851On PHP8, this PR will allow using an attribute instead of the public static properties for the name and the description.```php#[ConsoleCommand(name: 'app:my-command',description: '🌈',hidden: true,aliases: ['🌈'],)]class MyCommand extends Command{}```Commits-------0cbc9cc [Console] Add `ConsoleCommand` attribute for declaring commands on PHP 8
nicolas-grekas added a commit that referenced this pull requestMar 17, 2021
…nd in console (DemigodCode)This PR was merged into the 5.3-dev branch.Discussion----------[DebugBundle] Remove warning of ServerDumpPlaceholderCommand in console| Q             | A| ------------- | ---| Branch?       | 5.2.5| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#40495| License       | MIT| Doc PR        |In 5.2.5 the console commands are lazy. (#39851)With this change the ServerDumpCommand::$defaultName is used which isn't set in the placeholder command.If no vardump-server in debug.dump_destination is defined, this will lead to a warning and not adding the command to the console list.Commits-------37b2a19 [DebugBundle] Add $defaultName to PlaceholderCommand
@fabpotfabpot mentioned this pull requestApr 18, 2021
tucksaun added a commit to tucksaun/SwarrotBundle that referenced this pull requestMar 2, 2022
This allow Symfony to make the command lazy-loadable and thus avoiding instantiation of every processor just to list applications command.Seesymfony/symfony#39851 for more info
tucksaun added a commit to tucksaun/SwarrotBundle that referenced this pull requestMar 2, 2022
This allow Symfony to make the command lazy-loadable and thus avoiding instantiation of every processor just to list applications command.Seesymfony/symfony#39851 for more info
tucksaun added a commit to tucksaun/SwarrotBundle that referenced this pull requestMar 2, 2022
This allows Symfony to make the command lazy-loadable and thus avoiding instantiation of every processors only to list application's commands.Seesymfony/symfony#39851 for more info
tucksaun added a commit to tucksaun/SwarrotBundle that referenced this pull requestMar 2, 2022
This allows Symfony to make the command lazy-loadable and thus avoiding instantiation of every processors only to list application's commands.Seesymfony/symfony#39851 for more info
saylor-mik87786 added a commit to saylor-mik87786/maker-bundle that referenced this pull requestJun 3, 2025
This PR was merged into the 1.0-dev branch.Discussion----------Make commands compatible with LazyCommandSidekick ofsymfony/symfony#39851When run with Symfony < 5.3, the generated commands are made forward-compatible with 5.3+Commits-------42c199a Make commands compatible with LazyCommand
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@jderussejderussejderusse approved these changes

@wouterjwouterjwouterj approved these changes

@chalasrchalasrchalasr approved these changes

@lyrixxlyrixxAwaiting requested review from lyrixx

@srozesrozeAwaiting requested review from sroze

@xabbuhxabbuhAwaiting requested review from xabbuh

@ycerutoycerutoAwaiting requested review from yceruto

+1 more reviewer

@rhehrhehrheh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

[Console] Distinguish definition of a command from its execution

11 participants

@nicolas-grekas@chalasr@javiereguiluz@noniagriconomie@jderusse@natewiebe13@stof@ro0NL@wouterj@rheh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp