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

Use shell functions instead of aliases to avoid interfering with shell completion #433

Open
jimeh opened this issue Feb 23, 2024 · 7 comments
Labels
op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers.

Comments

@jimeh
Copy link

jimeh commented Feb 23, 2024

op CLI version

2.24.0

Goal or desired behavior

I want to setup the homebrew shell plugin (as an example), and have shell completion work correctly for brew, and also my custom shell alias (alias br=brew).

This can be achieved by using a shell function in ~/.config/op/plugins.sh instead of an alias:

brew() { op plugin run -- brew "$@"; }

Current behavior

Currently when you run op plugin init brew, a shell alias is written to ~/.config/op/plugins.sh, for example:

alias brew="op plugin run -- brew"

This results in the brew command no longer having shell completions available on it, unless setopt completealiases is used. But that breaks completion for shorthand aliases like alias br=brew, because it will try to find completions for a command named br.

Hence the shell function approach is the only one that works correctly in both scenarios.

Relevant log output

No response

@jimeh jimeh added the op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers. label Feb 23, 2024
@mrclrchtr
Copy link

Thank you so much for sharing the workaround!

@mrjones2014
Copy link
Member

Note for myself: we'll also need to make this change in ./nix/shell-plugins.nix. In a Nix environment, the aliases and managed by Nix instead of by the 1Password CLI.

@mrjones2014
Copy link
Member

I started playing around with this idea and ran into a small snag -- it's easy to do in the Nix Flake way of configuring things, because we can just generate separate versions of the shell code.

However, for users not using Nix and having the CLI manage their plugins.sh file, we run into a bit of an issue with Fish shell users, because Fish uses a different function syntax.

Bash/Zsh:

my-func() {
  echo "stuff"
}

Fish:

function my-func
  echo "stuff
end

I'm not sure the best way to approach this with the CLI.

@jimeh
Copy link
Author

jimeh commented May 11, 2024

Simplest approach I guess would to also write out a plugin.fish file, and just update instructions for fish users to load that file instead.

Another approach could be to have a plugin.bash file for bash/zsh and plugin.fish for fish. Then have the existing plugin.sh load one of the files conditionally based on the value of $SHELL. Assuming such a conditional can be written in a bash and fish compatible way.

@jimeh
Copy link
Author

jimeh commented May 12, 2024

In the meantime, I've made my own hacky but automated solution in my dotfiles:

https://github.com/jimeh/dotfiles/blob/f30c675f3b5cebd6a0b58be3905425e50d1b4ba3/zsh/1password.zsh

Why? Because automating stuff is what we do right? Even if said automation might be horrible 😱... Hence, I don't suggest anyone uses my hacky solution, but if you fully understand how it works, and you're still ok with it, feel free to steal it... lol

It basically loads the plugins.sh file as is with aliases, and then finds all aliases which call op plugin run --, replacing them with relevant functions. The shell source that does the replacement is written to a cache file as well which is only updated when needed, ensuring shell startup times aren't slowed down.

herder added a commit to herder/dotfiles that referenced this issue Jun 19, 2024
Was caused by the 1password plugins messing up completions.

Issue: 1Password/shell-plugins#433
@punk-dev-robot
Copy link

Hi, I also recently stumbled upon this issue. This is a clever solution but unfortunately, it doesn't work for some completions I use.

It looks like it's popular for completions to include that snippet of code (example from glab - which is gitlab cli tool.

# don't run the completion function when being source-ed or eval-ed
if [ "$funcstack[1]" = "_glab" ]; then
    _glab
fi

I think when we use the function method described here it changes the location of glab on funcstack?

@punk-dev-robot
Copy link

I don't have much experience in zsh scripting but the solution that reliably works for me atm is #122 (comment)

bclews added a commit to bclews/dotfiles that referenced this issue Jun 28, 2024
This commit replaces the aliases created by 1Password's CLI with shell
functions. This ensures proper shell completions for commands managed by
1Password’s CLI. The change includes updates to the `plugins.sh` file and
the addition of a new guide in the `@notes` directory. The Makefile has
been updated to exclude the `@notes` directory from stow/unstow operations.
The `gh` plugin has been added to the zsh plugins list in `.zshrc`.

see 1Password/shell-plugins#433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op-cli Functionality to be implemented in 1Password CLI. Needs to be done by 1Password Developers.
Projects
None yet
Development

No branches or pull requests

4 participants