[Bash-completion-devel] Review Request: hellanzb
David Paleino
d.paleino at gmail.com
Mon Feb 7 09:20:08 UTC 2011
Hello Stefano,
On Sun, 5 Dec 2010 14:58:38 +0200, Stefano Rivera wrote:
> I just fell down the rabbit hole of writing bash completion scripts. I
> wish I knew where my Sunday morning went... :)
>
> Here's a completion script for hellanzb. Review please?
Sorry for the need of a poke :)
> One question:
> "_filedir -d" doesn't seem interested in subdirectories, it adds a space
> as soon as there's an unambiguous completion. It doesn't do this without
> the -d. Is that really the desired behaviour?
With the current git revision, it doesn't add any space. It must've been fixed
already.
> BTW: Are there guidelines about when bash completion scripts belong in
> bash-completion and when they belong in the package they complete for? I
> couldn't find any...
Unfortunately, not. This is a grey area, and we (the team) have had different
opinions over the time.
Here's the review:
> # hellanzb(1) completion script
> # Copyright 2010, Stefano Rivera <stefano at rivera.za.net>
We don't have a particular policy about completions licensing; I believe giving
copyright to you is not a problem, as long as the license is compatible with
ours. :)
> have hellanzb && _hellanzb_opts() {
^^^
It's better if you do something like:
have hellanzb && {
... # the whole completion
} &&
complete -F ...
> # First extract long options then short options & rpc commands
> hellanzb -h 2> /dev/null \
> | sed -nre '/^ .*, --/ {h; s/.*(--[a-z-]+).*/\1/ p; g}' \
> -e 's/^ ([a-zA-Z-]+)[, ].*/\1/ p'
> }
This catches both "remote-calls" and proper "options". You might want to split
this.
Also, we have an helper function, _parse_help(), you might give it a look. I
can't tell whether it works or not (probably not, but it's worth a try, at
least)
> [ -n "${have:-}" ] && _hellanzb() {
^^^^^^^^^^^^^^^^^^^^^
See the first comment :)
> local cur prev
> COMPREPLY=()
> cur="${COMP_WORDS[COMP_CWORD]}"
> prev="${COMP_WORDS[COMP_CWORD-1]}"
Use "_get_comp_words_by_ref cur prev" instead of those two lines (cur=* and
prev=*).
> if [[ $COMP_CWORD -eq 1 ]]; then
> COMPREPLY=( $(compgen -W "$(_hellanzb_opts)" -- "$cur") )
> return
> fi
> if [[ "$prev" = "enqueue" ]]; then
> _filedir '@(nzb|NZB|zip|ZIP)'
^^^^^^^^^^^^^^^^^^^^
_filedir now supports uppercase versions of the given extensions automatically.
Thus a "_filedir '@(nzb|zip)'" should suffice.
> return
> fi
> local dir_options file_options
> file_options="-c --config -l --log-file -d --debug-file"
> if echo "" $file_options | grep -Fwq -- "$prev"; then
> _filedir
> return
> fi
> dir_options="process -p --post-process-dir"
> if echo "" $dir_options | grep -Fwq -- "$prev"; then
> _filedir -d
> return
> fi
Given you're hardcoding some options here, does it make sense to parse them
from the help output? Why not hardcoding them all? Do they really change that
often?
> # Complete commands if we haven't had one yet
> local i hellanzb_opts commands
> hellanzb_opts="$(_hellanzb_opts)"
> commands="$(echo "" $hellanzb_opts | tr ' ' '\n' | grep -v '^-')"
> for ((i=0; i < COMP_CWORD; i++)); do
> if echo "" $commands | grep -Fwq -- "${COMP_WORDS[i]}"; then
> return
> fi
> done
> COMPREPLY=( $(compgen -W "$hellanzb_opts" -- "$cur") )
All of this would be simpler if you parse/hardcode commands and options in two
different variables.
Also, from the help output, I see:
hellanzb [options] [remote-call] [remote-call-options]
so I understand that position on the command line is important. You could
follow one of the other completions, which more or less all have the following
structure:
case $prev in
command1)
# complete for remote-call-options for command1
return 0
;;
...
esac
if [[ $cur == -* ]]; then
COMPREPLY=( $( compgen -W '--option1 --option2 --option3' -- "$cur" ) )
return 0
else
# complete commands, since we're not starting with "-"
fi
>}
>[ -n "${have:-}" ] && complete -F _hellanzb hellanzb
See the first comment :)
Additional comments: we usually use "return 0" instead of simply "return"; and
we prefer long options over short ones. This means, if you have "-h" and
"--help", only complete "--help".
I have missed something for sure. Please take your time to review this
completion, and don't hesitate to ask if you need help :)
Thank you for your work!
Have a nice day,
David
--
. ''`. Debian developer | http://wiki.debian.org/DavidPaleino
: :' : Linuxer #334216 --|-- http://www.hanskalabs.net/
`. `'` GPG: 1392B174 ----|---- http://deb.li/dapal
`- 2BAB C625 4E66 E7B8 450A C3E1 E6AA 9017 1392 B174
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/bash-completion-devel/attachments/20110207/610afe02/attachment.pgp>
More information about the Bash-completion-devel
mailing list