[PATCH 0/5] bashcompinit: several fixes

[PATCH 0/5] bashcompinit: several fixes

[Felipe Contreras]

Hi,

While trying to resolve the issue I reported before[1], I stumbled upon many
problems in the current code, here are a bunch of patches that should address
all the issues I found.

I used the following test script to verify my changes, and now all the tests
pass, just like in bash :)

---
#!/bin/sh

if [[ -n ${ZSH_VERSION-} ]]; then
    autoload -U +X bashcompinit && bashcompinit
fi

cwords[1]='first '
cwords[2]='second and space '
cwords[3]='third\\ quoted\\ space '
cwords[4]='fourth\\ quoted\\ space plus'

unescape()
{
	printf "%b" "$1"
}

prepare()
{
	local IFS=$'\n'
	echo "== prepare =="
	mkdir -p tmp
	for e in "${cwords[@]}"
	do
		u=$(unescape "$e")
		touch "tmp/$u"
	done
}

compare()
{
	test "$1" != "$2" && echo -e "error: '$1' != '$2'" && return -1 || return 0
}

_foo()
{
	local IFS=$'\n'
	COMPREPLY=( $(compgen -W "${cwords[*]}" -- "$2") )
}

test_3()
{
	local fail
	echo "== test 3 =="
	pushd "tmp"
	for e in "${cwords[@]}"
	do
		compare "$(compgen -o filenames -f -- ${e:0:2})" "$(unescape "$e")" || fail=true
	done
	popd
	test $fail || echo "OK"
}

test_2()
{
	local fail
	echo "== test 2 =="
	for e in "${cwords[@]}"
	do
		compare "$(compgen -F _foo -- ${e:0:2})" "$(unescape "$e")" || fail=true
	done
	test $fail || echo "OK"
}

test_1()
{
	local IFS=$'\n'
	local fail
	echo "== test 1 =="
	for e in "${cwords[@]}"
	do
		compare "$(compgen -W "${cwords[*]}" -- ${e:0:2})" "$(unescape "$e")" || fail=true
	done
	test $fail || echo "OK"
}

prepare
test_1
test_2
test_3
---

[1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24279

Felipe Contreras (5):
  bashcompinit: remove _compgen_opt_words
  bashcompinit: fix COMP_POINT
  bashcompinit: fix quoting code
  bashcompinit: simplify result matching code
  bashcompinit: improve compgen -F argument passing

 Completion/bashcompinit |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

-- 
1.7.8.3

[PATCH 1/5] bashcompinit: remove _compgen_opt_words

[Felipe Contreras]

There's already code for this[1]:

  support for the last, `word' option to compgen. Zsh's matching does a
  better job but if you need to, comment this in and use compadd -U

So either we rely on zsh's matching (compadd), or we enable this code
unconditionally so compgen works the same as in bash--and use compadd -U.

This kinds of reverts 2e25dfb[2], except that the original code:

 eval "results+=( $OPTARG )"

was wrong; it would fail if IFS is different. The new code has
'words=( ${~=1} )', which is corrent, and I transalted that to
'results+=( ${~=OPTARG} )', so there should not be any regression.

The original report for 2e25dfb[2] explained this is the desired
behavior.

 $ compgen -W 'abc abe ab a def' ab
 abc
 abe
 ab

If the code in [1] is enabled, I get _exactly_ the same result.

[1] (( $# )) && results=( "${(M)results[@]:#$1*}" )
[2] Rocky Bernstein: 29135 (plus tweaks): compgen -W in bash completion

Cc: Rocky Bernstein <rocky@gnu.org>
Cc: Peter Stephenson <pws@users.sourceforge.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Completion/bashcompinit |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index 63101a9..01cc38b 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -41,13 +41,6 @@ _bash_complete() {
   return ret
 }
 
-_compgen_opt_words() {
-  typeset -a words
-  words=( ${~=1} )
-  local find="$2"
-  results=(${(M)words[@]:#$find*})
-}
-
 compgen() {
   local opts prefix suffix job OPTARG OPTIND ret=1
   local -a name res results jids
@@ -141,7 +134,7 @@ compgen() {
         results+=( ${~OPTARG} )
 	unsetopt nullglob
       ;;
-      W) _compgen_opt_words "$OPTARG" "${@[-1]}" ;;
+      W) results+=( ${~=OPTARG} ) ;;
       C) results+=( $(eval $OPTARG) ) ;;
       P) prefix="$OPTARG" ;;
       S) suffix="$OPTARG" ;;
-- 
1.7.8.3

[PATCH 2/5] bashcompinit: fix COMP_POINT

[Felipe Contreras]

And add a comment explaining how 'words' changes behavior while we are
no it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Completion/bashcompinit |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index 01cc38b..2d6743e 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -8,7 +8,7 @@ _bash_complete() {
   local COMP_LINE="$words"
   local -A savejobstates savejobtexts
 
-  (( COMP_POINT = 1 + ${#${(j. .)words[1,CURRENT-1]}} + $#QIPREFIX + $#IPREFIX + $#PREFIX ))
+  (( COMP_POINT = 1 + ${#${(j. .)words[1,CURRENT]}} + $#QIPREFIX + $#IPREFIX + $#PREFIX ))
   (( COMP_CWORD = CURRENT - 1))
   COMP_WORDS=( $words )
   BASH_VERSINFO=( 2 05b 0 1 release )
@@ -46,6 +46,7 @@ compgen() {
   local -a name res results jids
   local -A shortopts
 
+  # words changes behavior: words[1] -> words[0]
   emulate -L sh
   setopt kshglob noshglob braceexpand nokshautoload
 
-- 
1.7.8.3

[PATCH 3/5] bashcompinit: fix quoting code

[Felipe Contreras]

There's a mismatch between to zsh and bash's quoting:

---
if [[ -n ${ZSH_VERSION-} ]]; then
   autoload -U +X bashcompinit && bashcompinit
fi

_foo()
{
   local cur="${COMP_WORDS[COMP_CWORD]}"
   local IFS=$'\n'
   w[0]='first '
   w[1]='second and space '
   w[2]='third\\ quoted\\ space '
   COMPREPLY=( $(compgen -W "${w[*]}" -- $cur) )
}
complete -o nospace -F _foo foo
---

The result in bash (by typing 'foo f<tab>', 'foo s<tab>', and so on) is:

 'foo first '
 'foo second and space '
 'foo third\ quoted\ space '

In zsh, the results are quite different:

 'foo first\ '
 'foo second\ and\ space\ '
 'foo third\ quoted\ space\ '

The following patch fixes that.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Completion/bashcompinit |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index 2d6743e..4b2084a 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -24,9 +24,9 @@ _bash_complete() {
     if [[ ${argv[${argv[(I)filenames]:-0}-1]} = -o ]]; then
       compset -P '*/' && matches=( ${matches##*/} )
       compset -S '/*' && matches=( ${matches%%/*} )
-      compadd -f "${suf[@]}" -a matches && ret=0
+      compadd -Q -f "${suf[@]}" -a matches && ret=0
     else
-      compadd "${suf[@]}" -a matches && ret=0
+      compadd -Q "${suf[@]}" -a matches && ret=0
     fi
   fi
 
@@ -135,7 +135,7 @@ compgen() {
         results+=( ${~OPTARG} )
 	unsetopt nullglob
       ;;
-      W) results+=( ${~=OPTARG} ) ;;
+      W) results+=( ${(Q)~=OPTARG} ) ;;
       C) results+=( $(eval $OPTARG) ) ;;
       P) prefix="$OPTARG" ;;
       S) suffix="$OPTARG" ;;
@@ -154,7 +154,7 @@ compgen() {
   #shift $(( OPTIND - 1 ))
   #(( $# )) && results=( "${(M)results[@]:#$1*}" )
 
-  print -l -- "$prefix${^results[@]}$suffix"
+  print -l -r -- "$prefix${^results[@]}$suffix"
 }
 
 complete() {
-- 
1.7.8.3

[PATCH 4/5] bashcompinit: simplify result matching code

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Completion/bashcompinit |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index 4b2084a..9c561c5 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -151,8 +151,7 @@ compgen() {
   
   # support for the last, `word' option to compgen. Zsh's matching does a
   # better job but if you need to, comment this in and use compadd -U
-  #shift $(( OPTIND - 1 ))
-  #(( $# )) && results=( "${(M)results[@]:#$1*}" )
+  # (( $# >= OPTIND)) && results=( "${(M)results[@]:#${@[-1]}*}" )
 
   print -l -r -- "$prefix${^results[@]}$suffix"
 }
-- 
1.7.8.3

[PATCH 5/5] bashcompinit: improve compgen -F argument passing

[Felipe Contreras]

Otherwise something like

 % compgen -F _foo -- fi

Would not pass the arguments ('fi') to the function, like bash does.

I looked that the source code of bash, and they pass only the first
extra argument as argv $2. Let's do the same.

This is not a big deal, as 'compgen -F' would match the results, instead
of 'compgen -W' like in bash, but the end result is the same. Same as if
zsh's matching (compadd) is used instead.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Completion/bashcompinit |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index 9c561c5..31f64f8 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -18,7 +18,7 @@ _bash_complete() {
 
   [[ ${argv[${argv[(I)nospace]:-0}-1]} = -o ]] && suf=( -S '' )
 
-  matches=( ${(f)"$(compgen $@)"} )
+  matches=( ${(f)"$(compgen $@ ${words[CURRENT]})"} )
 
   if [[ -n $matches ]]; then
     if [[ ${argv[${argv[(I)filenames]:-0}-1]} = -o ]]; then
@@ -121,12 +121,13 @@ compgen() {
       ;;
       F)
         COMPREPLY=()
+        local -a args
+        args=( "${words[0]}" "${@[-1]}" "${words[CURRENT-2]}" )
         (){
-          set -- "${words[0]}" "${words[CURRENT-1]}" "${words[CURRENT-2]}"
           # There may be more things we need to add to this typeset to
           # protect bash functions from compsys special variable names
           typeset -h words
-          $OPTARG "$@"
+          $OPTARG "${args[@]}"
         }
 	results+=( "${COMPREPLY[@]}" )
       ;;
-- 
1.7.8.3

Re: [PATCH 5/5] bashcompinit: improve compgen -F argument passing

[Felipe Contreras]

On Sat, Jan 28, 2012 at 6:55 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:

> --- a/Completion/bashcompinit
> +++ b/Completion/bashcompinit
> @@ -18,7 +18,7 @@ _bash_complete() {
>
>   [[ ${argv[${argv[(I)nospace]:-0}-1]} = -o ]] && suf=( -S '' )
>
> -  matches=( ${(f)"$(compgen $@)"} )
> +  matches=( ${(f)"$(compgen $@ ${words[CURRENT]})"} )

Err, should be:
matches=( ${(f)"$(compgen $@ -- ${words[CURRENT]})"} )

-- 
Felipe Contreras

Re: [PATCH 0/5] bashcompinit: several fixes

[Peter Stephenson]

On Sat, 28 Jan 2012 18:55:46 +0200
Felipe Contreras <felipe.contreras@gmail.com> wrote:
> While trying to resolve the issue I reported before[1], I stumbled upon many
> problems in the current code, here are a bunch of patches that should address
> all the issues I found.
> 
> I used the following test script to verify my changes, and now all the tests
> pass, just like in bash :)

Thanks, I'm not in a position to test this seriously so I've simply
applied all the changes; see if the committed code does what you expect.
I'd be happy to add a file to the test system.

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/