GNOME Bugzilla – Bug 738369
bash-completion install unfit for completion autoloading
Last modified: 2015-01-28 09:32:28 UTC
Since version 1.90, bash-completion has new completion loading code. The completion files are loaded on demand, with ${completionsdir}/${command} being sourced when a matching command is typed. This requires the completion files to be named for all commands provided by package. Currently, gvfs installs a single 'gvfs' file which simply does not work with this scheme. It needs to install 'gvfs-ls', 'gvfs-info', 'gvfs-cat', ... I suggest installing the completion file for one of the named commands, and symlinking other commands to it.
Is somewhere any documentation for bash-completion? Will bash-completion loads completion code just once when we use symlinks? I'm not sure using symlinks in git repository is good idea. I'd rather use separate files and include the common completion code instead.
I overlooked the "See Also" link, where is told that there isn't any official "documentation" for.
(In reply to comment #1) > Is somewhere any documentation for bash-completion? Will bash-completion loads > completion code just once when we use symlinks? I'm not sure using symlinks in > git repository is good idea. I'd rather use separate files and include the > common completion code instead. As for your second question, the code will load completions only once. If you type, say: $ gvfs-ls [tab] bashcomp looks for ${completionsdir}/gvfs-ls and sources that. The file has 'complete ...' command which defines completion for all commands. Since all commands have completions now, none of the relevant commands will trigger autoloading again. In other words, the symlinks are there to load completions on any gvfs-* command being typed. Once one of them is completed, the remaining commands found in the file are loaded as well. Since all commands use the same completion function, I don't think there is a point in using multiple files. That would actually cause the common code to be sourced multiple times :). Lastly, I don't think you should put the symlinks in a git repository. Instead, create them in 'install' target. For example, bash-completion itself does the following in completions/Makefile.am: symlinks: $(targetdir) $(DATA) for file in aclocal-1.11 ; do \ rm -f $(targetdir)/$$file && \ $(LN_S) aclocal $(targetdir)/$$file ; \ done #... .PHONY: symlinks all-local: targetdir = . all-local: symlinks install-data-local: targetdir = $(DESTDIR)$(bashcompdir) install-data-local: symlinks However, that is rather complex and I think it'd be enough to just do something along the lines of [untested!]: install-symlinks: $(LN_S) gvfs-ls $(completionsdir)/gvfs-open $(LN_S) gvfs-ls $(completionsdir)/gvfs-info #... install-data-local: install-symlinks .PHONY: install-symlinks
Created attachment 288408 [details] [review] programs: allow bash completition for gvfs-rename and gvfs-set-attribute
Created attachment 288409 [details] [review] programs: install completion files for all commands
Review of attachment 288409 [details] [review]: Thanks for the quick patch :). However, I have a few minor notes as listed below. ::: programs/completion/Makefile.am @@ +28,3 @@ +install-symlinks: + for file in $(symlinks) ; do \ + $(LN_S) -f gvfs-cat $(BASHCOMP_DIR)/$$file ; \ This construct does not catch failures properly. Make will detect failure only if the last `ln -s` invocation fails. Failures in previous iterations will be discarded. I think you could add `set -e` on top of the shell block to force proper behavior. @@ +33,3 @@ +uninstall-symlinks: + for file in $(symlinks) ; do \ + rm -f $(BASHCOMP_DIR)/$$file ; \ A minor note but you may want to check whether it is a symlink before removing it.
Review of attachment 288408 [details] [review]: Both rename and set-attribute require more work since they take arguments that are not gvfs URIs. E.g. typing "gvfs-rename localtest:///tmp/oldname loc<TAB>" should not autocomplete. The same applies for set-attribute.
But e.g. "gvfs-save localtest:///a loc<TAB>" also doesn't make sense and it is completed. Wouldn't be better to have incomplete completion than don't have any?
Created attachment 288502 [details] [review] programs: install completion files for all commands Thanks for review, "set -e" added. Check for symlinks when removing isn't added, because I don't think it is necessary (since we create symlinks with -f and also don't check)...
(In reply to comment #8) > But e.g. "gvfs-save localtest:///a loc<TAB>" also doesn't make sense and it is > completed. Wouldn't be better to have incomplete completion than don't have > any? OK, fair enough.
Comment on attachment 288408 [details] [review] programs: allow bash completition for gvfs-rename and gvfs-set-attribute commit 1d98f7c8c4eb41fa83498bce23624e9cb89046e9
Ross, could you look at the attachment 288502 [details] [review] please? (Bash completion doesn't work e.g. in Fedora 21 currently.)
Review of attachment 288502 [details] [review]: Looks good, other than the missing $(DESTDIR) to support make install DESTDIR=... ::: programs/completion/Makefile.am @@ +28,3 @@ +install-symlinks: + set -e; for file in $(symlinks); do \ + $(LN_S) -f gvfs-cat $(BASHCOMP_DIR)/$$file; \ This needs to be: $(LN_S) -f gvfs-cat $(DESTDIR)$(BASHCOMP_DIR)/$$file; \ @@ +33,3 @@ +uninstall-symlinks: + set -e; for file in $(symlinks); do \ + rm -f $(BASHCOMP_DIR)/$$file; \ This needs to be: rm -f $(DESTDIR)$(BASHCOMP_DIR)/$$file; \
Created attachment 295621 [details] [review] programs: install completion files for all commands Thanks for the review, fixed :-)
Review of attachment 295621 [details] [review]: Looks good!
Comment on attachment 295621 [details] [review] programs: install completion files for all commands Thanks! Pushed to master and gnome-3-14 as: commit 9a0fe141561e9b9217bec658c886edef0a981ab6 commit a4400c2273277de1911af1efee7fd9f67279b44d