matcher-list doesn't work with some completers?

matcher-list doesn't work with some completers?

[Mikael Magnusson]

Hi,

mkdir 'a directory'
zsh -f
autoload -Uz compinit; compinit -D
zstyle ':completion:*' matcher-list 'm:{a-zA-Z}={A-Za-z}' +'l:|=* r:|=*'
ls dir<tab> #works
du dir<tab> #nothing
zstyle ':completion:*' matcher-list 'm:{a-zA-Z}={A-Za-z} l:|=* r:|=*'
ls dir<tab> #works
du dir<tab> #works

Obviously the latter style is useless in practice, but why doesn't the
first one work always? The _mkdir completer has the same issue as _du,
but _rar seems to work. I looked at them and I can't figure out what
the difference is. This is not a new issue, but I'm not sure when it
started happening. The _du in 4.3.6 works, but the _mkdir does not.
Copying the older _du doesn't seem to make any difference so it is
probably Something Else[tm].

-- 
Mikael Magnusson

Re: matcher-list doesn't work with some completers?

[Bart Schaefer]

On Jan 6, 11:14am, Mikael Magnusson wrote:
} Subject: matcher-list doesn't work with some completers?
} 
} mkdir 'a directory'
} zsh -f
} autoload -Uz compinit; compinit -D
} zstyle ':completion:*' matcher-list 'm:{a-zA-Z}={A-Za-z}' +'l:|=* r:|=*'
} ls dir<tab> #works
} du dir<tab> #nothing

This is a bug in _du, it's returning a 0 status without ever adding any
matches, which causes _dispatch in turn to report success to _complete
which then skips running the matcher-list.

This in turn is because _du checks for a state transition to handle the
--time and --time-style options, and that case statement masks the
return value from _arguments.

I don't know for sure whether the fix is to save the return value from
_arguments and then return that in a (*) branch of the case statement,
or to return immediately if _arguments fails and not bother checking
the state transition at all.  _mkdir does the former, so ...

Someone with copious free time should do a sweep of the completion
functions and make sure they're correctly propagating return values
from _arguments, _wanted, etc.


Index: Completion/Unix/Command/_du
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Completion/Unix/Command/_du,v
retrieving revision 1.4
diff -c -r1.4 _du
--- _du	21 Dec 2010 16:41:15 -0000	1.4
+++ _du	6 Jan 2011 16:38:35 -0000
@@ -1,6 +1,8 @@
 #compdef du
 
 if _pick_variant gnu=Free\ Soft unix --version /dummy/no-such-file; then
+  local ret=1
+
   _arguments -s \
     '(-a --all -s --summarize)'{-a,--all}'[write counts for all files]' \
     '--apparent-size[print apparent sizes rather than disc usage]' \
@@ -27,22 +29,24 @@
     '--time=-[show time of last modification of any file in the
directory]:property:->time' \
     '(* -)--help[display help information]' \
     '(* -)--version[display version information]' \
-    '*:file:_files'
+    '*:file:_files' && ret=0
 
   case $state in
     (time)
       local -a property
       property=(atime access use ctime status)
-      _wanted property expl property compadd -a property
+      _wanted property expl property compadd -a property && ret=0
       ;;
     (timestyle)
       local -a style desc
       style=(full-iso long-iso iso +)
       desc=('full-iso' 'long-iso' 'iso' '+FORMAT like `date'\''')
-      _wanted -V style expl style compadd -d desc -a style
+      _wanted -V style expl style compadd -d desc -a style && ret=0
       ;;
   esac
 
+  return ret
+
 else
   # based on $OSTYPE = solaris2.8
   local xdev='[skip directories on different filesystems]'


-- 

Re: matcher-list doesn't work with some completers?

[Bart Schaefer]

On Jan 6,  8:41am, Bart Schaefer wrote:
>
> > ls dir<tab> #works
> > du dir<tab> #nothing
> 
> This is a bug in _du, it's returning a 0 status without ever adding any
> matches, which causes _dispatch in turn to report success to _complete
> which then skips running the matcher-list.
> 
> This in turn is because _du checks for a state transition to handle the
> --time and --time-style options, and that case statement masks the
> return value from _arguments.

Incidentally, the way I approach debugging this stuff (especially when
there's one working and one non-working example as above) is to use
the _complete-debug binding (^X?) to get a temp file xtrace dump of
each of the different cases, and then diff them to look for places
where the flow of control may be going awry.

In the examples above, _complete went on to line 64 after _ls was
finished, but exited at line 63 for _du, which made it obvious that
_du was returning 0 even though it hadn't done anything.

-- 

Re: matcher-list doesn't work with some completers?

[Mikael Magnusson]

On 6 January 2011 18:24, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 6,  8:41am, Bart Schaefer wrote:
>>
>> > ls dir<tab> #works
>> > du dir<tab> #nothing
>>
>> This is a bug in _du, it's returning a 0 status without ever adding any
>> matches, which causes _dispatch in turn to report success to _complete
>> which then skips running the matcher-list.
>>
>> This in turn is because _du checks for a state transition to handle the
>> --time and --time-style options, and that case statement masks the
>> return value from _arguments.
>
> Incidentally, the way I approach debugging this stuff (especially when
> there's one working and one non-working example as above) is to use
> the _complete-debug binding (^X?) to get a temp file xtrace dump of
> each of the different cases, and then diff them to look for places
> where the flow of control may be going awry.
>
> In the examples above, _complete went on to line 64 after _ls was
> finished, but exited at line 63 for _du, which made it obvious that
> _du was returning 0 even though it hadn't done anything.

Thanks for the fix and the tip, I see also that I was probably the one
to break _du since I added those options, but I had no idea the return
value was even used by the completion system.

I guess I'll have a go at fixing _mkdir then...

-- 
Mikael Magnusson

Re: matcher-list doesn't work with some completers?

[Mikael Magnusson]

On 6 January 2011 19:42, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 6 January 2011 18:24, Bart Schaefer <schaefer@brasslantern.com> wrote:
>> On Jan 6,  8:41am, Bart Schaefer wrote:
>>>
>>> > ls dir<tab> #works
>>> > du dir<tab> #nothing
>>>
>>> This is a bug in _du, it's returning a 0 status without ever adding any
>>> matches, which causes _dispatch in turn to report success to _complete
>>> which then skips running the matcher-list.
>>>
>>> This in turn is because _du checks for a state transition to handle the
>>> --time and --time-style options, and that case statement masks the
>>> return value from _arguments.
>
> I guess I'll have a go at fixing _mkdir then...

Well, this is not my day. First I had _du in site-functions so nothing
I did fixed it (I figured this out before sending the previous mail).
Then as I was trying to fix _mkdir, I just did a cp **/_mkdir
$fpath[3] to save some time. BUT of course I had -test-3 in that shell
and the new shell had -dev-1 so that copy didn't have any effect. Heh.

Anyway here is a patch for _mkdir:
http://git.mika.l3ib.org/?p=zsh-cvs.git;a=patch;h=80fbf4aa928361bb9f60819c7b8d1922f1732487

Subject: [PATCH] _mkdir: don't set ret=0 when _wanted fails to find any matches

---
 Completion/Unix/Command/_mkdir |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Completion/Unix/Command/_mkdir b/Completion/Unix/Command/_mkdir
index 927b9df..b5f7519 100644
--- a/Completion/Unix/Command/_mkdir
+++ b/Completion/Unix/Command/_mkdir
@@ -60,9 +60,8 @@ case "$state" in
     if (( $ret )) && [[ ! -prefix - ]] || \
       [[ $variant == zsh && ${#${${words[2,-1]}:#-*}} -gt 0 ]]; then
       _wanted directories expl \
-	'parent directory (alternatively specify name of directory)' \
-	_path_files -/ || _message 'name of directory'
-      ret=0
+        'parent directory (alternatively specify name of directory)' \
+        _path_files -/ && ret=0 || _message 'name of directory'
     fi
     ;;
 esac
-- 
1.7.3


-- 
Mikael Magnusson