Re: zsh 4.2.1-test-A

Re: zsh 4.2.1-test-A

[Clint Adams]

> If this version doesn't fix any bug you've reported, please report it
> again.

Here's some that still seem to affect 4.2.1-test-A.

#134474 - tsetcap is based on short term instead of SINGLE_LINE_ZLE
#258431 - doesn't complete .iso files for cdrecord.
#261775 - remote rsync completion needs escaping a la scp completion
#256789 - brace completion commas aren't escaped (17791, 17913, 17926)

Re: zsh 4.2.1-test-A

[Oliver Kiddle]

Clint wrote:
> Here's some that still seem to affect 4.2.1-test-A.
> 
> #258431 - doesn't complete .iso files for cdrecord.

I can't reproduce this with 4.2.1-test-A so I suspect there is something
that fixed it which isn't in Debian's 4.2.0. Note that we probably
shouldn't add -g \*.iso to _files. There are many other file extensions
(and filesystems besides iso9660) relevant to CD (and DVD) images.

> #261775 - remote rsync completion needs escaping a la scp completion

I ripped the remote file completion out of _ssh and stuck it in. Patch
is below. Hopefully that doesn't break anything. We probably ought to
factor _remote_files out into a separate function given that all of
_ssh, _rsh and _rsync contain near identical code.

> #256789 - brace completion commas aren't escaped (17791, 17913, 17926)

I wouldn't want to attempt to fix that just before a release. Would be
too easy to break something.

Oliver

Index: Completion/Unix/Command/_rsync
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_rsync,v
retrieving revision 1.9
diff -u -r1.9 _rsync
--- Completion/Unix/Command/_rsync	14 Apr 2004 14:54:16 -0000	1.9
+++ Completion/Unix/Command/_rsync	7 Aug 2004 13:18:14 -0000
@@ -1,12 +1,9 @@
 #compdef rsync
 
 _rsync_remote_files() {
-local suf tag=accounts
+local expl remfiles remdispf remdispd remmodules suf ret=1 tag=accounts
 
-if [[ -prefix *::*/ ]]; then
-  local remfiles remdispf remdispd
-
-  compset -P '*::*/'
+if compset -P '*::*/'; then
 
   remfiles=(${(f)"$(_call_program files rsync ${words[CURRENT]%/*}/)"})
 
@@ -19,10 +16,7 @@
   _wanted files expl 'remote file or directory' \
       compadd -S/ -d remdispd ${remdispd##* }
 
-elif [[ -prefix 1 *:: ]]; then
-  local remfiles remmodules
-
-  compset -P 1 '*::'
+elif compset -P 1 '*::'; then
 
   remfiles=(${(f)"$(_call_program files rsync ${words[CURRENT]%::*}::)"})
 
@@ -30,32 +24,33 @@
 
   _describe "remote modules" remmodules -S/
 
-elif [[ -prefix 1 *: ]]; then
-  local remfiles remdispf remdispd slash
+elif compset -P 1 '*:'; then
 
-  compset -P 1 '*:'
-
-  if zstyle -T ":completion:${curcontext}:" remote-access; then
-    slash=/
-    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1FL "${${${words[CURRENT
-]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null")"})
+  if zstyle -T ":completion:${curcontext}:files" remote-access; then
+    remfiles=(${(M)${(f)"$(_call_program files ssh -a -x ${IPREFIX%:} ls -d1FL "${(Q)PREFIX%%[^./][^/]#}\*" 2>/dev/null)"}%%[^/]#(|/)})
+    compset -P '*/'
+    compset -S '/*' || suf='remote file'
 
     remdispf=(${remfiles:#*/})
     remdispd=(${(M)remfiles:#*/})
 
-    _wanted files expl 'remote file or directory' \
-        compadd -d remdispf ${${remfiles:#*/}/[*=@|](#e)/}
-
-    _wanted files expl 'remote file or directory' \
-        compadd -S/ -d remdispd ${${(M)remfiles:#*/}/${slash}(#e)/}
+    _tags files
+    while _tags; do
+      while _next_label files expl ${suf:-remote directory}; do
+        [[ -n $suf ]] && compadd "$@" "$expl[@]" -d remdispf \
+	    ${(q)remdispf%[*=@|]} && ret=0 
+	compadd ${suf:+-S/} "$@" "$expl[@]" -d remdispd \
+	    ${(q)remdispd%/} && ret=0
+      done
+      (( ret )) || return 0
+    done
   else
-    _message -e remote-files 'remote files'
+    _message -e remote-files 'remote file'
   fi
 
-elif [[ -prefix 1 *@ ]]; then
+elif compset -P 1 '*@'; then
   local user=${PREFIX%%@*}
 
-  compset -P 1 '*@'
   compset -S ':*' || suf=":"
 
   _wanted -C user-at hosts expl "host for $user" \

Re: zsh 4.2.1-test-A

[Clint Adams]

> I can't reproduce this with 4.2.1-test-A so I suspect there is something
> that fixed it which isn't in Debian's 4.2.0. Note that we probably
> shouldn't add -g \*.iso to _files. There are many other file extensions
> (and filesystems besides iso9660) relevant to CD (and DVD) images.

I just double-checked to make sure I had really tested this with
4.2.1-test-A, and I still can't complete any files at all; only options
are offered.

This happens with zsh -f followed by compinit.

default tag-order (was Re: zsh 4.2.1-test-A)

[Oliver Kiddle]

Clint Adams wrote:
> I just double-checked to make sure I had really tested this with
> 4.2.1-test-A, and I still can't complete any files at all; only options
> are offered.
> 
> This happens with zsh -f followed by compinit.

One of my styles made a difference. I can reproduce it with zsh -f.

The problem is with the default tag-order defined in _tags. The relevant
bit of code is as follows:
     zstyle -a ":completion:${curcontext}:" tag-order order ||
         order=('(|*-)argument-* (|*-)option[-+]* values' options)

There are a few different things we could do here. I can't work out why
"values" needs to be in there. Does _arguments ever add stuff with a
values tag or does _values ever add stuff with an options tag? Can we
remove the values tag from there without breaking anything? It has been
there since the very first version of _tags.

One safe option is to insert `(( ! ${@[(I)options]} )) ||'.
That checks if there is an options tag before applying the tag-order.

Any other thoughts?

Default tag-orders really need thinking about in general. It'd be nice
to be able to specify them from completion functions themself. This code
above should really be in _arguments. That would need care, though. It
is often the case that tag-orders are better not applied for _approximate
or _correct for example.

Oliver

Re: default tag-order (was Re: zsh 4.2.1-test-A)

[Bart Schaefer]

On Sun, 8 Aug 2004, Oliver Kiddle wrote:

> The problem is with the default tag-order defined in _tags. The relevant
> bit of code is as follows:
>      zstyle -a ":completion:${curcontext}:" tag-order order ||
>          order=('(|*-)argument-* (|*-)option[-+]* values' options)
>
> There are a few different things we could do here. I can't work out why
> "values" needs to be in there.
> 
> One safe option is to insert `(( ! ${@[(I)options]} )) ||'.
> That checks if there is an options tag before applying the tag-order.

I'm confused by this suggestion.  If there's no options tag, the tag-order 
doesn't make any difference, because it's in the second group of tags.  
And in the case of cdrecord, there _is_ an options tag.  How would that
proposed change help?

> Any other thoughts?
> 
> Default tag-orders really need thinking about in general. It'd be nice
> to be able to specify them from completion functions themself.

Why is that not possible?  For example, several completion functions set 
the cache-policy style if it's not already set.

Re: default tag-order (was Re: zsh 4.2.1-test-A)

[Bart Schaefer]

On Sun, 8 Aug 2004, Bart Schaefer wrote:

> On Sun, 8 Aug 2004, Oliver Kiddle wrote:
> 
> > The problem is with the default tag-order defined in _tags. The relevant
> > bit of code is as follows:
> >      zstyle -a ":completion:${curcontext}:" tag-order order ||
> >          order=('(|*-)argument-* (|*-)option[-+]* values' options)
> >
> > One safe option is to insert `(( ! ${@[(I)options]} )) ||'.
> > That checks if there is an options tag before applying the tag-order.
> 
> I'm confused by this suggestion.  If there's no options tag, the tag-order 
> doesn't make any difference, because it's in the second group of tags.
> And in the case of cdrecord, there _is_ an options tag.

OK, I worked this out now.  The problem is that there's no options tag *in 
the argument-rest subcontext* but there are both values and files tags, so 
the presence of values in this tag hides the files.  Sorry to be dense.

The canonical examples of _values are _dd and _chmod, neither of which 
allows ambiguity in whether an argument is a value or a filename (dd takes 
only values, and chmod requires exactly one value before any filename).  
So I suspect this simply never came up before.

Re: default tag-order (was Re: zsh 4.2.1-test-A)

[Oliver Kiddle]

You wrote:

> I'm confused by this suggestion.  If there's no options tag, the tag-order 
> doesn't make any difference, because it's in the second group of tags.  
> And in the case of cdrecord, there _is_ an options tag.  How would that
> proposed change help?

Well the options aren't completed in the same tag loop so it doesn't
affect _cdrecord. _arguments just doesn't get used that way. Checking
for the other tags ((|*-)argument-* (|*-)option[-+]* or values) doesn't
particularly help given that there is only a problem when they (or
values to be precise) are there.

> > Default tag-orders really need thinking about in general. It'd be nice
> > to be able to specify them from completion functions themself.
> 
> Why is that not possible?  For example, several completion functions set 
> the cache-policy style if it's not already set.

Doing it that way is ugly. And not just because the function has to
lookup the style before setting it. It just seems wrong to me for
completion functions to set styles. There's quite a lot of places where
default tag orders would be useful so it would potentially clutter the
style list. For all other styles, we don't set the style to achieve a
default so why should we make an exception for tag-order. It is also the
case that a certain zstyle context will apply to more than one tag loop
so you can't always give them different defaults.

I think the cache-policy stuff would be far better handled by guarding
the policy functions with (( $+functions[_foo_caching_policy] )) ||
I don't see why they should have a style when anything else you want to
override is just done by having a replacement function. Judging by
20211, the cache stuff isn't used much anyway.

Oliver

Re: default tag-order (was Re: zsh 4.2.1-test-A)

[Oliver Kiddle]

I wrote:
> 
> The problem is with the default tag-order defined in _tags. The relevant
> bit of code is as follows:
>      zstyle -a ":completion:${curcontext}:" tag-order order ||
>          order=('(|*-)argument-* (|*-)option[-+]* values' options)

Actually, this causes problems even with something like:

  _alternative \
    'options:option:(one two three)' \
    'foos:foo:(aa bbb cc)'

So it is probably worth checking for (|*-)argument-* (|*-)option[-+]* tags.

Oliver

Re: default tag-order (was Re: zsh 4.2.1-test-A)

[Oliver Kiddle]

I wrote:
> 
> One safe option is to insert `(( ! ${@[(I)options]} )) ||'.
> That checks if there is an options tag before applying the tag-order.

Unless anyone objects, I'll commit this as a temporary solution. Actual
patch is below. It is the one solution which, as far as I know, doesn't
break anything that isn't already broken (which is what I meant by
"safe" above). So at least _cdrecord can be fixed and this needn't hold
up 4.2.1.

As a longer term fix, that default tag-order has to go completely. I
just need to find all those things that removing it would break and find
a better fix.

Oliver

Index: Completion/Base/Core/_tags
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_tags,v
retrieving revision 1.3
diff -u -r1.3 _tags
--- Completion/Base/Core/_tags	29 May 2001 17:54:08 -0000	1.3
+++ Completion/Base/Core/_tags	8 Aug 2004 13:52:56 -0000
@@ -41,6 +41,7 @@
     "$_sort_tags" "$@"
   else
     zstyle -a ":completion:${curcontext}:" tag-order order ||
+        (( ! ${@[(I)options]} )) ||
         order=('(|*-)argument-* (|*-)option[-+]* values' options)
 
     for tag in $order; do