[WIP] [Console] Make creating single command app easier #9609
Conversation
|
you shoukd include the pr template in your pr description |
| * Note that is is still possible add additional commands | ||
| * to the application. These commands will not | ||
| * be directly accessible from the command line, but are | ||
| * still callable internally. |
There was a problem hiding this comment.
I don't think this is a good idea. Calling a command from another one is not the clean way to reuse the logic (especially if the seond command is not meant to be public, as it makes no sense to have it as a command).
If you need to share logic between commands, the clean way is to extract this logic in a service used by both commands
There was a problem hiding this comment.
Ok, fair enough. I've never used calling a command from a command myself, but though some people might want to use it. But there are indeed better ways of reusing code. I will remove the comment and override add() to throw an exception.
|
Is there any more work I have to do on this pull request? I see the Travis CI build failed, but that's on an unrelated component (Stopwatch), so not sure what is wrong here. |
| @@ -58,9 +58,15 @@ public function __construct(Application $application) | |||
| * | |||
| * @return integer The command exit code | |||
| */ | |||
| public function run(array $input, $options = array()) | |||
| public function run($input, $options = array()) | |||
There was a problem hiding this comment.
Why did you change the argument type?
There was a problem hiding this comment.
Because In SingleCommandApplicationTest::testRun() I use it, providing it with an ArgvInput instance instead of an array.
Otherwise I can not properly test the real command line interface handling.
Allowing $input to be an InputInterface makes ApplicationTester::run() more flexible and useful as test helper imho.
There was a problem hiding this comment.
I fail to see a difference in using ArrayInput vs ArgvInput in the application tester. What am I missing? It's not a job of the application tester to test cli handling.
There was a problem hiding this comment.
I fail to see a difference in using ArrayInput vs ArgvInput in the application tester
That's a bit my point: ApplicatationTester can be made agnostic about this and just accept an InputInterface object, instead of a less flexible array.
It's not a job of the application tester to test cli handling.
True, but for SingleCommandApplicationTest it doesn't hurt that there is additional testing of the cli handling, which is slightly different than "traditional" applications (no first "command" argument).
Anyway, I tried to change my patch to use ArrayInput instead of ArgvInput, but ended up finding out that ArrayInput does not properly support input arguments of type IS_ARRAY, around which a large part of the SingleCommandApplication tests are written. So, I'm not sure if using ArrayInput instead of the current ArgvInput is the best option at the moment.
There was a problem hiding this comment.
I think for an ApplicationTester this change is ok, if not then i don't see a problem splitting and having a SingleApplicationTester if it makes it simpler to test, no? although this is a small change for creating a new class. Could you not rather adapt your tests to comply with the ArgvInput?
There was a problem hiding this comment.
Unfortunately, removing the type hint would be a BC break.
There was a problem hiding this comment.
Ok, I'll drop usage of ApplicationTester and roll my own alternatuve (apparently just takes about 6 LOC for the functionality I need)
|
FWIW I've done this before by using // always run the foo command
array_splice($_SERVER['argv'], 1, 0, 'foo');
$app = new Application();
$app->add(new FooCommand());
$app->run();Passing in the |
| // Get generated output (and normalize newlines) | ||
| rewind($stream); | ||
| $display = stream_get_contents($stream); | ||
| $display = str_replace(PHP_EOL, "\n", $display); |
There was a problem hiding this comment.
FYI: I dropped usage of Symfony\Component\Console\Tester\ApplicationTester and the removal of the array type hint in ApplicationTester::run(array $input, $options)
(which apparently broke BC) and replaced it with the low fat roll-your-own alternative above
Added SingleCommandApplication to simplify creating and using Applications that only provide one Command.
|
ping? |
|
What is the status of this? Are we going to see the functionality as part of Symfony/Console? If not any time soon, is there any chance of making this a package in its own right so that I can just composer require it? |
|
Is this still relevant? @soxofaan How do you approach this now? @rquadling I think you can make own package a put a link here. |
|
sorry, I lost interest |
|
In wasp numbers of issues can be difficult to solve easily. So I guess you can close this. |
|
@soxofaan Well what particular shade of green SHOULD it be? |
|
It would be nice to have this as a natively supported feature of Symfony. Maybe as a helper class, supported and maintained within Symfony, rather than as a third party package. |
|
It seems this issue is really stuck for last 2 years. Need to move or close Could you help Ping @symfony/deciders ? |
|
I just found, there is article about this topic in Symfony docs: Building a single Command Application Pretty straightforward solution. |
|
Closing in favor of #16906, which seems better. |
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
This is PR for the discussions started at #9564
intended to make it easier (less boilerplate) to create single command applications
At the moment it is still work in progress, but already started this pull request to keep the discussion going