[Console] Bash completion integration #42251

wouterj · 2021-07-24T16:33:10Z

Q	A
Branch?	5.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #38275
License	MIT
Doc PR	-

This is a first shot at implementing interactive bash completion support in the Console component. Besides completing option and command names, commands can override Command::complete() to implement completing values.

I've added an example code to the secrets:remove command, which now supports autocompletion quite nicely:

And support for other applications using symfony/console is automatically included (if the autocompletion script is installed for that specific application):

This PR only implements Bash completion. Zsh and Fish have much more sophisticated completion systems, but I propose to keep those for a future PR if there is a need for this.

How it works

A bash completion function (_console) is defined by bin/console completion bash for the console command (name of the "binary" file)
This completion function calls the local bin/console _complete command to come up with suggestions
Bash parses these suggestions and shows them to the user.

This has one drawback: the _console function is defined globally only once. This means we cannot easily change it, as it would break if you run different Symfony versions. We should probably add versioning (e.g. bin/console _complete --version=1.0) and don't suggest anything if the version doesn't match.

~~Maybe it would be safer to mark this feature as experimental and target 6.0, to allow us to fine tune the shell related sides over the lifespan of 6.x?~~ #42251 (comment)

Steps to test yourself

Load this PR in your project, open a bash shell and run this command to "install" completion for this project:

bin/console completion bash > /etc/bash_completion.d/console

Then reload the bash shell and enjoy autocompletion.

TODO

Autocompleting in the middle of the input doesn't work yet (i.e. bin/console --en<TAB> cache:clear)
Better error handling
Add a bin/console completion command to dump the _console file, so users can install this locally
Add some versioning, to allow us to change the _console file in the future
~~See how we can better support standalone usage (e.g. Composer)~~ Tested on Laravel's artisan, works flawlessly

Nyholm · 2021-07-25T06:18:35Z

Thank you.

This looks super cool. =)

wouterj · 2021-07-25T10:50:05Z

Thanks :)

I've updated this PR to complete most of the important TODO items (and also updated the usage in the PR description). It still needs more unit tests (I used a local shell script to test things, I need to convert it to unit tests) and I need to add a line to the CHANGELOG. Other than that, I think this in a final state, so ready for reviews :)

src/Symfony/Component/Console/Resources/completion.bash

src/Symfony/Component/Console/Completion/Output/BashCompletionOutput.php

OskarStark · 2021-08-01T16:26:49Z

Should we target 6.0 and mark this big new feature as @experimental?

Nyholm · 2021-08-01T16:29:57Z

My general opinion is that we mark too many classes experimental.

But I let @wouterj decide in this specific case.

wouterj · 2021-08-02T10:47:44Z

The public API side of this feature is quite minimal (one method in Console which needs full BC and 2 classes that we should mark as @final). The feature itself is purely DX (i.e. it's not that big of a deal if it stops working), so I agree with not marking this as experimental.

Adding support for ZSH might break things, but a community member already stepped up to provide a PR for this if this is merged, so this can be caught before the 5.4 release. Other than that, all complexity of this feature is in the CompletionInput::bind() method, which will all be normal "bug fixes".

I guess the lack of reviewers in this PR indicates that it's probably a bit difficult to review. Some hints:

The main logic is in CompleteCommand and CompletionInput. The command receives an array of input "words" (similar to $argv) and the index in that array of the current cursor position (to know where completion is requested). Based on this, CompletionInput has to provide the correct type, value, option name, etc. That is the core logic of this PR (see also its tests).
From a DX side, see SecretsRemoveCommand::complete(), Command::complete() and Application::complete() for PHP usage of this feature.
If you're into bash, consider reviewing completion.bash and BashCompletionOutput. That latter is not doing much (only providing completion options as space-separated values). The first one is a bit more complex than the basic bash completion function, in order to simplify working with colons in the command names and = signs in between option name and value.

dkarlovi · 2021-08-03T11:36:59Z

@wouterj I'm looking for a simple way to test this in my actual app, IMO it would be most beneficial to figure it all out. I'll provide my findings if I find a way to do that.

Other than that, a cool sample might be to make a fork of Composer with this enabled. It might make it simple to visualize once you can test it for yourself easily.

dkarlovi · 2021-08-03T11:37:45Z

Also, this is an absurdly valuable feature, thank you a lot for working on this! 😍

wouterj · 2021-08-03T12:44:46Z

@dkarlovi if you have an app running Symfony 5.3, you can try to clone my fork locally and use the link utility:

git clone https://github.com/wouterj/symfony -b issue-38275/shell-autocomplete
cd symfony
./link /path/to/project

And then use the eval $(bin/console completion bash) in your app.

dkarlovi · 2021-08-05T09:54:50Z

@wouterj which Bash did you use to test this? For me, if I test with bin/console, it doesn't work, but ./console works, but need to change CWD obviously.

To get it to work, I needed to change it like this:

complete -F _sf_console bin/console

Also, the eval trick you gave didn't register the completion for me (checked with complete -p console), had to pipe it to a file and source it

bin/console completion bash > foo.sh
source foo.sh

src/Symfony/Bundle/FrameworkBundle/Command/SecretsRemoveCommand.php

src/Symfony/Component/Console/Resources/completion.bash

src/Symfony/Component/Console/Command/CompleteCommand.php

dkarlovi · 2021-08-05T10:11:55Z

One thing which might be neat to have is to provide a flag for the dumper, something like

bin/console completion bash --verbose-completion

or similar, where it would create additional code in there to help you debug it if you're working on it. Might be too niche, maybe wait for use cases, but as I was testing this, I needed to edit the completion function to add them myself.

theofidry

A few minor comments/nitpicks. A big 👍 for this feature

src/Symfony/Component/Console/Completion/Completion.php

src/Symfony/Component/Console/Completion/CompletionInput.php

theofidry · 2021-10-15T08:02:39Z

src/Symfony/Component/Console/Completion/CompletionInput.php

+
+            $option = $this->getOptionFromToken($optionToken);
+            if (null === $option && !$this->isCursorFree()) {
+                $this->completionType = self::TYPE_OPTION_NAME;


I tend to find this sort of API a bit brittle: it's easy to miss that all properties need to be set together. I am also not sure if this class is supposed to be extendable or not (it is not final at least, didn't check how hard/easy it would be to extend).

WDYT of adding a private accessor setCompletionValues(string $type, string $anme, string $value)?

I do agree with you that this a bit brittle, but this is indeed not meant to be extended (I've made the class final now - thanks to your comment). The same comment as #42251 (comment) applies here imho.
Besides, we don't always set all 3 properties at the same time, sometimes only 1 or 2.

src/Symfony/Component/Console/Completion/CompletionInput.php

theofidry · 2021-10-15T08:05:02Z

src/Symfony/Component/Console/Completion/CompletionInput.php

+
+        $relevantToken = $this->getRelevantToken();
+        if ('-' === $relevantToken[0]) {
+            // the current token is an input option: complete either option name or option value


what about moving the different blocks into dedicated methods? e.g. addCompletionForInputOption()

As with all parsing, it's really horrible code (lots of nested if statements) but not so easy to clean-up.

In this case, $relevantToken[0] === '-' can have multiple meanings: Either it's just - or --, indicating the next tokens are all argument values. Or it's the start of an option (short or long). And things get even more messy with things like -eprod, -vvv and -ev (indicating respectively an option with its value, an option without value, and 2 options).

I also don't think it's worth cleaning this code up. All "messiness" is contained in a single method, which will not be touched regularly (how we parse options, arguments and values is pretty static).

wouterj · 2021-10-18T19:08:07Z

Thanks for the review @theofidry!

PR updated and rebased. We have a new failure (7.2) but I can't find any error logs whatsoever about this failure.

src/Symfony/Component/Console/Command/DumpCompletionCommand.php

src/Symfony/Component/Console/Command/CompleteCommand.php

src/Symfony/Component/Console/Command/DumpCompletionCommand.php

wouterj · 2021-10-19T08:38:20Z

Thanks, PR updated.

fabpot · 2021-10-19T09:33:51Z

Thank you @wouterj.

This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Console] Bash completion integration | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #38275 | License | MIT | Doc PR | - This is a first shot at implementing interactive bash completion support in the Console component. Besides completing option and command names, commands can override `Command::complete()` to implement completing values. I've added an example code to the `secrets:remove` command, which now supports autocompletion quite nicely: ![render1630315116886](https://user-images.githubusercontent.com/749025/136708284-bf2e4c12-7cb7-4d5e-9c8d-68bcdca6fd7c.gif) And support for other applications using `symfony/console` is automatically included (if the autocompletion script is installed for that specific application): ![render1630314403752](https://user-images.githubusercontent.com/749025/136708323-dfbccb77-dcbd-4d1e-8bb5-85b88f0b358b.gif) This PR only implements Bash completion. Zsh and Fish have much more sophisticated completion systems, but I propose to keep those for a future PR if there is a need for this. ### How it works 1. A bash completion function (`_console`) is defined by `bin/console completion bash` for the `console` command (name of the "binary" file) 2. This completion function calls the local `bin/console _complete` command to come up with suggestions 3. Bash parses these suggestions and shows them to the user. This has one drawback: the `_console` function is defined globally only once. This means we cannot easily change it, as it would break if you run different Symfony versions. We should probably add versioning (e.g. `bin/console _complete --version=1.0`) and don't suggest anything if the version doesn't match. <s> **Maybe it would be safer to mark this feature as experimental and target 6.0, to allow us to fine tune the shell related sides over the lifespan of 6.x?** </s> symfony/symfony#42251 (comment) ### Steps to test yourself Load this PR in your project, open a bash shell and run this command to "install" completion for this project: ``` bin/console completion bash > /etc/bash_completion.d/console ```` Then reload the bash shell and enjoy autocompletion. ### TODO * [x] Autocompleting in the middle of the input doesn't work yet (i.e. `bin/console --en<TAB> cache:clear`) * [x] Better error handling * [x] Add a `bin/console completion` command to dump the `_console` file, so users can install this locally * [x] Add some versioning, to allow us to change the `_console` file in the future * [x] <s>See how we can better support standalone usage (e.g. Composer)</s> Tested on Laravel's artisan, works flawlessly Commits ------- e0a174f877 [FrameworkBundle] Add CLI completion to secrets:remove 82ef399de3 [Console] Bash completion integration

stof · 2021-10-19T15:18:19Z

src/Symfony/Component/Console/Command/DumpCompletionCommand.php

+        if (!file_exists($debugFile)) {
+            touch($debugFile);
+        }
+        $process = new Process(['tail', '-f', $debugFile], null, null, null, 0);


what happens on windows (and yes, bash can be used on windows, which does not imply that tail is available in the PATH AFAICT) ?

This needs some more special casing. For example, you might not have symfony/process installed.

GromNaN · 2021-10-19T20:27:07Z

src/Symfony/Component/Console/Completion/CompletionInput.php

+        return $parseOptions;
+    }
+
+    private function getOptionValueFromToken(string $optionToken): string


Is this function actually used?

GromNaN · 2021-10-19T20:47:17Z

src/Symfony/Component/Console/Completion/CompletionInput.php

+
+    private $tokens;
+    private $currentIndex;
+    private $indexToArgumentIndex = [];


This property is not used.

wouterj requested a review from chalasr as a code owner July 24, 2021 16:33

carsonbot added Status: Needs Work Status: Needs Review Console Feature labels Jul 24, 2021

carsonbot changed the title ~~[WIP] [Console] Bash completion integration~~ [Console] [WIP] Bash completion integration Jul 24, 2021

wouterj force-pushed the issue-38275/shell-autocomplete branch 2 times, most recently from 47efd2b to 0623991 Compare July 24, 2021 19:46

wouterj force-pushed the issue-38275/shell-autocomplete branch from 0623991 to 5986f27 Compare July 25, 2021 10:46

wouterj changed the title ~~[Console] [WIP] Bash completion integration~~ [Console] Bash completion integration Jul 25, 2021

wouterj removed the Status: Needs Work label Jul 25, 2021

wouterj force-pushed the issue-38275/shell-autocomplete branch from 5986f27 to 8873ba0 Compare July 25, 2021 10:52

wouterj commented Jul 25, 2021

View reviewed changes

src/Symfony/Component/Console/Resources/completion.bash Outdated Show resolved Hide resolved

src/Symfony/Component/Console/Completion/Output/BashCompletionOutput.php Outdated Show resolved Hide resolved

wouterj force-pushed the issue-38275/shell-autocomplete branch from 8873ba0 to cff89a6 Compare July 27, 2021 19:13

derrabus added this to the 5.4 milestone Jul 29, 2021

wouterj force-pushed the issue-38275/shell-autocomplete branch from cff89a6 to ca0b1db Compare July 30, 2021 09:45

This comment has been minimized.

Sign in to view

dkarlovi suggested changes Aug 5, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Aug 5, 2021

theofidry reviewed Oct 15, 2021

View reviewed changes

wouterj force-pushed the issue-38275/shell-autocomplete branch from f071e5f to 7810d19 Compare October 18, 2021 19:02

fabpot requested changes Oct 19, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Oct 19, 2021

wouterj added 2 commits October 19, 2021 10:38

[Console] Bash completion integration

82ef399

[FrameworkBundle] Add CLI completion to secrets:remove

e0a174f

wouterj force-pushed the issue-38275/shell-autocomplete branch from 7810d19 to e0a174f Compare October 19, 2021 08:38

wouterj added Status: Needs Review and removed Status: Needs Work labels Oct 19, 2021

fabpot approved these changes Oct 19, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 19, 2021

fabpot mentioned this pull request Oct 19, 2021

[Console] Bash completion integration symfony/symfony-docs#15947

Closed

fabpot merged commit cf8a997 into symfony:5.4 Oct 19, 2021

wouterj deleted the issue-38275/shell-autocomplete branch October 19, 2021 09:40

stof reviewed Oct 19, 2021

View reviewed changes

stof mentioned this pull request Oct 19, 2021

[Console] Implement completion for zsh #43592

Closed

GromNaN reviewed Oct 19, 2021

View reviewed changes

froschdesign mentioned this pull request Oct 20, 2021

Simplify autocompletion support laminas/laminas-cli#84

Open

stof mentioned this pull request Oct 20, 2021

[Console] Add completion for fish #43602

Closed

This was referenced Nov 5, 2021

Release v5.4.0-BETA1 #43937

Merged

Release v6.0.0-BETA1 #43939

Merged

fritzmg mentioned this pull request Nov 29, 2021

Check for specific commands in ApplicationTest contao/contao#3745

Closed

wouterj mentioned this pull request Jan 7, 2022

[Console] Add completion values to input definition #44948

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Bash completion integration #42251

[Console] Bash completion integration #42251

wouterj commented Jul 24, 2021 •

edited

Loading

Nyholm commented Jul 25, 2021

wouterj commented Jul 25, 2021

OskarStark commented Aug 1, 2021

Nyholm commented Aug 1, 2021

wouterj commented Aug 2, 2021 •

edited

Loading

dkarlovi commented Aug 3, 2021

dkarlovi commented Aug 3, 2021

wouterj commented Aug 3, 2021

This comment has been minimized.

dkarlovi commented Aug 5, 2021

dkarlovi commented Aug 5, 2021

theofidry left a comment •

edited

Loading

theofidry Oct 15, 2021

wouterj Oct 18, 2021

theofidry Oct 15, 2021

wouterj Oct 18, 2021

wouterj commented Oct 18, 2021

wouterj commented Oct 19, 2021

fabpot commented Oct 19, 2021

stof Oct 19, 2021

dkarlovi Oct 20, 2021

GromNaN Oct 19, 2021 •

edited

Loading

GromNaN Oct 19, 2021

[Console] Bash completion integration #42251

[Console] Bash completion integration #42251

Conversation

wouterj commented Jul 24, 2021 • edited Loading

How it works

Steps to test yourself

TODO

Nyholm commented Jul 25, 2021

wouterj commented Jul 25, 2021

OskarStark commented Aug 1, 2021

Nyholm commented Aug 1, 2021

wouterj commented Aug 2, 2021 • edited Loading

dkarlovi commented Aug 3, 2021

dkarlovi commented Aug 3, 2021

wouterj commented Aug 3, 2021

This comment has been minimized.

dkarlovi commented Aug 5, 2021

dkarlovi commented Aug 5, 2021

theofidry left a comment • edited Loading

Choose a reason for hiding this comment

theofidry Oct 15, 2021

Choose a reason for hiding this comment

wouterj Oct 18, 2021

Choose a reason for hiding this comment

theofidry Oct 15, 2021

Choose a reason for hiding this comment

wouterj Oct 18, 2021

Choose a reason for hiding this comment

wouterj commented Oct 18, 2021

wouterj commented Oct 19, 2021

fabpot commented Oct 19, 2021

stof Oct 19, 2021

Choose a reason for hiding this comment

dkarlovi Oct 20, 2021

Choose a reason for hiding this comment

GromNaN Oct 19, 2021 • edited Loading

Choose a reason for hiding this comment

GromNaN Oct 19, 2021

Choose a reason for hiding this comment

wouterj commented Jul 24, 2021 •

edited

Loading

wouterj commented Aug 2, 2021 •

edited

Loading

theofidry left a comment •

edited

Loading

GromNaN Oct 19, 2021 •

edited

Loading