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

Add completion command and fix convert #9269

Merged
merged 4 commits into from Sep 26, 2022
Merged

Conversation

ulyssessouza
Copy link
Contributor

@ulyssessouza ulyssessouza commented Mar 11, 2022

What I did
Add completion command and fix convert

To test, you can trigger the completions with the following example:

  1. Create a compose file with the following content:
    compose.yaml
services:
  foo:
    image: nginx
  bar:
    image: nginx
  baz:
    image: nginx
  1. Then run the following command:
$ docker compose __complete up ""

This should print the 3 services as proposals

  1. By running
$ docker compose __complete up "b"

This should propose the 2 services starting with the letter "b"

For now, the Docker CLI is not triggering compose's completion yet. That should be done in a following PR.

@ulyssessouza ulyssessouza force-pushed the add-completion branch 2 times, most recently from f98c167 to 694c6b5 Compare September 15, 2022 01:53
@ulyssessouza ulyssessouza marked this pull request as ready for review September 15, 2022 11:07
@ulyssessouza ulyssessouza requested a review from a team September 15, 2022 11:08
@ulyssessouza ulyssessouza force-pushed the add-completion branch 2 times, most recently from bc0cb8c to 593d252 Compare September 19, 2022 22:50
Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Can't wait to get it connected to the docker CLI 🎉

@milas
Copy link
Contributor

milas commented Sep 21, 2022

Ah project name completion isn't working, it looks like the backend instance isn't initialized?

I'm getting not implemented error and all the funcs on the ServiceProxy are nil at the point that completeProjectNames is called 😭

@milas
Copy link
Contributor

milas commented Sep 21, 2022

Oh, we lazily initialize the ServiceProxy via PersistentPreRunE which presumably does NOT run for the auto-completion 😬

compose/cmd/main.go

Lines 41 to 44 in db88241

if err := plugin.PersistentPreRunE(cmd, args); err != nil {
return err
}
lazyInit.WithService(compose.NewComposeService(dockerCli))

ulyssessouza and others added 4 commits September 23, 2022 19:13
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Co-authored-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
@ulyssessouza
Copy link
Contributor Author

@milas Good catch!
I changed to populate the serviceProxy on creation... We could actually get rid of this, but that can be useful on state tracing with interceptors.

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Project name completion looks good now. Can't wait 🥳

@milas milas merged commit 140dc51 into docker:v2 Sep 26, 2022
@nicksieger
Copy link
Member

Should this close #8550 as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants