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

[DX] Add completion to Symfony commands #43594

Closed
54 tasks done
GromNaN opened this issue Oct 19, 2021 · 72 comments · Fixed by #43850
Closed
54 tasks done

[DX] Add completion to Symfony commands #43594

GromNaN opened this issue Oct 19, 2021 · 72 comments · Fixed by #43850
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony) Help wanted Issues and PRs which are looking for volunteers to complete them. Keep open

Comments

@GromNaN
Copy link
Member

GromNaN commented Oct 19, 2021

Completion of command arguments and options is coming in Symfony 5.4 (#38275). Command and option names are already autocompleted, but argument and option values needs to be implemented on each command.

As an example, #42251 added completion to secrets:remove.

This feature needs to be implemented on Symfony commands that has predictable values for their arguments and options.

Some guidelines:

PLEASE, comment here BEFORE starting to work on something and WAIT for the confirmation to avoid duplicate work.

All commands have been claimed

Commands that does not require auto-completion:

  • Command about
  • Command assets:install (target) (directory completion)
  • Command cache:clear
  • Command cache:pool:list
  • Command cache:pool:prune
  • Command cache:warmup
  • Command debug:dotenv
  • Command dotenv:dump
  • Command debug:validator (class) @Kocal completion support not yet possible
  • Command lint:container
  • Command messenger:stop-workers
  • Command router:match (path_info)
  • Command secrets:decrypt-to-local
  • Command secrets:encrypt-from-local
  • Command secrets:generate-keys
  • Command secrets:list
  • Command server:log (--host, --format) @StaffNowa [MonologBridge] Add completion for server:log #43892
  • Command ulid:inspect
  • Command uuid:inspect
@carsonbot carsonbot added the DX DX = Developer eXperience (anything that improves the experience of using Symfony) label Oct 19, 2021
@wouterj wouterj added Console Help wanted Issues and PRs which are looking for volunteers to complete them. and removed Console labels Oct 19, 2021
@wouterj wouterj pinned this issue Oct 19, 2021
@GromNaN
Copy link
Member Author

GromNaN commented Oct 19, 2021

I'm taking help and list commands.

@GromNaN
Copy link
Member Author

GromNaN commented Oct 19, 2021

  1. Since compgen filters the suggestion with the current input, can we agree that we don't have to filter in PHP? If filtering is necessary, it should be implemented in CompletionSuggestions.

  2. Several commands uses the DescriptorHelper and have common options --format. Some code might be factorized.

@andyexeter
Copy link
Contributor

I'll take cache:pool:clear and cache:pool:delete.

Should I wait for #43596 to be merged? Seems like that has a couple of useful things for testing.

@GromNaN
Copy link
Member Author

GromNaN commented Oct 19, 2021

@andyexeter they are yours.
I will wait for reviews to decide if the testing class is accepted.

@StaffNowa
Copy link
Contributor

I can try help uuid and config 🤔

@IonBazan
Copy link
Contributor

IonBazan commented Oct 20, 2021

Can I take following?

  • debug:firewall
  • debug:form
  • debug:messenger
  • debug:router

Also, is there a way to suggest directories or files? Using Finder for that seems a bit overkill.

@GromNaN
Copy link
Member Author

GromNaN commented Oct 20, 2021

@StaffNowa ulid:generate and uuid:generate are for you.
@IonBazan you get debug:firewall, debug:form, debug:messenger, debug:router.

File and directory autocompletion seems to be a native feature of compgen: https://unix.stackexchange.com/a/26733

@dkarlovi
Copy link
Contributor

@wouterj and I discussed files/folders being completed by falling back when nothing is suggested by Symfony. It will require a small tweak to the completion script generator.

@IonBazan
Copy link
Contributor

@GromNaN I'm asking because compgen supports filtering files and directories: https://www.linuxjournal.com/content/more-using-bash-complete-command and some commands we use, like lint:twig should only accept *.twig files and some other accept only directories, not files.

@dkarlovi
Copy link
Contributor

@IonBazan you should assume paths will get completed by compgen indeed. 👍

The plan is to add a feature to the completion which will allow "signaling" from the completion output to the completion script, something like suggesting (just an example)

type: file, glob: *.twig

and the completion script running native filtering. I guess it's too late to add this now (feature freeze)? /cc @wouterj

@fabpot
Copy link
Member

fabpot commented Oct 20, 2021

@dkarlovi It's not too late, but we would need to add support quickly if we want to have it in 5.4.

@dkarlovi
Copy link
Contributor

@fabpot this described feature is sure nice, but take into account that the completion script should get tweaked that it falls back to the native completion anyway if Symfony doesn't suggest anything, as noted in #43594 (comment) That is a much smaller change so doable today.

It will result in paths being completed as they usually would, so it goes:

  1. use whatever Symfony suggested
  2. Symfony didn't suggest anything, use the native file system suggestions

(currently missing 2)

With the signaling feature:

  1. use whatever Symfony suggested
    1.1. if a signal, use the signal to invoke compgen (maybe other things in the future?) in a specific way and use its suggestions
    1.2. else a literal value, suggest those
  2. Symfony didn't suggest anything, use the native file system suggestions

(1.1. is the new signaling feature)

The "signaling" feature is absolutely a nice one, but IMO not a blocker. If somebody did it in time, it would be awesome, but it should get some design time since the use cases will need to be get worked out. The "fallback to filesystem when nothing suggested" thing is a blocker IMO, but luckily much simpler so can be done before 5.4.

@wouterj
Copy link
Member

wouterj commented Oct 20, 2021

To keep things focused: please open a poc PR or issue if we need to discuss the file completion more in detail. Personally, given the late timing of this feature, I would propose to just rely on the default file autocompletion as a fallback. Then, in 6.1 (or later, whenever someone has time) we can introduce filtered file autocompletion by using signaling.

Symfony <5.4 had zero auto completion, having a "working but not 100% feature complete" autocompletion in 5.4/6.0 is fine.

Since compgen filters the suggestion with the current input, can we agree that we don't have to filter in PHP? If filtering is necessary, it should be implemented in CompletionSuggestions.

Yes, agreed. We don't have to filter, compgen is capable of that.

@dkarlovi
Copy link
Contributor

dkarlovi commented Oct 20, 2021

I just realized falling back when Symfony suggests nothing doesn't work because Symfony might be right.

For example, completing this:

bin/console compo<TAB>

to

bin/console composer. # json or lock

is incorrect since Symfony was right to suggest nothing: in this context it's completing command names and there is no command matching this pattern, it would yield bad UX.

So, seems signaling (however primitive) is the only correct way forward.

Maybe we can suggest

compgen! # using ! as a special character, any syntax can do

when we want native completion. Later more semantics can be added.

fabpot added a commit that referenced this issue Oct 20, 2021
…NaN)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Console] Add completion to help & list commands

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

- Add completion for commands `help` and `list`
- Create a tester class to wrap completion code in tests
- To autocomplete option `--format`, supported formats are exposed by `DescriptorHelper`

Commits
-------

dc94c72 [Console] Add completion to help & list commands
@wouterj
Copy link
Member

wouterj commented Oct 20, 2021

Taking inspiration from https://github.com/posener/complete/tree/master , the only thing we might need to introduce in 5.4 is a complete.nothing "signal".

  1. Values suggested? -> complete suggested values
  2. "nothing" returned? -> don't do anything
  3. No values suggested (e.g. no complete support yet)? -> fallback to default bash behavior (i.e. complete files/dirs)

@dkarlovi
Copy link
Contributor

@GromNaN completion command actually has an argument which you can suggest, shell.

@welcoMattic
Copy link
Member

@GromNaN Great initiative! I can help to make auto-complete for translation:pull and translation:push commands

@GromNaN
Copy link
Member Author

GromNaN commented Oct 20, 2021

@welcoMattic you are the one for translation:pull and translation:push.

@wouterj
Copy link
Member

wouterj commented Oct 20, 2021

Just a recap of the debugging info (as it's not yet documented):

  1. You can enable debug mode by running export SYMFONY_COMPLETION_DEBUG=1 once in your shell
  2. Then open another terminal window and run bin/console completion --debug.
  3. Enjoy live debugging information when trying to use the bash completion in the shell of (1):
    image

fabpot added a commit that referenced this issue Nov 1, 2021
This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

Add completion for debug:twig

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

Add completion for debug:twig

Commits
-------

68c5e3c Add completion for debug:twig
@GromNaN
Copy link
Member Author

GromNaN commented Nov 2, 2021

Hello @makraz, can I take messenger:consume?

Edit: done #43891

@StaffNowa
Copy link
Contributor

Hey @makraz, do you have time or we can help you with server:log too?

@StaffNowa
Copy link
Contributor

@GromNaN now I see server:log. Do we need to autocomplete hostname and format? According me it is not predictable
--host localhost or any IP (can put some localhost IP addreses).
--format - not sure what we can predict :)

@GromNaN
Copy link
Member Author

GromNaN commented Nov 2, 2021

@GromNaN now I see server:log. Do we need to autocomplete hostname and format? According me it is not predictable --host localhost or any IP (can put some localhost IP addreses). --format - not sure what we can predict :)

What do you think of suggesting the default value for both? It would be useful as a starting point, instead of reading the doc.

@StaffNowa
Copy link
Contributor

@GromNaN now I see server:log. Do we need to autocomplete hostname and format? According me it is not predictable --host localhost or any IP (can put some localhost IP addreses). --format - not sure what we can predict :)

What do you think of suggesting the default value for both? It would be useful as a starting point, instead of reading the doc.

Yes good point👌

@StaffNowa
Copy link
Contributor

@GromNaN added default hosts and ports from real life :)

fabpot added a commit that referenced this issue Nov 3, 2021
…affNowa)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Add completion for workflow:dump

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

Add completion for workflow:dump
We do not have tests here at all

Commits
-------

5d3b60a [FrameworkBundle] Add completion for workflow:dump
fabpot added a commit that referenced this issue Nov 3, 2021
…e (GromNaN)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Add completion to command messenger:consume

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

Command `messenger:consume` takes a list of `receivers` that should be unique. The values for option `--bus` where easy to inject into the command.

I skipped the option `--queue` whose values are defined deep in the transport options.

Commits
-------

f99f586 [Messenger] Add completion to command messenger:consume
fabpot added a commit that referenced this issue Nov 3, 2021
This PR was merged into the 5.4 branch.

Discussion
----------

[Framework] Add completion to debug:container

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

The command `debug:container` has several options which cannot be used all together. This PR adds completion for service names, tag names, parameter names and output format.

Commits
-------

01fe89e [Framework] Add completion to debug:container
fabpot added a commit that referenced this issue Nov 3, 2021
… (scyzoryck)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Add  command completion for failed messages

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

Add command completion for failed messages.

Commits
-------

a01a895 [Messenger] Add completion for failed messages commands.
fabpot added a commit that referenced this issue Nov 4, 2021
…nt (eclairia, Adrien Jourdier)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

feat: add completion for DebugAutowiring search argument

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

Adding Bash completion debug:autowiring command.

Commits
-------

ef8c518 feat: add completion for DebugAutowiring search argument
@fabpot fabpot closed this as completed in d77697c Nov 4, 2021
@wouterj wouterj reopened this Nov 4, 2021
@wouterj
Copy link
Member

wouterj commented Nov 4, 2021

Hi @stephenkhoo

do I need to include that test?

Yes, you can add a test for the completion support.

There is no direct need to add all missing tests (but if you want to do that as well, a PR is welcome to e.g. 4.4 or 5.3).

@makraz
Copy link
Contributor

makraz commented Nov 8, 2021

Hey @makraz, do you have time or we can help you with server:log too?

Hi @StaffNowa sorry just back to online work

@StaffNowa
Copy link
Contributor

@makraz Hey, it is not needed anymore for the server:log command. People decided do not nothing for this command :)

@fabpot fabpot closed this as completed Nov 8, 2021
@wouterj wouterj unpinned this issue Nov 18, 2021
GromNaN added a commit that referenced this issue Nov 29, 2021
…stephenkhoo)

This PR was merged into the 5.4 branch.

Discussion
----------

[Framework] Add completion to `debug:event-dispatcher`

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

Rework PR from `@stephenkhoo`

Commits
-------

97160dd Complete event name & dispatcher in EventDispatcherDebugCommand
SerhiyMytrovtsiy added a commit to SerhiyMytrovtsiy/translation that referenced this issue Oct 19, 2022
…ll and push commands (welcoMattic)

This PR was merged into the 5.4 branch.

Discussion
----------

[Translation] Add completion feature on translation pull and push commands

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix (partially) symfony/symfony#43594 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        |

This PR adds completion for arguments and options on `translation:pull` and `translation:push` commands.

Commits
-------

036b03278f Add completion feature on translation pull and push commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony) Help wanted Issues and PRs which are looking for volunteers to complete them. Keep open
Projects
None yet