[Console] Better support for one command app #16906

lyrixx · 2015-12-08T12:41:18Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#9564
License	MIT

Hello;

I write many CLI application, and "single command" in cli application is not so easy to write.
This is why I propose this patch. IMHO, this PR could replaces #9609.

See it in application:

#!/usr/bin/env php
<?php

require __DIR__.'/vendor/autoload.php';


use Symfony\Component\Console\Application;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

(new Application('echo', '1.0.0'))
    ->register('echo')
        ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo')
        ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar')
        ->setCode(function(InputInterface $input, OutputInterface $output) {
            $output->writeln('start');
            $output->writeln($input->getArgument('foo'));
            $output->writeln($input->getOption('bar'));
        })
    ->getApplication()
    ->setDefaultCommand('echo', true)
    ->run();

Some usage:

>(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php 
start
foo
bar

>(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php  "first argument"
start
first argument
bar

>(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php  "first argument" --bar="first option"
start
first argument
first option

>(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php  "first argument" --bar="first option" --help
Usage:
  echo [options] [--] [<foo>]

Arguments:
  foo                   The directory [default: "foo"]

Options:
      --bar=BAR         Foobar [default: "bar"]
  -h, --help            Display this help message
  -q, --quiet           Do not output any message
  -V, --version         Display this application version
      --ansi            Force ANSI output
      --no-ansi         Disable ANSI output
  -n, --no-interaction  Do not ask any interactive question
  -v|vv|vvv, --verbose  Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

jakzal · 2015-12-10T17:07:18Z

"Unique command" doesn't imply that there's only one command. How about calling it a "single command"?

lyrixx · 2015-12-10T17:16:22Z

Indeed ;) I will update my PR.

stof · 2015-12-10T17:16:44Z

src/Symfony/Component/Console/Application.php

+
+    public function setUniqueCommand($commandName)
+    {
+        if (null !== $this->uniqueCommand) {


should we also validate that it is registered ?

What do you mean?

Triggering an exception if you configure setUniqueCommand('foobar') and there is no foobar command registered

Why not, but if we do that, we are forcing the order. I mean will not able anymore to first set-up the single command name, then register the command.

lyrixx · 2015-12-11T15:06:16Z

PR updated ;)

javiereguiluz · 2015-12-11T15:13:42Z

I like this proposal. However, I don't like these two lines from the above example:

    ->getApplication()
    ->setSingleCommand('echo')

Would this other code work?

(new Application())
    ->register('echo')
        ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo')
        ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar')
        ->setCode(function(InputInterface $input, OutputInterface $output) {
            // ...
        })
    ->run();

lyrixx · 2015-12-11T15:32:30Z

@javiereguiluz I think we can implement that, but it's a bit too magic for me. And we don't want magic in symfony.

lyrixx · 2015-12-11T15:40:26Z

@javiereguiluz

What about:

(new Application('echo', '1.0.0'))
    ->registerSingleCommand('echo') // <--------------------- The change is HERE
        ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo')
        ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar')
        ->setCode(function(InputInterface $input, OutputInterface $output) {
            $output->writeln('start');
            $output->writeln($input->getArgument('foo'));
            $output->writeln($input->getOption('bar'));
        })
    ->run();

nicolas-grekas · 2015-12-11T15:42:39Z

I like it :)

javiereguiluz · 2015-12-11T15:43:20Z

I'm afraid that the new example doesn't follow the Symfony philosophy either. It complicates the learning curve because it adds a new method (registerSingleCommand()) and it doesn't follow Fabien's idea of priming composition (generic methods that can be combined, instead of specific and "niche" methods).

lyrixx · 2015-12-11T15:45:48Z

Then setSingleCommand is the best choice for now.

javiereguiluz · 2015-12-11T15:54:48Z

Yes. Would this work?

(new Application())
    ->register('echo')
        ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo')
        ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar')
        ->setCode(function(InputInterface $input, OutputInterface $output) {
            // ...
        })
    ->run('echo');  // <-- pass the command to run

lyrixx · 2015-12-11T16:23:49Z

This is a good idea, but a BC break (ref)

nicolas-grekas · 2015-12-11T17:13:25Z

->runSingleCommand('echo')?

jvasseur · 2015-12-11T17:27:01Z

Why not

(new SingleCommandApplication())
    ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo')
    ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar')
    ->setCode(function(InputInterface $input, OutputInterface $output) {
            // ...
    })
    ->run();

?

stof · 2015-12-11T17:28:31Z

@nicolas-grekas this might complicate the Application a lot though, as run() is only indirectly responsible for resolving the command name, so we would have to do weird things.

@jvasseur this is a bad idea, as your SingleCommandApplication does not have the API of the Application anymore, but of the Command

jakzal · 2015-12-11T17:28:54Z

@jvasseur I personally like your idea the most.

@lyrixx is it possible to achieve this with a small amount of code? :)

lyrixx · 2015-12-11T20:27:35Z

Yes, but whit black magic :-)

nicolas-grekas · 2016-03-02T15:52:34Z

src/Symfony/Component/Console/Application.php

+    public function setSingleCommand($commandName)
+    {
+        if (null !== $this->singleCommand) {
+            throw new \LogicException('A Single command is already defined.');


I don't think this check is needed. It looks like a useless restriction to me.

I thinks it's useful restriction, yet it makes this feature less flexible.
And on framework level, I think it should be that way.

Strict code should be in app/domain level

lyrixx · 2016-03-04T13:02:55Z

src/Symfony/Component/Console/Tests/Fixtures/application_run2.txt

@@ -1,28 +1,26 @@
 Usage:
-  help [options] [--] [<command_name>]
+  list [options] [--] [<namespace>]


I updated the fixture, because now, the --help display the help of the default command.

fabpot · 2016-06-15T08:47:24Z

src/Symfony/Component/Console/Application.php

@@ -1044,6 +1052,18 @@ private function findAlternatives($name, $collection)
    public function setDefaultCommand($commandName)


What about adding a new argument here that says it is the default but also the only command?

And so replace the method I added? It sounds good to me.

Done. I also added a test.

fabpot · 2016-06-15T10:17:10Z

src/Symfony/Component/Console/Application.php

@@ -1039,11 +1047,21 @@ private function findAlternatives($name, $collection)
    /**
     * Sets the default Command name.
     *
-     * @param string $commandName The Command name
+     * @param string $commandName   The Command name
+     * @param bool   $singleCommand Set to true if there is only one command is this Application


typo: in this

fabpot · 2016-06-15T10:22:43Z

👍

javiereguiluz · 2016-06-15T10:34:33Z

src/Symfony/Component/Console/Application.php

     */
-    public function setDefaultCommand($commandName)
+    public function setDefaultCommand($commandName, $singleCommand = false)


Should this boolean argument be called $isSingleCommand instead?

I don't know.... Just tell me ;)

@javiereguiluz : so ?

In my opinion it should be called $isSingleCommand because $singleCommand looks like it's asking for the name of that single command.

fabpot · 2016-06-15T16:18:17Z

Thank you @lyrixx.

This PR was merged into the 3.2-dev branch. Discussion ---------- [Console] Better support for one command app | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9564 | License | MIT Hello; I write many CLI application, and "single command" in cli application is not so easy to write. This is why I propose this patch. IMHO, this PR could replaces #9609. See it in application: ```php #!/usr/bin/env php <?php require __DIR__.'/vendor/autoload.php'; use Symfony\Component\Console\Application; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; (new Application('echo', '1.0.0')) ->register('echo') ->addArgument('foo', InputArgument::OPTIONAL, 'The directory', 'foo') ->addOption('bar', null, InputOption::VALUE_REQUIRED, 'Foobar', 'bar') ->setCode(function(InputInterface $input, OutputInterface $output) { $output->writeln('start'); $output->writeln($input->getArgument('foo')); $output->writeln($input->getOption('bar')); }) ->getApplication() ->setSingleCommand('echo') ->run(); ``` Some usage: ``` >(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php start foo bar ``` ``` >(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php "first argument" start first argument bar ``` ``` >(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php "first argument" --bar="first option" start first argument first option ``` ``` >(3)[{..}eg/dev/github.com/symfony/symfony](console-one-app) php test.php "first argument" --bar="first option" --help Usage: echo [options] [--] [<foo>] Arguments: foo The directory [default: "foo"] Options: --bar=BAR Foobar [default: "bar"] -h, --help Display this help message -q, --quiet Do not output any message -V, --version Display this application version --ansi Force ANSI output --no-ansi Disable ANSI output -n, --no-interaction Do not ask any interactive question -v|vv|vvv, --verbose Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug ``` Commits ------- 4a9bb1d [Console] Better support for one command app

chalasr · 2016-07-07T23:38:01Z

Does the documentation have been updated, or it is planned to? Otherwise it should. 👍

This PR was merged into the master branch. Discussion ---------- Updated single command How to Hi, I've updated "Single command application" tutorial, regarding the recent pull request merged (symfony/symfony#16906). Mickaël Commits ------- 5535c88 Updated single command How to

carsonbot added the Status: Needs Review label Dec 8, 2015

lyrixx force-pushed the console-one-app branch from 06485a9 to acc2ab5 Compare December 8, 2015 12:41

jakzal added the Console label Dec 10, 2015

stof reviewed Dec 10, 2015
View reviewed changes

lyrixx force-pushed the console-one-app branch from acc2ab5 to b342502 Compare December 11, 2015 15:01

lyrixx force-pushed the console-one-app branch from b342502 to 42a9111 Compare January 14, 2016 13:11

nicolas-grekas reviewed Mar 2, 2016
View reviewed changes

fabpot mentioned this pull request Mar 2, 2016

[WIP] [Console] Make creating single command app easier #9609

Closed

3 tasks

javiereguiluz changed the title ~~[Command] Better support for one command app~~ [Console] Better support for one command app Mar 3, 2016

javiereguiluz added the Feature label Mar 3, 2016

lyrixx force-pushed the console-one-app branch from 42a9111 to e013d3d Compare March 4, 2016 12:52

lyrixx reviewed Mar 4, 2016
View reviewed changes

lyrixx force-pushed the console-one-app branch from e013d3d to 07549ad Compare March 4, 2016 13:12

lyrixx force-pushed the console-one-app branch from 07549ad to 3fa83cd Compare April 4, 2016 08:46

javiereguiluz mentioned this pull request May 25, 2016

[Console Component] Make creating single command app easier #9564

Closed

fabpot reviewed Jun 15, 2016
View reviewed changes

lyrixx force-pushed the console-one-app branch from 3fa83cd to 99676f2 Compare June 15, 2016 09:05

fabpot reviewed Jun 15, 2016
View reviewed changes

javiereguiluz reviewed Jun 15, 2016
View reviewed changes

lyrixx force-pushed the console-one-app branch 2 times, most recently from a7d0fa2 to 02914e6 Compare June 15, 2016 16:05

[Console] Better support for one command app

4a9bb1d

lyrixx force-pushed the console-one-app branch from 02914e6 to 4a9bb1d Compare June 15, 2016 16:15

fabpot merged commit 4a9bb1d into symfony:master Jun 15, 2016

lyrixx deleted the console-one-app branch June 15, 2016 16:21

chalasr mentioned this pull request Jul 8, 2016

Update the doc for one command applications symfony/symfony-docs#6729

Closed

mickaelandrieu mentioned this pull request Aug 15, 2016

Updated single command How to symfony/symfony-docs#6876

Merged

ro0NL mentioned this pull request Sep 29, 2016

[2.7][Console] consistent application help with 3.2 #20090

Closed

fabpot mentioned this pull request Oct 27, 2016

Release v3.2.0-BETA1 #20317

Merged

		@@ -1044,6 +1052,18 @@ private function findAlternatives($name, $collection)
		public function setDefaultCommand($commandName)

[Console] Better support for one command app #16906

[Console] Better support for one command app #16906

Conversation

lyrixx commented Dec 8, 2015 • edited Loading

jakzal commented Dec 10, 2015

lyrixx commented Dec 10, 2015

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

lyrixx commented Dec 11, 2015

javiereguiluz commented Dec 11, 2015

lyrixx commented Dec 11, 2015

lyrixx commented Dec 11, 2015

nicolas-grekas commented Dec 11, 2015

javiereguiluz commented Dec 11, 2015

lyrixx commented Dec 11, 2015

javiereguiluz commented Dec 11, 2015

lyrixx commented Dec 11, 2015

nicolas-grekas commented Dec 11, 2015

jvasseur commented Dec 11, 2015

stof commented Dec 11, 2015

jakzal commented Dec 11, 2015

lyrixx commented Dec 11, 2015

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

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

fabpot commented Jun 15, 2016

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

fabpot commented Jun 15, 2016

chalasr commented Jul 7, 2016

lyrixx commented Dec 8, 2015 •

edited

Loading