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 bash-completion for docker command linked from inside the Docker.app #59696

Closed
petr-ujezdsky opened this issue Mar 2, 2019 · 3 comments
Closed
Labels

Comments

@petr-ujezdsky
Copy link

Description of feature/enhancement

Add bash-completion for docker command linked from inside the Docker.app.

Justification

Non-cask docker has support for bash-completion (although it does not work right now, see bug) and so could the bundled one in Docker.app.

Right now you have to link those bash-completion files manually as is written in docker docs.

It would be nice to have this out-of-the-box.

Example use case

$ brew cask install docker
$ . ~/bash_profile
$ docker st<TAB>
stack  start  stats  stop
@vitorgalvao
Copy link
Member

We don’t typically add the completions. Since in this case it’d mean using the Homebrew (non-Cask) directories and we don’t yet handle conflicts with formulae, adding this could be problematic, so I’d be wary of making this happen (as of now).

Either way, you’re unlikely to get this unless you submit a PR.

@petr-ujezdsky
Copy link
Author

I thought that when you bundle the docker command with Docker.app and link it into /usr/local/bin, you could do the same with completions :)

$ ls -l /usr/local/bin | grep docker
... docker -> /Applications/Docker.app/Contents/Resources/bin/docker

And the brew [un]link command could [un]link the completions too (if it don't do that already).
Btw right now the cask docker command clashes with non-cask docker. I have found it out when upgrading non-cask docker and still seeing older version in Docker.app.

Unfortunately I am lacking the brew knowledge to make a patch.

@vitorgalvao
Copy link
Member

I thought that when you bundle the docker command with Docker.app and link it into /usr/local/bin

Which can itself lead to problems, because

we don’t yet handle conflicts with formulae

you could do the same with completion

If we already do one thing that could be problematic, that doesn’t make it an excuse to make a second point of problems. That’s how bad unmaintainable software gets written. The solution here is to fix the first problem. Only then can we consider adding the second one, which wouldn’t be a problem anymore.

And the brew [un]link command could [un]link the completions too (if it don't do that already).

That would be making non-cask commands directly affect cask installs with no way of communicating. Not a good idea, and hard to implement for no real benefit.

Btw right now the cask docker command clashes with non-cask docker

Because it doesn’t overwrite the binary already in place and bails earlier, I think. Which means it’s bailing out of necessity, not by design.

I have found it out when upgrading non-cask docker and still seeing older version in Docker.app.

Which means that had the conflicts resolution been in place, you wouldn’t have been able to install the app with the formula at the same time, and would not have gotten that surprise.

Whichever way you go about this, it comes back to the same thing: we need to add formula conflicts before we think about complicating the cask. Especially since you’re the only person asking for this.

Unfortunately I am lacking the brew knowledge to make a patch.

In that case, I’ll close earlier, if you don’t mind. The core team is unlikely to work on this, so the issue would just linger open.

Thank you anyway for the request, but as it is right now, we can’t proceed with it.

@lock lock bot added the outdated label Apr 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants