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

some plugins don't work well without setopt completealiases #4407

Closed
la-magra opened this issue Sep 25, 2015 · 5 comments
Closed

some plugins don't work well without setopt completealiases #4407

la-magra opened this issue Sep 25, 2015 · 5 comments
Labels
Topic: alias Pull Request or issue regarding aliases Topic: completion Pull Request or issue regarding completion

Comments

@la-magra
Copy link
Contributor

Reproduction steps

  • default oh-my-zsh installation
  • enter a git repo
    cd ~/.oh-my-zsh
  • git checkout <tab>
HEAD           master         origin/HEAD    origin/master
  • gco <tab>
HEAD           master         origin/HEAD    origin/master
  • this is OK, now let's set completealiases
  • setopt completealiases
  • git checkout <tab>
HEAD           master         origin/HEAD    origin/master
  • OK
  • gco <tab>
cache/           custom/          lib/             log/             MIT-LICENSE.txt  oh-my-zsh.sh     plugins/         README.markdown  templates/       themes/          tools/
  • NOT OK

Test environment data

Ubuntu 15.04 x64
zsh 5.0.7 (x86_64-pc-linux-gnu)
oh-my-zsh (76a26a2)

Cygwin
zsh 5.0.8 (x86_64-unknown-cygwin)
oh-my-zsh (76a26a2)

Further information

5b75cc7 seems to be the commit having made the regression.
Alias completion of git commands works with or without setopt completealiases in commits past that one.

Solution

Make sure the git plugin fully works even when completealiases is set.
Or even better, that aliases completion like gco<tab> works only when completealiases is set...

@apjanke
Copy link
Contributor

apjanke commented Sep 28, 2015

I can reproduce on OS X 10.9, and expect it will be the same on all platforms: this is normal behavior for complete_aliases.

Yeah, this is from #2790 and the commit 5b75cc7 you identified. The current implementation assumes complete_aliases is off, which is the default for OMZ since 8883ace. See this comment.

To fix it to work with complete_aliases, those compdefs need to be added back in.

Many other plugins and core aliases will also be affected by complete_aliases, since they generally do not explicitly define compdefs to go with the alias definitions. (Although it won't matter for many of them, since they take files as arguments and the default completion will work.) Maybe a more experienced OMZ developer can comment on interaction between OMZ and complete_aliases, and whether it's supported?

@apjanke
Copy link
Contributor

apjanke commented Sep 28, 2015

As an aside, why do you have complete_aliases set?

@la-magra
Copy link
Contributor Author

Yep, just read c3a58b0 too.
I kind of feel that this issue has more to do with the standardization of completions of (I guess) most plugins or the fact that the git plugin is non standard among all others (is there even a standard?...)?

Pretty much the reason for completealiases is because several other plugins do not work fully without it. And it makes sense (in some cases):
no completealiases => no alias completion, completealiases => alias completion.

Some examples with rake and debian plugins:

unsetopt completealiases

Enter some ruby/rails project.

cd somerails
rake db:<tab>

No completion.
Let's try the debian plugin.

ai zs<tab>

No completion, while

aptitude (or apt-get) install zs<tab>
zsh-antigen   zsh-beta-doc  zsh-dbg       zsh-doc       zsh-static    zssh                                  
zsh-beta      zshdb         zsh-dev       zsh-lovers    zsnes-dbg     zsync

works as intended.
Now with completealiases on:

setopt completealiases

the rake plugin

rake db:<tab>
db:create                -- creates the database from database_url or config/database.yml for the current rails_
db:drop                  -- drops the database from database_url or config/database.yml for the current rails_en
db:fixtures:load         -- load fixtures into the current environment's database
db:migrate               -- migrate the database (options: version=x, verbose=false, scope=blog)
...

completes correctly, and the debian plugin too:

ai zs<tab>
zsh-antigen   zsh-beta-doc  zsh-dbg       zsh-doc       zsh-static    zssh                                  
zsh-beta      zshdb         zsh-dev       zsh-lovers    zsnes-dbg     zsync

@apjanke
Copy link
Contributor

apjanke commented Sep 28, 2015

Yeah, I think you're right that it's about standardization of how plugins do completion.

In my opinion, if a plugin requires complete_aliases at this point, that's a bug in the plugin. The default options after the core OMZ code loads are "authoritative", and plugins should be written to work under that state. It's probably a historical accident that some of them require it: they were probably written back when complete_aliases was the default in OMZ and didn't get updated when that changed.

You're right, we should standardize all the plugins to work with one setting or the other. But I think the correct one is "off": going with complete_aliases on would require extra, possibly tricky, configuration work for every alias we define (about 1,200 of them in all of OMZ), and would force users to do so as well for aliases they define themselves. If you leave complete_aliases off, you get completion for many aliases "for free".

On the flip side, if you have a case where you have an alias and you do want a distinct completion behavior for it (as opposed to the completion that would happen for the expanded alias), you can easily fix that on a per-command basis by converting the alias to a function.

The debian plugin is a good example. There's an open issue for it #3686. Check out the extra code it used to get working with complete_aliases enabled. And it is still buggy (e.g. it's using the apt-get completion when it should be using the aptitude completion).

I don't use rails, so I don't know why the db:... completion for rake requires complete_aliases. (Can you point me at an example rails project I can use to reproduce it?) But check out the flip side: that plugin also defines srake, brake, and other commands. Getting completion for them needs no_complete_aliases: if I do srake -<tab>, it will complete the options for me; if complete_aliases is on, it won't.

Let's fix the plugins to work under no_complete_aliases. I'll work through debian and rake with you. Let me know if you see others that are broken.

@mcornella mcornella changed the title git plugin aliases completion disabled by setopt completealiases some plugins don't work well without setopt completealiases Dec 15, 2015
@mcornella mcornella added Bug Something isn't working Topic: alias Pull Request or issue regarding aliases labels Dec 15, 2015
@mcornella mcornella removed the Bug Something isn't working label Jun 12, 2018
@mcornella mcornella added the Topic: completion Pull Request or issue regarding completion label Mar 24, 2019
@mcornella
Copy link
Member

When you mentioned the rake command you must have been referring to the rails plugin, which is already protected against nocompletealiases because they are functions: https://github.com/robbyrussell/oh-my-zsh/blob/6d143d42ea2e16610855e5b1e912b2962d44d9ab/plugins/rails/rails.plugin.zsh#L30-L31

As per the debian plugin, there is already issue #3686 open, so I'm closing this issue unless there are other plugins that should be fixed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: alias Pull Request or issue regarding aliases Topic: completion Pull Request or issue regarding completion
Projects
None yet
Development

No branches or pull requests

3 participants