[arch-projects] rc.d bash_completion
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
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@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?
+which rc.d >/dev/null 2>&1 &&
'type -P', never which.
+_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 We don't use camel case, either.
+ 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.
+ + _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[*]}" )
+ 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.
+ ;; + 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
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@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
On Wed, Jun 08, 2011 at 01:32:36PM +0200, Andrwe wrote: 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.
Sorry, I fail to see how keeping things as strings makes things _easier_. In my experiences, and as shown here, it leads to complex solutions which are not easy to read. I've still yet to sit down and really grok what the hell is going on in that regex. Set subtraction and intersection can be done using grep or comm. I prefer grep as it doesn't require the inputs to be sorted. For example: We can do intersection... elements from array1 are matched with elements from array2. $ grep -xFf <(printf '%s\n' "${array1[@]}") <(print '%s\n' "${array2[@]}") We can do subtraction... array1 is subtracted from array2. $ grep -vxFf <(printf '%s\n' "${array1[@]}") <(print '%s\n' "${array2[@]}") And if all else fails, really, what's the problem with using nested for loops? It's iteration of small datasets using bash builtins. That's guaranteed to be fast enough for this. I've still got 2 major concerns: 1) You're parsing Bash which you expect to have a specific form, when in fact, there is no required format. Someone could easily come along and create a script which your completion cannot handle. For all this added complexity, you get functionality which is going to be rarely used. I, of course, don't have the final say in this, but I really prefer the simplicity of our current approach. That said, it does have some bugs. I encourage you to fix them. Which brings me to point 2... 2) This is still a code dump, and as such, I don't think anyone will accept this regardless of how good or bad it is. You've created a diff of your script versus what we currently have in git (and not the official git). We accept properly formed patches (that means commit messages) sent with git-send-email. Alternatively, I know Tom is a fan of pull requests from remote branches, such as my initscripts or mkinitcpio repos on Github. regards, dave
On Thu, Jun 9, 2011 at 2:43 PM, Dave Reisner <d@falconindy.com> wrote:
On Wed, Jun 08, 2011 at 01:32:36PM +0200, Andrwe wrote: 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.
Sorry, I fail to see how keeping things as strings makes things _easier_. In my experiences, and as shown here, it leads to complex solutions which are not easy to read. I've still yet to sit down and really grok what the hell is going on in that regex.
Set subtraction and intersection can be done using grep or comm. I prefer grep as it doesn't require the inputs to be sorted. For example:
We can do intersection... elements from array1 are matched with elements from array2.
$ grep -xFf <(printf '%s\n' "${array1[@]}") <(print '%s\n' "${array2[@]}")
We can do subtraction... array1 is subtracted from array2.
$ grep -vxFf <(printf '%s\n' "${array1[@]}") <(print '%s\n' "${array2[@]}")
And if all else fails, really, what's the problem with using nested for loops? It's iteration of small datasets using bash builtins. That's guaranteed to be fast enough for this.
I've still got 2 major concerns:
1) You're parsing Bash which you expect to have a specific form, when in fact, there is no required format. Someone could easily come along and create a script which your completion cannot handle. For all this added complexity, you get functionality which is going to be rarely used. I, of course, don't have the final say in this, but I really prefer the simplicity of our current approach. That said, it does have some bugs. I encourage you to fix them. Which brings me to point 2...
2) This is still a code dump, and as such, I don't think anyone will accept this regardless of how good or bad it is. You've created a diff of your script versus what we currently have in git (and not the official git). We accept properly formed patches (that means commit messages) sent with git-send-email. Alternatively, I know Tom is a fan of pull requests from remote branches, such as my initscripts or mkinitcpio repos on Github.
Functionnality is useful, but i agree on two points. -- Sébastien Luttringer www.seblu.net
participants (3)
-
Andrwe
-
Dave Reisner
-
Seblu