Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] feat: Add ability to set directory with console commands #51884

Closed
wants to merge 1 commit into from

Conversation

freebuu
Copy link

@freebuu freebuu commented Jun 22, 2024

Hello!

I want to add the ability to specify the path to the directory for storing console commands when configuring the application.

Now in the withCommands() method the path is hardcoded as Console/Commands and it cannot be changed, and if you store commands in another directory, they must be registered manually.

In my opinion, after removing the \Console\Kernel, a more suitable place is Commands.

Application::configure(basePath: dirname(__DIR__))
  ->withCommands([
    \Commands\SomeCommand::class,
    \Commands\AnotherDir\AnotherCommand::class,
  ])
  ->create();

My propporsal:

Application::configure(basePath: dirname(__DIR__))
  ->withCommands('Commands')
  ->create();

PS: I haven't found tests for the withCommands() method and I'm not sure if I need to write them and if so, where to do it.

@freebuu freebuu force-pushed the commands-dir-configuration branch from 2886db8 to dd6dc1d Compare June 22, 2024 15:02
@ahinkle
Copy link
Contributor

ahinkle commented Jun 22, 2024

Can't you just write a callback that resolves the locations?

@freebuu
Copy link
Author

freebuu commented Jun 22, 2024

@ahinkle This is exactly how I solved this problem for now, but I thought that the ability to specify a Command directory when configuring the application would be useful not only for me.

@taylorotwell
Copy link
Member

You can just include paths in the array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants