protected static $defaultDescription = 'Say hi'; seems tempting :}

protected static $defaultDescription = 'Say hi'; seems tempting :}

that'd work only for one item in the list of things that need to be configurable...

well, do we need anything else? Discouraging constructor injection for this feels like the wrong trade off.

There is no tradeoff here. Constructor injection is not the only way to inject deps. ServiceSubscriberInterface is a perfectly valid way, and that's the main one when laziness is desired, which is exactly what is needed here.

The main design flaw of the component is that we have to instantiate the command to get its definition. And I feel like we're wrapping the the command into yet another layer of workarounds here. 😕

There are only two alternatives here:

1. make the configuration of the command static. In terms of design, this damages extensibility.
2. split configuration and implementation into two separate classes. What a DX overhead!

The current design is not flawed to me. It provides just the right amount of DX + maintainability.

Providing laziness is the very purpose of ServiceSubscriberInterface, let's use it, and let's make it easy to use. That my proposal here.

If this is accepted, I think make:command should generate code that uses this trait by default btw /cc @weaverryan

regardless of LazyCommandTrait as a feature (works for me), should we split name+description from the definition? in favor of static props

currently extensibility is already broken given setName doesnt overrule $defaultName

aliases are shown in list but dont work either

it's a WTF actually :P

currently extensibility is already broken given setName doesnt overrule $defaultName

What do you mean? I just had a look at the code and the name looks extensible to me.

There are only two alternatives here:

1. make the configuration of the command static. In terms of design, this damages extensibility.

2. split configuration and implementation into two separate classes. What a DX overhead!

3. Implement a configurable routing for commands.

should we split name+description from the definition? in favor of static props

That wasn't obvious to me, but now I think that what you mean is that the list command needs only description+name, and that we would be OK with instantiating the command to get its arguments, isn't it?

Could be...

I like the current proposal because it gives a simple way to wire commands (no need for the constructor boilerplate, which provides nothing useful).

3. Implement a configurable routing for commands.

Can you elaborate? I don't understand what you mean here...

@nicolas-grekas given sf/console@v5.2.1 i have

class FooBarCommand extends Command
{
//    protected static $defaultName = 'foo:bar';

    protected function configure()
    {
        $this
            ->setName('foo:abc')
            ->setAliases(['foo:def'])
        ;
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        return Command::SUCCESS;
    }
}

list shows

 foo
  foo:abc                     [foo:def] Add a short description for your command

both foo:abc and foo:def work at this point

next uncomment protected static $defaultName = 'foo:bar';

now list shows

 foo
  foo:bar                     [foo:def] Add a short description for your command

but only foo:bar works, whereas i expected the previous output as well.

If you remove setName, but keep $defaultName the alias foo:def still doesnt work.

Hence i think if we extract name+description+aliases (and deprecate it from the definition) we're good to go

Can you elaborate? I don't understand what you mean here...

If we look at how an HTTP request is matched to a controller, we find a router in between that knows everything we need to know to decide which controller exactly we need to instantiate (or pull from the container). The controller instance does not know which paths it is attached to. It can be a dump callable if we want.

I think, we can build something similar to match a CLI call to a command. The command itself is only pulled from the container if we really intend to execute it.

but only foo:bar works, whereas i expected the previous output as well.

Oh, OK. That doesn't mean that extensibility is broken. It means it doesn't work the way you thought it works... You can change the name e.g by using a DI tag attribute. The reason is of course that we built for laziness when reading names.

Hence i think if we extract name+description+aliases (and deprecate it from the definition) we're good to go

That could work. We could even extend the format of $defaultName to allow listing aliases in the same string, eg $defaultName = 'app:foo|bar' (this works with the constraints that DI tag attributes should be strings, unless #38540 is accepted)

For the description, I guess we should do the same: a DI attribute + a default static prop. Up for a PR?

i like the name|alias|alias syntax :)

we have also setHidden, it's weird this does work with $defaultName set (which implies we can read name+description as well 🤔)

That doesn't mean that extensibility is broken. It means it doesn't work the way you thought it works...

does it mean setName shouldnt be allowed if $defaultName is set?

I also like the command_routing.yaml proposal, to define everything in one go. But it separates some concerns of which im not sure it scales well. Eg. for CLI, vendors are allowed to mount commands currently.

If we look at how an HTTP request is matched to a controller, we find a router in between that knows everything we need to know to decide which controller exactly we need to instantiate (or pull from the container). The controller instance does not know which paths it is attached to. It can be a dump callable if we want.

On the other hand, a command must declare it's arguments, because that's its signature. I think having this signature declared in the same place as the implementation is critical. A design based around a command router would be radically different. Enough to warrant experimenting the idea in a separate package, if someone is interested in it.

does it mean setName shouldnt be allowed if $defaultName is set?

nope, as that would damage reusability (setName() is just ignored when using the command with the DI container, but otherwise it's honored)

fair enough

we have also setHidden, it's weird this does work with $defaultName set (which implies we can read name+description as well thinking)

i figured list isnt currently lazy at all, as it iterates each command 😅 my bad.

at this point im indeed wondering if LazyCommandTrait would be sufficient.

we have also setHidden

$defaultName = '|app:foo';

i think that + $defaultDescription will do for a lazy list command 👍

in the long run i think we should prefer $app->addNamedCommand($vendorCommand, 'my:name') rather than mutating

signature (opt+args) are bound to implementation, so making it immutable as of then only seems reasonable.

and if list is lazy out-of-the box im not sure LazyCommandTrait serves a purpose then.

It still unpacks all deps at once, whereas using ServiceSubscriberInterface directly allows invoking them on demand.

I'd prefer not having to teach such an alternative execute() signature to enable full laziness. Good old constructors should still be the preferred way to inject class dependencies imho :).
Since we already have a static prop for the name, I'd be fine adding the equivalent for description and hidden

Since we already have a static prop for the name, I'd be fine adding the equivalent for description and hidden

So am I. Thanks for the discussion.

PR welcome if anyone is up to giving this a try!

See #39851 for a follow-up PR.