[Console] Add option to automatically run suggested command if there is only 1 alternative #25732

pierredup · Jan 9, 2018

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

When mistyping a console command, you get an error giving suggested commands.
If there is only 1 alternative suggestion, this PR will give you the option to run that command instead. This makes it easier to run the correct command without having to re-type/copy-paste/update the previous run command

chalasr · Jan 11, 2018

src/Symfony/Component/Console/Exception/NamespaceNotFoundException.php

+ */
+class NamespaceNotFoundException extends CommandNotFoundException
+{
+}


This addition is not strictly needed, right? Not sure it's useful, and it involves a BC break with no upgrade path in the future (stop extending CommandNotFoundException, new unhandled exceptions in userland).

No it's not strictly needed, but I needed a way to distinguish between not finding a namespace and not finding a command to prevent incorrect suggestions when typing an incorrect namespace. Keeping this class extending from CommandNotFound seemed weird as they are for different use cases, which is why I added the deprecation

chalasr · Jan 11, 2018

src/Symfony/Component/Console/Exception/NamespaceNotFoundException.php

+ *
+ * @author Pierre du Plessis <pdples@gmail.com>
+ *
+ * @deprecated This class won't extend CommandNotFoundException in Symfony 5


@deprecated should only be used when a class is actually deprecated, the debug class loader parses such annotations, could lead to wrong notices e.g. when extending it.

chalasr · Jan 12, 2018

src/Symfony/Component/Console/Application.php

@@ -226,7 +228,18 @@ public function doRun(InputInterface $input, OutputInterface $output)
                $e = $event->getError();
            }

-            throw $e;
+            if (!($e instanceof CommandNotFoundException && !$e instanceof NamespaceNotFoundException) || 1 !== count($alternatives = $e->getAlternatives()) || !$input->isInteractive()) {


Doing this here keeps dispatching a console.error event, not sure it's desired in case the user choose to run the single alternative? Especially given it's currently not taking into account the potential changes made on the event (see comment below)

chalasr · Jan 12, 2018

src/Symfony/Component/Console/CHANGELOG.md

-* removed `ConsoleExceptionEvent`
-* removed `ConsoleEvents::EXCEPTION`
+ * removed `ConsoleExceptionEvent`
+ * removed `ConsoleEvents::EXCEPTION`


please revert, should be done on 3.4

chalasr · Jan 12, 2018

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

@@ -1,5 +1,5 @@

-In ApplicationTest.php line 770:
+In ApplicationTest.php line 795:


Going to be a nightmare, we should make the related tests more flexible by using a placeholder where they have been introduced

Addressed in #25794

chalasr · Jan 12, 2018

src/Symfony/Component/Console/Application.php

+            $question = new ConfirmationQuestion(sprintf("<error>Command \"%s\" is not defined.</error>\n\nDo you want to run \"%s\" instead? [y/n] ", $name, $alternative), false);
+
+            if (!(new QuestionHelper())->ask($input, $output, $question)) {
+                return 1;


The error event allows to set a custom exit code and override the exception, it must keep working

chalasr · Jan 12, 2018

No it's not strictly needed, but I needed a way to distinguish between not finding a namespace and not finding a command to prevent incorrect suggestions when typing an incorrect namespace

How that? AFAIU it behaves the same for both exceptions currently

fabpot · Jan 23, 2018

@pierredup Can you rebase to get rid of the merge commit? We never merge a PR with a merge commit. Thanks.

… alternative

chalasr · Jan 29, 2018

src/Symfony/Component/Console/Application.php

-            if (null !== $this->dispatcher) {
-                $event = new ConsoleErrorEvent($input, $output, $e);
-                $this->dispatcher->dispatch(ConsoleEvents::ERROR, $event);
+            if (!($e instanceof CommandNotFoundException && !$e instanceof NamespaceNotFoundException) || 1 !== count($alternatives = $e->getAlternatives()) || !$input->isInteractive()) {


This proposes to run the alternative even if it's a namespace only, right? A test case would be good to make sure it does not

No, if it's a namespace only then the behaviour stays unchanged (Which would giving you an error and giving a list suggested namespaces without the option to run a command)

Added another test case to cover namespace error

chalasr · Feb 8, 2018

src/Symfony/Component/Console/Application.php

+                return 1;
+            }
+
+            $command = $this->find($alternative);


~~I suggest to use SymfonyStyle instead~~ wrong line

chalasr · Feb 8, 2018

src/Symfony/Component/Console/Tests/ApplicationTest.php

-            $this->assertRegExp('/There are no commands defined in the "foo2" namespace./', $e->getMessage(), '->find() throws a CommandNotFoundException if namespace does not exist, with alternative');
-            $this->assertRegExp('/foo/', $e->getMessage(), '->find() throws a CommandNotFoundException if namespace does not exist, with alternative : "foo"');
-            $this->assertRegExp('/foo1/', $e->getMessage(), '->find() throws a CommandNotFoundException if namespace does not exist, with alternative : "foo1"');
-            $this->assertRegExp('/foo3/', $e->getMessage(), '->find() throws a CommandNotFoundException if namespace does not exist, with alternative : "foo3"');


They are still valid and ensure the new class keeps extending it (prevent a BC break)

I would revert and only add a new assertion, avoiding merge conflicts

chalasr · Feb 11, 2018

src/Symfony/Component/Console/Application.php

+            $alternative = $alternatives[0];
+            $question = new ConfirmationQuestion(sprintf("<error>Command \"%s\" is not defined.</error>\n\nDo you want to run \"%s\" instead? [y/n] ", $name, $alternative), false);
+
+            if (!(new QuestionHelper())->ask($input, $output, $question)) {


I suggest to use SymfonyStyle instead

chalasr · Feb 17, 2018

@pierredup Can you look at the failing tests? looks like a spacing issue.

pierredup · Feb 22, 2018

New failures on travis seems unrelated

fabpot · Feb 22, 2018

Thank you @pierredup.

…mmand if there is only 1 alternative (pierredup) This PR was squashed before being merged into the 4.1-dev branch (closes #25732). Discussion ---------- [Console] Add option to automatically run suggested command if there is only 1 alternative | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | When mistyping a console command, you get an error giving suggested commands. If there is only 1 alternative suggestion, this PR will give you the option to run that command instead. This makes it easier to run the correct command without having to re-type/copy-paste/update the previous run command ![console](https://user-images.githubusercontent.com/144858/34724377-4b46c726-f556-11e7-94a3-a9d7c9d75e74.gif) Commits ------- 83d52f0 [Console] Add option to automatically run suggested command if there is only 1 alternative

carsonbot added Status: Needs Review Console Feature Deprecation labels Jan 9, 2018

pierredup force-pushed the console-run-command-suggestion branch from e8f9bdf to 5abc30f Compare January 9, 2018 14:02

nicolas-grekas added this to the 4.1 milestone Jan 9, 2018

pierredup changed the title ~~[Console] Add options to automatically run suggested command if there is only 1 alternative~~ [Console] Add option to automatically run suggested command if there is only 1 alternative Jan 9, 2018

pierredup force-pushed the console-run-command-suggestion branch 2 times, most recently from 3524d9d to bc36d20 Compare January 9, 2018 20:26

chalasr reviewed Jan 11, 2018

View reviewed changes

chalasr requested changes Jan 12, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jan 12, 2018

pierredup force-pushed the console-run-command-suggestion branch from 6c1cd01 to 34222d7 Compare January 15, 2018 12:31

chalasr self-requested a review January 19, 2018 14:47

carsonbot added Status: Needs Review and removed Status: Needs Work labels Jan 19, 2018

pierredup added 3 commits January 23, 2018 08:33

Add options to automatically run suggested command if there is only 1…

5bdcdda

… alternative

Remove deprecation and future BC break

d93ca23

Revert unrelated changes in the changelog

f42a2bb

pierredup force-pushed the console-run-command-suggestion branch from 6e93303 to 222af7c Compare January 23, 2018 06:36

Dispatch console error event

e4deda7

pierredup force-pushed the console-run-command-suggestion branch from 222af7c to e4deda7 Compare January 23, 2018 06:38

chalasr reviewed Jan 29, 2018

View reviewed changes

Dont run command if suggestion is only a namespace

1b9f9eb

fabpot approved these changes Feb 7, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Feb 7, 2018

chalasr reviewed Feb 8, 2018

View reviewed changes

chalasr reviewed Feb 11, 2018

View reviewed changes

pierredup added 3 commits February 11, 2018 20:49

Use SymfonyStyle

f8a816d

Revert some changes to tests

bf46ffe

Fix CS

d7dc6c7

Change spacing

4de52d2

fabpot approved these changes Feb 22, 2018

View reviewed changes

fabpot closed this Feb 22, 2018

pierredup deleted the console-run-command-suggestion branch February 22, 2018 06:50

javiereguiluz mentioned this pull request Mar 2, 2018

[Console] Add option to automatically run suggested command if there is only 1 alternative symfony/symfony-docs#9377

Closed

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Add option to automatically run suggested command if there is only 1 alternative #25732

[Console] Add option to automatically run suggested command if there is only 1 alternative #25732

pierredup commented Jan 9, 2018 •

edited

Loading

chalasr Jan 11, 2018 •

edited

Loading

pierredup Jan 11, 2018

chalasr Jan 11, 2018

chalasr Jan 12, 2018

chalasr Jan 12, 2018

chalasr Jan 12, 2018

pierredup Jan 15, 2018

chalasr Jan 12, 2018

chalasr commented Jan 12, 2018 •

edited

Loading

fabpot commented Jan 23, 2018

chalasr Jan 29, 2018

pierredup Jan 29, 2018

pierredup Jan 29, 2018

chalasr Feb 8, 2018 •

edited

Loading

chalasr Feb 8, 2018

chalasr Feb 8, 2018

chalasr Feb 11, 2018

chalasr commented Feb 17, 2018

pierredup commented Feb 22, 2018

fabpot commented Feb 22, 2018

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

[Console] Add option to automatically run suggested command if there is only 1 alternative #25732

[Console] Add option to automatically run suggested command if there is only 1 alternative #25732

Conversation

pierredup commented Jan 9, 2018 • edited Loading

chalasr Jan 11, 2018 • edited Loading

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

chalasr commented Jan 12, 2018 • edited Loading

fabpot commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr Feb 8, 2018 • edited Loading

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

chalasr commented Feb 17, 2018

pierredup commented Feb 22, 2018

fabpot commented Feb 22, 2018

pierredup commented Jan 9, 2018 •

edited

Loading

chalasr Jan 11, 2018 •

edited

Loading

chalasr commented Jan 12, 2018 •

edited

Loading

chalasr Feb 8, 2018 •

edited

Loading