Patchwork [enhancement,4] bash_completion Use bash features, prefer (( arithmetic expressions )), [[ conditional expressions ]], avoid useless filename globbing trials in [ … ] expressions

login
register
mail settings
Submitter Roland Eggner
Date Dec. 30, 2013, 7:19 p.m.
Message ID <20131230191920.GG23913@mobil.systemanalysen.net>
Download mbox | patch
Permalink /patch/3246/
State Superseded
Headers show

Comments

Roland Eggner - Dec. 30, 2013, 7:19 p.m.
# HG changeset patch
# Parent 9856580690a2eb4fd691f3c51dec6d1d966b2038
[enhancement 4] bash_completion  Use bash features, prefer (( arithmetic expressions )), [[ conditional expressions ]], avoid useless filename globbing trials in [ … ] expressions.

In a bash_completion script compatibility with other shells is pointless, thus use bash features which ease code maintainance.

For string comparisons use [[ … ]] expressions instead of [ … ], this avoids useless filename globbing trials of bash.
Sean Farley - Dec. 30, 2013, 7:39 p.m.
Thanks for the contribution! Overall, these seem fine but I admittedly
didn't do a thorough reading because these patches were
attachments. Please do take a moment to read

http://mercurial.selenic.com/wiki/ContributingChanges

which will guide you on the first line of the commit message and using
the patchbomb extension.
Roland Eggner - Dec. 30, 2013, 8:28 p.m.
On 2013-12-30 Monday at 13:39 -0600 Sean Farley wrote:
> Thanks for the contribution! Overall, these seem fine but I admittedly
> didn't do a thorough reading because these patches were
> attachments. Please do take a moment to read
> 
> http://mercurial.selenic.com/wiki/ContributingChanges

Already done, thanks for the hint.
AFAICS I have followed every point.

> which will guide you on the first line of the commit message and using
> the patchbomb extension.
Pierre-Yves David - Dec. 30, 2013, 9:25 p.m.
On 12/30/2013 12:28 PM, Roland Eggner wrote:
> On 2013-12-30 Monday at 13:39 -0600 Sean Farley wrote:
>> Thanks for the contribution! Overall, these seem fine but I admittedly
>> didn't do a thorough reading because these patches were
>> attachments. Please do take a moment to read
>>
>> http://mercurial.selenic.com/wiki/ContributingChanges
> Already done, thanks for the hint.
> AFAICS I have followed every point.

You failed at least for those two ;-)

1. first line of commit message is of the form "*topic: uncapitalized, 
no trailing period*"

4. patch is in the body of the email in the form produced by export 
<http://www.selenic.com/hg/help/export> for easy review

(no garantee that there is no other issue)


Welcome aboard!
Roland Eggner - Dec. 31, 2013, 1:23 a.m.

Patch

diff --git a/contrib/bash_completion b/contrib/bash_completion
--- a/contrib/bash_completion
+++ b/contrib/bash_completion
@@ -118,16 +118,16 @@  shopt -s extglob
     local i count=0
     local filters="$1"
 
-    for ((i=1; $i<=$COMP_CWORD; i++)); do
+    for ((i=1; i<=COMP_CWORD; i++)); do
 	if [[ "${COMP_WORDS[i]}" != -* ]]; then
-	    if [[ ${COMP_WORDS[i-1]} == @($filters|$global_args) ]]; then
+	    if [[ "${COMP_WORDS[i-1]}" == @($filters|$global_args) ]]; then
 		continue
 	    fi
-	    count=$(($count + 1))
+	    (( count++ ))
 	fi
     done
 
-    echo $(($count - 1))
+    echo $(( count - 1 ))
 }
 
 _hg()
@@ -155,10 +155,10 @@  shopt -s extglob
 	fi
     done
 
-    if [[ "$cur" == -* ]]; then
-	if [ "$(type -t "_hg_opt_$cmd")" = function ] && "_hg_opt_$cmd"; then
-	    return
-	fi
+    if [[ "$cur" == -* ]] ;  then
+    [[ "$( type -t "_hg_opt_$cmd" )" == "function" ]] \
+	    && "_hg_opt_$cmd" \
+	    && return
 
 	opts=$(_hg_cmd debugcomplete --options "$cmd")
 
@@ -179,23 +179,24 @@  shopt -s extglob
 	;;
     esac
 
-    if [[ -z "${cmd-}" ]] || [ $COMP_CWORD -eq $i ]; then
+    if
+	[[ -z "${cmd:-}" ]] \
+	|| (( COMP_CWORD == i ))
+    then
 	_hg_commands
 	return
     fi
 
     # try to generate completion candidates for whatever command the user typed
     local help
-    if _hg_command_specific; then
-	return
-    fi
+    _hg_command_specific \
+	&& return
 
     # canonicalize the command name and try again
-    help=$(_hg_cmd help "$cmd")
-    if [ $? -ne 0 ]; then
+    help=$(_hg_cmd help "$cmd") \
+	|| return
 	# Probably either the command doesn't exist or it's ambiguous
-	return
-    fi
+
     cmd=${help#hg }
     cmd=${cmd%%[$' \n']*}
     canonical=1
@@ -209,11 +210,14 @@  shopt -s extglob
 	return 0
     fi
 
-    if [ "$cmd" != status ] && [ "$prev" = -r ] || [ "$prev" == --rev ]; then
-	if [ $canonical = 1 ]; then
+    if
+	[[ "$cmd" != status \
+	&& "$prev" == @(-r|--rev) ]]
+    then
+	if (( canonical == 1 )) ;  then
 	    _hg_labels
 	    return 0
-	elif [[ status != "$cmd"* ]]; then
+	elif [[ "status" != "$cmd"* ]] ;  then
             _hg_labels
 	    return 0
 	else
@@ -268,7 +272,7 @@  shopt -s extglob
 	;;
 	clone)
 	    local count=$(_hg_count_non_option)
-	    if [ $count = 1 ]; then
+	    if (( count == 1 )) ;  then
 		_hg_paths
 	    fi
 	    _hg_repos
@@ -296,7 +300,7 @@  complete -o bashdefault -o default -F _h
 # bookmarks
 _hg_cmd_bookmarks()
 {
-    if [[ "$prev" = @(-d|--delete|-m|--rename) ]]; then
+    if [[ "$prev" == @(-d|--delete|-m|--rename) ]]; then
         _hg_bookmarks
         return
     fi
@@ -307,7 +311,10 @@  complete -o bashdefault -o default -F _h
 {
     local patches
     patches=$(_hg_cmd $1)
-    if [ $? -eq 0 ] && [ "$patches" ]; then
+    if
+	(( $? == 0 )) \
+	&& [[ -n "$patches" ]]
+    then
 	COMPREPLY=(${COMPREPLY[@]:-} $(compgen -W '$patches' -- "$cur"))
 	return 0
     fi
@@ -329,7 +336,7 @@  complete -o bashdefault -o default -F _h
 
 _hg_cmd_qpop()
 {
-    if [[ "$prev" = @(-n|--name) ]]; then
+    if [[ "$prev" == @(-n|--name) ]]; then
 	_hg_ext_mq_queues
 	return
     fi
@@ -338,7 +345,7 @@  complete -o bashdefault -o default -F _h
 
 _hg_cmd_qpush()
 {
-    if [[ "$prev" = @(-n|--name) ]]; then
+    if [[ "$prev" == @(-n|--name) ]]; then
 	_hg_ext_mq_queues
 	return
     fi
@@ -347,7 +354,7 @@  complete -o bashdefault -o default -F _h
 
 _hg_cmd_qgoto()
 {
-    if [[ "$prev" = @(-n|--name) ]]; then
+    if [[ "$prev" == @(-n|--name) ]]; then
 	_hg_ext_mq_queues
 	return
     fi
@@ -357,7 +364,7 @@  complete -o bashdefault -o default -F _h
 _hg_cmd_qdelete()
 {
     local qcmd=qunapplied
-    if [[ "$prev" = @(-r|--rev) ]]; then
+    if [[ "$prev" == @(-r|--rev) ]]; then
 	qcmd=qapplied
     fi
     _hg_ext_mq_patchlist $qcmd
@@ -365,7 +372,7 @@  complete -o bashdefault -o default -F _h
 
 _hg_cmd_qfinish()
 {
-    if [[ "$prev" = @(-a|--applied) ]]; then
+    if [[ "$prev" == @(-a|--applied) ]]; then
 	return
     fi
     _hg_ext_mq_patchlist qapplied
@@ -373,14 +380,14 @@  complete -o bashdefault -o default -F _h
 
 _hg_cmd_qsave()
 {
-    if [[ "$prev" = @(-n|--name) ]]; then
+    if [[ "$prev" == @(-n|--name) ]]; then
 	_hg_ext_mq_queues
 	return
     fi
 }
 
 _hg_cmd_rebase() {
-   if [[ "$prev" = @(-s|--source|-d|--dest|-b|--base|-r|--rev) ]]; then
+   if [[ "$prev" == @(-s|--source|-d|--dest|-b|--base|-r|--rev) ]]; then
        _hg_labels
        return
    fi
@@ -417,9 +424,8 @@  complete -o bashdefault -o default -F _h
 _hg_cmd_qclone()
 {
     local count=$(_hg_count_non_option)
-    if [ $count = 1 ]; then
-	_hg_paths
-    fi
+    (( count == 1 )) \
+	&& _hg_paths
     _hg_repos
 }
 
@@ -439,13 +445,13 @@  complete -o bashdefault -o default -F _h
     local prefix=''
 
     if [[ "$cur" == +* ]]; then
-	prefix=+
+	prefix="+"
     elif [[ "$cur" == -* ]]; then
-	prefix=-
+	prefix="-"
     fi
-    local ncur=${cur#[-+]}
+    local ncur="${cur#[-+]}"
 
-    if ! [ "$prefix" ]; then
+	if [[ -z "$prefix" ]] ;  then
 	_hg_ext_mq_patchlist qseries
 	return
     fi
@@ -498,7 +504,11 @@  complete -o bashdefault -o default -F _h
 	fi
     done
 
-    if [ -z "$subcmd" ] || [ $COMP_CWORD -eq $i ] || [ "$subcmd" = help ]; then
+	if
+	    [[ -z "$subcmd" \
+	    || "$subcmd" == "help" ]] \
+	    || (( COMP_CWORD == i ))
+	then
 	COMPREPLY=(${COMPREPLY[@]:-}
 		   $(compgen -W 'bad good help init next reset' -- "$cur"))
 	return