[arch-projects] rc.d bash_completion

[Andrwe]

On Wed, 8 Jun 2011 08:58:00 -0400, Dave Reisner wrote:
> On Wed, Jun 08, 2011 at 01:32:36PM +0200, Andrwe wrote:
>> Hi,
>>
>> I've improved the bash-completion of rc.d.
>> I created a patch for latest git
>> (https://github.com/seblu/arch-initscripts/tree/).
>>
>> The patched version extracts all actions of an initscript and adds
>> them to the list of possible actions and only list initscripts
>> supporting the given action.
>>
>>
>> Kind regards,
>> Andrwe
>
>> diff --git a/bash-completion b/bash-completion
>> index 5151972..03ce0e6 100644
>> --- a/bash-completion
>> +++ b/bash-completion
>> @@ -1,22 +1,50 @@
>> -# rc.d bash completion by Seblu <seblu at seblu.net>
>> -
>> -_rc.d ()
>> +# rc.d completion
>> +#
>> +#  Author: Andrwe Lord Weber
>> +#  Mail: lord-weber-andrwe <at> andrwe <dot> org
>> +#
>
> Ouch. Really? Not going to leave _any_ credit behind for the original
> author?

Sorry, forgot to add credits but will definitely.

>> +which rc.d >/dev/null 2>&1 &&
>
> 'type -P', never which.

Thanks for this hint.

>> +_rc_d()
>>  {
>> -	local action="help list start stop reload restart"
>> -	local cur="${COMP_WORDS[COMP_CWORD]}"
>> -	local caction="${COMP_WORDS[1]}"
>> -	if ((${COMP_CWORD} == 1)); then
>> -		COMPREPLY=($(compgen -W "${action}" -- "$cur"))
>> -	elif [[ "$caction" =~ help|list ]]; then
>> -		COMPREPLY=()
>> -	elif [[ "$caction" == start ]]; then
>> -		COMPREPLY=($(comm -23 <(cd /etc/rc.d && compgen -f -X 
>> 'functions*' "$cur"|sort) <(cd /run/daemons/ && compgen -f 
>> "$cur"|sort)))
>> -	elif [[ "$caction" =~ stop|restart|reload ]]; then
>> -		COMPREPLY=($(cd /run/daemons/ && compgen -f "$cur"|sort))
>> -	elif ((${COMP_CWORD} > 1)); then
>> -		COMPREPLY=($(cd /etc/rc.d && compgen -f -X 'functions*' 
>> "$cur"|sort))
>> +	local cur prev initdir long_opts cmds shoptExtglob
>> +
>> +	shoptExtglob="$(shopt -p | grep extglob)"
>
> No grep needed here:
>
>   shopt -p extglob

Thanks for this hint.

> We don't use camel case, either.

Didn't know but will respect it now.

>> +	shopt -s extglob
>> +
>> +	long_opts='list help'
>> +	initdir=/etc/rc.d
>> +	cmds="$(sed '/#.*sh/,/^.*case\s*"*\${*1}*"*.*/ { 
>> /^.*case\s*"*\${*1}*"*.*/!d 
>> };s/^.*case\s*"*\${*1}*"*.*/;;/g;/;;/,/^\s*.*)\s*$/ !d;/^\s*.*)\s*$/ 
>> !d;/\*/d;/;;/d;s/\s*//g;s/)//g;s/|/ /g' ${initdir}/!(*functions*) | 
>> LANG=C sort -u | tr '\n' ' ')"
>
> The default IFS is '\n\t ', so the final tr isn't needed here. This
> really should be an array, too.

Done.

>> +
>> +	_get_comp_words_by_ref cur prev
>> +
>> +	cmd="${COMP_WORDS[1]}"
>> +
>> +	if [[ "${cmd}" =~ ^(${cmds// /|}|${long_opts// /|})$ ]] && [ -n 
>> "${cmd}" ]; then
>> +		daemons="$(ls -1 /run/daemons/ | tr '\n' '|')"
>
> Never parse ls. The need for the pipe delimiter isn't obvious either.
>
>   pushd /run/daemons &>/dev/null && { daemons=(*); popd &>/dev/null; 
> }
>
> If you really need the pipe delimiter, you can then later:
>
>   ( IFS='|'; echo "${daemons[*]}" )


Is needed for the regex-based check whether it's a valid action and to 
decrease the amount of listed initscripts.
For now I've removed this functionality.

>> +		case "${cmd}" in
>> +			start)
>> +		                COMPREPLY+=( $( compgen -W '`grep -El "^[ ]*[^ 
>> ]*${cmd}[|\)][^ ]*\)*[ ]*$" ${initdir}/* | sed 's#${initdir}/##' | 
>> grep -vE "^(${daemons})$"`' -- "$cur" ) )
>> +				;;
>> +			stop|restart|reload)
>> +		                COMPREPLY+=( $( compgen -W '`grep -El "^[ ]*[^ 
>> ]*${cmd}[|\)][^ ]*\)*[ ]*$" ${initdir}/* | sed 's#${initdir}/##' | 
>> grep -E "^(${daemons})$"`' -- "$cur" ) )
>> +				;;
>> +			*)
>> +		                COMPREPLY+=( $( compgen -W '`grep -El "^[ ]*[^ 
>> ]*${cmd}[|\)][^ ]*\)*[ ]*$" ${initdir}/* | sed 's#${initdir}/##'`' -- 
>> "$cur" ) )
>
> We use $() for command substitution, not backticks.

Thanks for the hint.

>> +				;;
>> +		esac
>> +	else
>> +		COMPREPLY=( $( compgen -W '${long_opts} ${cmds}' -- "$cur" ) )
>>  	fi
>> -}
>> -complete -F _rc.d rc.d
>>
>> -# vim: set ts=2 sw=2 ft=sh noet:
>> +	${shoptExtglob}
>> +	return 0
>> +} &&
>> +complete -F _rc_d rc.d
>> +
>> +# Local variables:
>> +# mode: shell-script
>> +# sh-basic-offset: 4
>> +# sh-indent-comment: t
>> +# indent-tabs-mode: nil
>> +# End:
>> +# vim: ts=2 sw=2 et filetype=sh
>
> A general lack of commenting and an overbearing sense of 
> overcomplicating
> things gets a NACK from me.
>
> d

So here is a version which should satisfy your hints.
Why should I always use an array?
It's easier to manipulate a string using bash internals than 
manipulating an array.
Because of that I had to remove a functionality of the script because I 
don't know how I can merge two arrays by removing all elemts which can 
be found in both arrays without using multiple for-loops.


Kind regards,
Andrwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bash-completion.patch
Type: text/x-diff
Size: 2700 bytes
Desc: not available
URL: <http://mailman.archlinux.org/pipermail/arch-projects/attachments/20110609/73613ef6/attachment.bin>