beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.96k stars 1.82k forks source link

zsh completion cache is broken #2266

Open ogarcia opened 8 years ago

ogarcia commented 8 years ago

Problem

The zsh completion is broken. After zsh loads the cached functions you give the following error when try to complete (beet <tab>).

_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
_beet:4: command not found: _ra_comp
(repeat several times)

My zshrc file is very simple.

# Lines configured by zsh-newuser-install
HISTFILE=~/.histfile
HISTSIZE=1000
SAVEHIST=1000
bindkey -e
# End of lines configured by zsh-newuser-install
# The following lines were added by compinstall
zstyle :compinstall filename '/home/ogarcia/.zshrc'

autoload -Uz compinit
compinit
# End of lines added by compinstall

The problem is introduced in this commit: https://github.com/beetbox/beets/commit/5282d2882eb9cd4cb7e9d9370256c599575bcaee

sampsyo commented 8 years ago

Thanks for reporting! @vapniks, can you please take a look?

vapniks commented 8 years ago

@ogarcia that is strange, _ra_comp must exist otherwise it would not have been added to the cache file. Could you try entering _ra_comp on the command line and tell me what happens? Also could you post a copy of your _beet_cached file?

ogarcia commented 8 years ago

@vapniks the _ra_comp returns command not found.

inferno% _ra_comp
zsh: command not found: _ra_comp

And the copy of _beet_cached: _beet_cached.txt

vapniks commented 8 years ago

What version of zsh are you using?

ogarcia commented 8 years ago

zsh 5.2 on Arch Linux.

If you wish you can test it on vagrant machine.

vagrant init ogarcia/archlinux-x64; vagrant up --provider virtualbox
vapniks commented 8 years ago

Could you try deleting _beet_cached, starting a new shell, try completion again, and tell me what happens.

ogarcia commented 8 years ago

Same error. In fact the first test was do with clean environment.

.zshrc file:

# Lines configured by zsh-newuser-install
HISTFILE=~/.histfile
HISTSIZE=1000
SAVEHIST=1000
bindkey -e
# End of lines configured by zsh-newuser-install
# The following lines were added by compinstall
zstyle :compinstall filename '/home/vagrant/.zshrc'

autoload -Uz compinit
compinit
# End of lines added by compinstall

You can make tests easy:

vagrant init ogarcia/archlinux-x64; vagrant up --provider virtualbox
vagrant ssh # enter in virtual machine

sudo pacman -Sy # Update package database
sudo pacman -S zsh beets # Install beets and zsh
zsh # Enter in zsh shell and follow initial assistant to minimal config or copy from above.

beet <tab> # try completion, works well in first time
exit # exit from zsh to force load completion from cached file
zsh # enter in zsh again
beet <tab> # BAM! ERROR!!! xDDD

Note, in arch the _beet_cached is stored in ~/.config/beets/ cause $HOME/.oh-my-zsh/custom/completions not exists by default.

NOTE: The completion is stored in /usr/share/zsh/site-functions/_beet

vapniks commented 8 years ago

I can't think what the problem can be. Did you change the value of completionsdir at the top of the _beets file? I will have a look at vagrant (new to me).

ogarcia commented 8 years ago

@vapniks Yes, the completionsdir was changed to ~/.config/beets/ as I say in my last update :wink:

Vagrant is a tool to get virtual machines. It's very easy and it allows have clean environments to make tests.

In Ubuntu you only need install VirtualBox and do sudo apt-get install vagrant to get vagrant up and running.

Jucgshu commented 7 years ago

Hey guys,

Is there a way to disable the autocompletion feature in zsh to avoid this bug ?

Thanks ;p

ogarcia commented 7 years ago

@Jucgshu the best way is simply remove the completion file:

sudo rm /usr/share/zsh/site-functions/_beet

Other option is use an old version before cache "patch".

cd /tmp
wget 'https://raw.githubusercontent.com/beetbox/beets/9dc79950d3d9ebdfe09224e27e3af258f7438ae6/extra/_beet'
sudo cp _beet /usr/share/zsh/site-functions/_beet
rm _beet

@vapniks any news about this?

Jucgshu commented 7 years ago

Yeap, switching to the previous did the trick.

Thanks @ogarcia ✨

simonbcn commented 7 years ago

Same problem in Arch Linux with zsh.

ogarcia commented 7 years ago

@simonbcn yes, in Arch we have this bug: https://bugs.archlinux.org/task/51819 but it depends of this.

sampsyo commented 7 years ago

Perhaps we should just roll back the change?

ogarcia commented 7 years ago

@sampsyo yes, I think that may be a temporary fix while the bug is investigated in detail. :wink:

sampsyo commented 7 years ago

OK! Just to double-check, is 9dc79950d3d9ebdfe09224e27e3af258f7438ae6 the right revision to roll back to?

ogarcia commented 7 years ago

@sampsyo yes https://github.com/beetbox/beets/commit/9dc79950d3d9ebdfe09224e27e3af258f7438ae6 works :+1:

vapniks commented 7 years ago

Sorry, had some more important things to attend to, and forgot about this. Anyway, you have something working now. It may be a while before I get a chance to properly investigate the problem. I am unable to download the vagrant box ogarcia/archlinux-x64.

barraponto commented 7 years ago

My experiments today found that adding _ra_comp to _beet_cached fixes the issues. I'll add it to the instructions as soon as #2436 is merged.

barraponto commented 7 years ago

FYI, _ra_comp gets created once you run the autocomplete for the first time (the first step in the instructions).

vapniks commented 7 years ago

If others ( @ogarcia @sampsyo @simonbcn ) can confirm that this works then I will update _beet to automatically create _beet_cached with _ra_comp included.

To test it:

First try completing beet to ensure that the beet completion functions are created. Then create a _beet_cached file like this: (replacing /path/to/zsh/completions with a directory in your $fpath where completion functions can be stored. If none of these are user writable you can add one like this: fpath+=~/.zshcompletions)

beetcachefile=/path/to/zsh/completions/_beet_cached
which _ra_comp > $beetcachefile
which _beet_field_values >> $beetcachefile
which _beet_query >> $beetcachefile
which _beet >> $beetcachefile
echo zstyle \":completion:\${curcontext}:\" tag-order \'! options\' >> $beetcachefile
echo "\n\n_beet \"\$@\"\n" >> $beetcachefile

Then change the completion function for beet to _beet_cached:

autoload -zU _beet_cached
compdef _beet_cached beet

Try completing beet again, and let me know if it works in the comments below.

simonbcn commented 7 years ago

I have reinstalled beets-git from AUR package now but the autocomplete zsh functions doesn't exist. Should not it be installed with the package?

And what is /path/to/zsh/completions/? And $cachefile (in my system doesn't exist)?

ogarcia commented 7 years ago

This is a very bad idea. You are making a new completion from a completion. That is ilogic.

Please, take a look at this, more specifically _store_cache and _retrieve_cache that do the trick.

simonbcn commented 7 years ago

https://github.com/beetbox/beets/issues/2266#issuecomment-279182352

First try completing beet to ensure that the beet completion functions are created. ....

https://github.com/beetbox/beets/issues/2266#issuecomment-279207657

And what is /path/to/zsh/completions/? And $cachefile (in my system doesn't exist)?

ogarcia commented 7 years ago

A small sample of cache functions.

my_fancy_var='Hello World!' # make a var
my_other_var='We can put tons of info'
_store_cache put_here_the_name_that_you_wish my_fancy_var my_other_var # store vars in your cache named put_here_the_name_that_you_wish
# Open a new console
_retrieve_cache put_here_the_name_that_you_wish # get your cache name put_here_the_name_that_you_wish
echo $my_fancy_var # return your cached vars
echo $my_other_var
simonbcn commented 7 years ago

@ogarcia I don't sure but I think you're confused. With those instructions we create a cache file to accelerate the completion, but it isn't the completion function itself.

ogarcia commented 7 years ago

@simonbcn nop, with this guide you makes a new completion file with completion functions. That not is the way. If you want use cache you must use zsh completion cache functions that works transparently for users.

simonbcn commented 7 years ago

@ogarcia ok, and repeat:

And what is /path/to/zsh/completions/? And $cachefile (in my system doesn't exist)?

I can't test it if I don't know how do it!

Then this post should be corrected: https://github.com/beetbox/beets/issues/2266#issuecomment-279182352

First try completing beet to ensure that the beet completion functions are created. ....

ogarcia commented 7 years ago

@simonbcn In Archlinux the zsh completions dir is /usr/share/zsh/site-functions/ but may vary in other distros.

But I repeat, this can work, but this is not te way.

aperezdc commented 7 years ago

@simonbcn: Also /usr/share/zsh/site-functions/ will be readonly for non-root users. You should use _store_cache, _retrieve_cache, and _invalidate_cache (as suggested by @ogarcia) because they will use an user-specific location for storing cached data, and they respect the use-cache completion widget setting as well.

simonbcn commented 7 years ago

ok, then this issue is still not resolved, am I right?

ogarcia commented 7 years ago

@simonbcn no, the issue is still open

vapniks commented 7 years ago

Sorry for the mistake in the previous post. I have corrected the error, please read it again. If that works for others then I can quickly fix _beet so that it automatically creates the _beet_cached with the _ra_comp function included. If you don't like this method, then you can try fixing it yourself using _store_cache, _retrieve_cache and _invalidate_cache, but I don't have time to do that right now.

xeals commented 7 years ago

Bit of an old thread, but my current computer takes a long time to generate completions on the fly, so I looked into this. Adding the _ra_comp function to the cache using which _ra_comp >> $cachefile does work.

vapniks commented 6 years ago

OK, I've now fixed _beet to use the proper zsh caching functions _store_cache, _retrieve_cache and _cach_invalid: https://github.com/beetbox/beets/commit/209d281871e7d86bf60729f2ca25e60b0aaa0e2b Let me know if you still have problems.

ogarcia commented 6 years ago

Great!

But I see a small issue. The line 16:

typeset -g BEETS_LIBRARY="$(beet config|grep library|cut -f 2 -d ' ')"

must be:

typeset -g BEETS_LIBRARY="${$(beet config|grep library|cut -f 2 -d ' '):-~/.config/beets/library.db}"

Because if you has not defined the library in your config file the BEETS_LIBRARY variable value is always empty.

UPDATE: I found other issue. Seems that cached values in fields are never used again. If I remove the cache and do beet list <tab> can complete fields, but if I open a new console and do the same cannot complete it.

vapniks commented 6 years ago

Thanks. I'm making a few more changes aswell.

vapniks commented 6 years ago

OK, I've updated it so that it now caches subcommand completers only when it needs to. This makes it a bit faster, but means there are a lot of cache files (one for each subcommand).

ogarcia commented 6 years ago

@vapniks for BEETS_CONFIG variable i think that is better use:

BEETS_CONFIG="$(beet config -p)"

What do you think?

vapniks commented 6 years ago

Yes, good idea. Done.

ogarcia commented 6 years ago

I think that now works well and is fast :1st_place_medal:.

May be better (or a future enhancement) have only a cache file, but have several ones not is a problem.

vapniks commented 6 years ago

The problem is that there is no function to add to a cache file; you can only (re)create the whole thing in one go, which means that if you want to do incremental caching it makes more sense to have separate cache files. If someone could write an _append_cache function (e.g. by making some adjustments to _storecache) that would be really useful.. fancy trying it? Another problem with my solution is that it pollutes the environment with lots of variables (with names starting `beets`). Unfortunately this is unavoidable when using zsh completion caches since the variables are made global when retrieved from the cache.