Quoting problems with _zip (unzip) completer

Quoting problems with _zip (unzip) completer

[Mikael Magnusson]

Completing contents of zip files with metachars doesn't work,
completing actual files in a zip file that you have renamed also
doesn't work properly. These issues seem to be separate, but I could
be wrong. While hacking around in _zip it seems like I "fixed" at
least the first thing.

The most obvious example is:
% touch a b c
% zip test.zip a b c
% unzip test.zip <tab completes fine>a
% mv test.zip test\[.zip
% unzip test\[.zip <tab>
_zip:117: bad pattern: test[.zip(|.zip|.ZIP)
_zip:117: bad pattern: test[.zip(|.zip|.ZIP)
_zip:117: bad pattern: test[.zip(|.zip|.ZIP)

naming the file test[].zip which is a valid pattern simply doesn't
give any matches. Naming the files inside the zip file and calling it
test.zip fails in various ways, sometimes it just completes the last
file without giving a list, even though set -x shows the full file
list being passed to the completion system, and typing a partial name
+ tab completes that file. Mysterious.

The change I did that seems to solve the actual .zip file name issue is:

@@ -114,7 +114,7 @@ case $state in
     if [[ $service = zip ]] && (( ! ${+opt_args[-d]} )); then
       _wanted files expl zfile _files -g
'^(#i)*.(zip|xpi|[ejw]ar)(-.)' && return
     else
-      zipfile=( $~line[1](|.zip|.ZIP) )
+      zipfile=( $line[1](|.zip|.ZIP) )
       [[ -z $zipfile[1] ]] && return 1
       if [[ $zipfile[1] !=  $_zip_cache_list ]]; then
 	_zip_cache_name="$zipfile[1]"

i.e. remove the ~. I didn't test if this breaks completion if you
didn't type the ".zip" part of the filename. I've fiddled around
adding (q) and quotes everywhere but it doesn't seem to improve
matters any.

-- 
Mikael Magnusson

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

Mikael Magnusson wrote:
> % unzip test\[.zip <tab>
> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> 
>..
> @@ -114,7 +114,7 @@ case $state in
>      if [[ $service = zip ]] && (( ! ${+opt_args[-d]} )); then
>        _wanted files expl zfile _files -g
> '^(#i)*.(zip|xpi|[ejw]ar)(-.)' && return
>      else
> -      zipfile=( $~line[1](|.zip|.ZIP) )
> +      zipfile=( $line[1](|.zip|.ZIP) )
>        [[ -z $zipfile[1] ]] && return 1
>        if [[ $zipfile[1] !=  $_zip_cache_list ]]; then
>  	_zip_cache_name="$zipfile[1]"

The trouble is this stops you using filenames with a ~, among other
things.  This suggests the value of "line" is a bit inconsistent.  Sure
enough if I use ~/tmp/zip/tmp\[.zip $line[1] comes back (using print -r) as

  ~/tmp/zip/test[.zip

which is wrong---either the ~ needs to be expanded, or the [ needs to be
quoted.  So this needs tracking internally, unfortunately.  If it hits
the internal completion quoting system we're probably stuck---I spent
weeks looking at that a couple of years ago and got virtually nowhere.
However, it may not be that bad in this case.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


'member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom'

Re: Quoting problems with _zip (unzip) completer

[Mikael Magnusson]

2009/8/4 Peter Stephenson <pws@csr.com>:
> Mikael Magnusson wrote:
>> % unzip test\[.zip <tab>
>> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>>
>>..
>> @@ -114,7 +114,7 @@ case $state in
>>      if [[ $service = zip ]] && (( ! ${+opt_args[-d]} )); then
>>        _wanted files expl zfile _files -g
>> '^(#i)*.(zip|xpi|[ejw]ar)(-.)' && return
>>      else
>> -      zipfile=( $~line[1](|.zip|.ZIP) )
>> +      zipfile=( $line[1](|.zip|.ZIP) )
>>        [[ -z $zipfile[1] ]] && return 1
>>        if [[ $zipfile[1] !=  $_zip_cache_list ]]; then
>>       _zip_cache_name="$zipfile[1]"
>
> The trouble is this stops you using filenames with a ~, among other
> things.  This suggests the value of "line" is a bit inconsistent.  Sure
> enough if I use ~/tmp/zip/tmp\[.zip $line[1] comes back (using print -r) as
>
>  ~/tmp/zip/test[.zip
>
> which is wrong---either the ~ needs to be expanded, or the [ needs to be
> quoted.  So this needs tracking internally, unfortunately.  If it hits
> the internal completion quoting system we're probably stuck---I spent
> weeks looking at that a couple of years ago and got virtually nowhere.
> However, it may not be that bad in this case.

I would argue it is better without the $~; I can replace the ~ with
$PWD on the commandline, but I can't rename the file to not have
brackets without actually renaming it.

I came up with a reproducible test case for the other issue:
% mkdir '[test] a name with spaces'
% touch '[test] a name with spaces'/{'a file with spaces','another
file with spaces',a file with a unique name','afilewithoutspaces'}
% zip -r test.zip '[test] a name with spaces'
% unzip test.zip <tab> # -> gives "\[test\]\ a\ name\ with\ spaces "
with the space after
% unzip test.zip \[test\]\ a\ name\ with\ spaces<tab> # -> just adds
the space back
% unzip test.zip \[test\]\ a\ name\ with\ spaces/<tab>
# -> \[test\]\ a\ name\ with\ spaces/afilewithoutspaces
% unzip test.zip \[test\]\ a\ name\ with\ spaces/a<tab>
# same
% unzip test.zip \[test\]\ a\ name\ with\ spaces/an<tab>
# -> "\[test\]\ a\ name\ with\ spaces/another\ file\ with\ spaces "
# (as a unique match)
% unzip test.zip \[test\]\ a\ name\ with\ spaces/a\ file<tab><tab><tab><tab>
# just beeps, ie no matches

calling the directory 'normaldirectoryname' causes the directory to
get a slash appended at the first step, but the files inside then
behave exactly the same as above.

According to _complete_debug the whole array of filenames gets passed
down along the chain, it should be available here for some time:
http://mika.l3ib.org/zsh-zip-complete-debug.txt

-- 
Mikael Magnusson

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

On Tue, 04 Aug 2009 09:50:59 +0100
Peter Stephenson <pws@csr.com> wrote:
> Mikael Magnusson wrote:
> > % unzip test\[.zip <tab>
> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> 
> ... the value of "line" [in _zip] is a bit inconsistent.  Sure
> enough if I use ~/tmp/zip/tmp\[.zip $line[1] comes back (using print -r) as
> 
>   ~/tmp/zip/test[.zip
> 
> which is wrong---either the ~ needs to be expanded, or the [ needs to be
> quoted.  So this needs tracking internally, unfortunately.  If it hits
> the internal completion quoting system we're probably stuck---I spent
> weeks looking at that a couple of years ago and got virtually nowhere.
> However, it may not be that bad in this case.

It's inside comparguments, but luckily in only one place.  This removes
the internal unquoting so the form above works.

This could easily have knock-on effects in other callers of _arguments,
however they should be fixable by simple local changes (and at least
one case, the one above, works without updating, so others probably do,
too), while with the old code there were unfixable cases, so I think we
just need to identify them and change them as they come up.

So please watch out for any anomalous _arguments quoting behaviour.

Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.115
diff -u -r1.115 computil.c
--- Src/Zle/computil.c	5 Aug 2009 10:14:54 -0000	1.115
+++ Src/Zle/computil.c	17 Aug 2009 20:52:20 -0000
@@ -1866,9 +1866,12 @@
     Caopt ptr, wasopt = NULL, dopt;
     struct castate state;
     char *line, *oline, *pe, **argxor = NULL;
-    int cur, doff, argend, arglast, ne;
+    int cur, doff, argend, arglast;
     Patprog endpat = NULL, napat = NULL;
     LinkList sopts = NULL;
+#if 0
+    int ne;
+#endif
 
     /* Free old state. */
 
@@ -1927,13 +1930,24 @@
 	dopt = NULL;
 	doff = state.singles = arglast = 0;
 
-        /* remove quotes */
         oline = line;
+#if 0
+        /*
+	 * remove quotes.
+	 * This is commented out:  it doesn't allow you to discriminate
+	 * between command line values that can be expanded and those
+	 * that can't, and in some cases this generates inconsistency;
+	 * for example, ~/foo\[bar unqotes to ~/foo[bar which doesn't
+	 * work either way---it's wrong if the ~ is quoted, and
+	 * wrong if the [ isn't quoted..  So it's now up to the caller to
+	 * unquote.
+	 */
         line = dupstring(line);
         ne = noerrs;
         noerrs = 2;
         parse_subst_string(line);
         noerrs = ne;
+#endif
         remnulargs(line);
         untokenize(line);
 



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

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

On Mon, 17 Aug 2009 21:58:19 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> So please watch out for any anomalous _arguments quoting behaviour.

Found one in _rm... but it's not a knock-on effect of this patch, it's
another thing that was already broken.

  rm stuff <TAB>

is supposed to exclude "stuff" from being completed again by using the
-F option to _files by together with the line from _arguments:

    ignored=(${line//(#m)[\[\]()\\*?#<>~\^]/\\$MATCH})
    _files -F ignored && ret=0

However, the code for "_files -F <array>" was
broken:  instead of appending the contents of <array> after the -F to
the contents of the existing array, it appended the name <array>.

After fixing this, I'm gratified to find that in a directory containing
files called "normal" and "foo[bar", it's possible to have both files
ignored owing to the previous fix (with just this fix, and not the
previous one, it only works for "normal").

(I'm not *quite* sure how this works, actually.  The extra quoting shown
in the code above protects pattern characters, since we're actually
passing a pattern, not a file name, so 'foo\[bar' from line becomes
'foo\\\[bar'.  Unquoting the pattern characters gives 'foo\[bar' back as
a literal match.  I don't understand why this matches the file...  I'm
going to pretend I didn't notice.)

I'm a bit surprised nobody noticed this wasn't working.  If anyone finds
this has a negative rather an a positive effect on ignoring files in
"rm" completion, please try to reproduce it starting from "zsh -f".

Index: Completion/Unix/Type/_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_files,v
retrieving revision 1.10
diff -u -r1.10 _files
--- Completion/Unix/Type/_files	21 Aug 2008 15:53:00 -0000	1.10
+++ Completion/Unix/Type/_files	17 Aug 2009 21:42:03 -0000
@@ -1,7 +1,7 @@
 #compdef -redirect-,-default-,-default-
 
 local opts tmp glob pat pats expl tag i def descr end ign ret=1 match tried
-local type sdef
+local type sdef ignvars ignvar
 
 zparseopts -a opts \
     '/=tmp' 'f=tmp' 'g+:-=tmp' q n 1 2 P: S: r: R: W: X+: M+: F: J+: V+:
@@ -18,14 +18,18 @@
 fi
 tmp=$opts[(I)-F]
 if (( tmp )); then
-  ign=( $=opts[tmp+1] )
-  if [[ $ign = _comp_ignore ]]; then
+  ignvars=($=opts[tmp+1])
+  if [[ $ignvars = _comp_ignore ]]; then
     ign=( $_comp_ignore )
   else
+    ign=()
+    for ignvar in $ignvars; do
+      ign+=(${(P)ignvar})
+    done
     opts[tmp+1]=_comp_ignore
   fi
 else
-  ign=
+  ign=()
 fi
 
 if zstyle -a ":completion:${curcontext}:" file-patterns tmp; then


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

Re: Quoting problems with _zip (unzip) completer

[Nikolai Weibull]

On Mon, Aug 17, 2009 at 23:49, Peter
Stephenson<p.w.stephenson@ntlworld.com> wrote:
> On Mon, 17 Aug 2009 21:58:19 +0100
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>> So please watch out for any anomalous _arguments quoting behaviour.
>
> Found one in _rm... but it's not a knock-on effect of this patch, it's
> another thing that was already broken.
>
>  rm stuff <TAB>
>
> is supposed to exclude "stuff" from being completed again by using the
> -F option to _files by together with the line from _arguments:
>
>    ignored=(${line//(#m)[\[\]()\\*?#<>~\^]/\\$MATCH})
>    _files -F ignored && ret=0
>
> However, the code for "_files -F <array>" was
> broken:  instead of appending the contents of <array> after the -F to
> the contents of the existing array, it appended the name <array>.

Great!

> (I'm not *quite* sure how this works, actually.  The extra quoting shown
> in the code above protects pattern characters, since we're actually
> passing a pattern, not a file name, so 'foo\[bar' from line becomes
> 'foo\\\[bar'.  Unquoting the pattern characters gives 'foo\[bar' back as
> a literal match.  I don't understand why this matches the file...  I'm
> going to pretend I didn't notice.)

I don't want to sound pessimistic, but aren't we approaching a point
where the quoting and pattern matching code simply has to be
re(written|factored)?  I'm not volunteering, but it seems that more or
less every bug fix goes something like "OK, I'm not sure what this
code is supposed to do, but it seems to work if we do X first".  Who
wrote the quoting and pattern matching code?  Was it Sven?  Is he not
available to document this kind of stuff any longer?

I hope I don't sound critical of the development process.  I have no
right to be.  I just thought I'd mention my observation, seeing as I
failed to do so for the one below.

> I'm a bit surprised nobody noticed this wasn't working.  If anyone finds
> this has a negative rather an a positive effect on ignoring files in
> "rm" completion, please try to reproduce it starting from "zsh -f".

I must confess that I had noticed it, but ignored it, figuring that my
code was broken and that it had been fixed in CVS.  I should have
investigated this, and for not doing so I apologize.

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

Nikolai Weibull wrote:
> I don't want to sound pessimistic, but aren't we approaching a point
> where the quoting and pattern matching code simply has to be
> re(written|factored)?

To some extent, yes, and I looked at the completion quoting code: it's
absolutely ghastly and I got pretty much nowhere.  (This was the case of
nested quoting, however, so it's harder than the sort of thing we've
been talking about.)  Sven hasn't been available for some years and
anyway the object would be to tease something more manageable out, which
is a rather different prospect from fixing up the existing code.

The pattern matching code in use here is the main shell's; this is
actually relatively under control and doesn't merit a rewrite.  The big
problem we have in cases like yesterday's is the standard one with
regular expressions that it's not trivial to quote them from expansion.
We could fix that with a flag.

There is the case of the completion matching code, which is separate but
calls the normal pattern matching code.  I have started to improve this
to support multibyte characters but got bogged down; the core now works
but there's much more work to be done in the logic that uses it, which
is heavily tied to one byte per character.

It's not entirely clear you're ever going to end up with something
completely clean anyway, and a lot of the problem isn't the
internals---broadly these do the right thing in most cases or can be
persuaded to do so with local changes like yesterday's.  There's no
guarantee that a rewrite is going to pick remaining issues up.  There is
the further problem of the conventions of passing between different
parts of the system, which need to know what's quoted, what's a pattern,
what can be expanded, etc.  This is probably what I ran into
yesterday---some part of the code has a convention where a file name is
quoted where you would expect it not to.  Fixing all that would really
mean a complete rewrite of the entire system, adding much more internal
state.  I don't think there's any prospect of that.

Then of course there's the problem that at the current rate any changes
other than simple, local ones are going to take longer than the age of
the universe.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


'member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom'

Re: Quoting problems with _zip (unzip) completer

[Mikael Magnusson]

On 17 August 2009 21:58, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Tue, 04 Aug 2009 09:50:59 +0100
> Peter Stephenson <pws@csr.com> wrote:
>> Mikael Magnusson wrote:
>> > % unzip test\[.zip <tab>
>> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>>
>> ... the value of "line" [in _zip] is a bit inconsistent.  Sure
>> enough if I use ~/tmp/zip/tmp\[.zip $line[1] comes back (using print -r) as
>>
>>   ~/tmp/zip/test[.zip
>>
>> which is wrong---either the ~ needs to be expanded, or the [ needs to be
>> quoted.  So this needs tracking internally, unfortunately.  If it hits
>> the internal completion quoting system we're probably stuck---I spent
>> weeks looking at that a couple of years ago and got virtually nowhere.
>> However, it may not be that bad in this case.
>
> It's inside comparguments, but luckily in only one place.  This removes
> the internal unquoting so the form above works.
>
> This could easily have knock-on effects in other callers of _arguments,
> however they should be fixable by simple local changes (and at least
> one case, the one above, works without updating, so others probably do,
> too), while with the old code there were unfixable cases, so I think we
> just need to identify them and change them as they come up.
>
> So please watch out for any anomalous _arguments quoting behaviour.

Hm, I can't remember if this fixed my original problem of not being
able to complete after unzip test\[.zip, if it did, something seems to
have broken it again. I do still have it in my "zsh-unfixed" dir so I
think it didn't. I don't get any errors, but I also don't get any
completions. (I actually encountered it when completing zdelattr,
which for some reason also uses $~, ah, you added that for me ;).

Long story short, if it was never fixed, never mind, I just want to
know if I'm getting senile. :)

--
Mikael Magnusson

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

On Wed, 3 Feb 2010 02:16:15 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> On 17 August 2009 21:58, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On Tue, 04 Aug 2009 09:50:59 +0100
> > Peter Stephenson <pws@csr.com> wrote:
> >> Mikael Magnusson wrote:
> >> > % unzip test\[.zip <tab>
> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)

The exact test above is currently working for me, with my default
completion setup.

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

Re: Quoting problems with _zip (unzip) completer

[Mikael Magnusson]

>> > Peter Stephenson <pws@csr.com> wrote:
>> >> Mikael Magnusson wrote:
>> >> > % unzip test\[.zip <tab>
>> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
>
> The exact test above is currently working for me, with my default
> completion setup.

As in, if that is an actual zip file with files in it, you get a
listing of those files?

-- 
Mikael Magnusson

Re: Quoting problems with _zip (unzip) completer

[Benjamin R. Haskell]

On Wed, 3 Feb 2010, Mikael Magnusson wrote:

> >> > Peter Stephenson <pws@csr.com> wrote:
> >> >> Mikael Magnusson wrote:
> >> >> > % unzip test\[.zip <tab>
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >
> > The exact test above is currently working for me, with my default 
> > completion setup.
> 
> As in, if that is an actual zip file with files in it, you get a 
> listing of those files?
> 
> 

Breaks for me w/ latest git.  Changing _zip line 117:
from      zipfile=( $~line[1](|.zip|.ZIP) )
to        zipfile=( $line[1](|.zip|.ZIP) )

fixes the 'test[.zip' case, but renaming 'test[.zip' to '*.zip' shows a 
weirder problem (present w/ or w/o the change):

$ unzip '*.zip' <TAB>
3 archives were successfully processed.
[Contents of a.zip]

The '3' comes from 'a.zip', 'test[.zip', and '*.zip'
$ cp a.zip b.zip
$ unzip '*.zip' <TAB>
4 archives were successfully processed.
[Contents of a.zip]

Even more interesting is that '*.zip' doesn't need to exist. :-)

Contents appear to be the first .zip in alpha order.

unzip -v
UnZip 6.00 of 20 April 2009, by Info-ZIP.[...]

(more version info available if needed)

-- 
Best,
Ben

Re: Quoting problems with _zip (unzip) completer

[Benjamin R. Haskell]

On Wed, 3 Feb 2010, Benjamin R. Haskell wrote:

> On Wed, 3 Feb 2010, Mikael Magnusson wrote:
> 
> > >> > Peter Stephenson <pws@csr.com> wrote:
> > >> >> Mikael Magnusson wrote:
> > >> >> > % unzip test\[.zip <tab>
> > >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > >
> > > The exact test above is currently working for me, with my default 
> > > completion setup.
> > 
> > As in, if that is an actual zip file with files in it, you get a 
> > listing of those files?
> > 
> > 
> 
> Breaks for me w/ latest git.  Changing _zip line 117:
> from      zipfile=( $~line[1](|.zip|.ZIP) )
> to        zipfile=( $line[1](|.zip|.ZIP) )
> 
> fixes the 'test[.zip' case, but renaming 'test[.zip' to '*.zip' shows a 
> weirder problem (present w/ or w/o the change):
> 
> $ unzip '*.zip' <TAB>
> 3 archives were successfully processed.
> [Contents of a.zip]
> 
> The '3' comes from 'a.zip', 'test[.zip', and '*.zip'
> $ cp a.zip b.zip
> $ unzip '*.zip' <TAB>
> 4 archives were successfully processed.
> [Contents of a.zip]
> 
> Even more interesting is that '*.zip' doesn't need to exist. :-)
> 
> Contents appear to be the first .zip in alpha order.
> 
> unzip -v
> UnZip 6.00 of 20 April 2009, by Info-ZIP.[...]
> 
> (more version info available if needed)
> 

More importantly:

ZipInfo 3.00 of 20 April 2009, by Greg Roelofs and the Info-ZIP group.

[...]

"file[.zip]" may be a wildcard name containing *, ?, [] (e.g., "[a-j]*.zip").

-- 
Best,
Ben

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

Mikael Magnusson wrote:
> >> > Peter Stephenson <pws@csr.com> wrote:
> >> >> Mikael Magnusson wrote:
> >> >> > % unzip test\[.zip <tab>
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >> >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> >
> > The exact test above is currently working for me, with my default
> > completion setup.
> 
> As in, if that is an actual zip file with files in it, you get a
> listing of those files?

Yes, just tried it here and it worked again (but that's not really a
surprise since my settings are extremely similar).

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

On Wed, 3 Feb 2010 22:09:58 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Wed, 3 Feb 2010 02:16:15 +0100
> Mikael Magnusson <mikachu@gmail.com> wrote:
> > On 17 August 2009 21:58, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > > On Tue, 04 Aug 2009 09:50:59 +0100
> > > Peter Stephenson <pws@csr.com> wrote:
> > >> Mikael Magnusson wrote:
> > >> > % unzip test\[.zip <tab>
> > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> 
> The exact test above is currently working for me, with my default
> completion setup.

It's working starting from zsh -f just loading compinit, too.  I tried
another vanilla version of zsh just to make sure.

If the other part of the thread doesn't yield anything, could you see if
you can find what's breaking it, in terms of shell settings or environment?
(To be quite clear: I'm not going to be doing this myself without some
indication of what to do.)

Ben wrote:
> Breaks for me w/ latest git.  Changing _zip line 117:
> from      zipfile=( $~line[1](|.zip|.ZIP) )
> to        zipfile=( $line[1](|.zip|.ZIP) )
> 
> fixes the 'test[.zip' case

This isn't a proper fix.  If you want to play along, read and digest the
description with my first patch.  The key point is to keep ~-expansion
working.

However, this form does certainly have the globbing problems you pointed
out.  $~ is doing multiple things not all of which we want; I wonder if
this might be causing other problems, too.  It's annoyingly hard to get the
effect of file expansion and removal of quotes without the effect of
globbing, but a scalar assignment with $~ and an explicit unquote seems to
do the trick.  Does changing it along the lines of this patch fix the
original problem?

Index: Completion/Unix/Command/_zip
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_zip,v
retrieving revision 1.9
diff -u -r1.9 _zip
--- Completion/Unix/Command/_zip	7 Nov 2006 10:38:54 -0000	1.9
+++ Completion/Unix/Command/_zip	4 Feb 2010 11:30:48 -0000
@@ -1,6 +1,6 @@
 #compdef zip unzip zipinfo
 
-local suffixes suf zipfile uzi
+local suffixes suf zipfile uzi testfile
 local expl curcontext="$curcontext" state line ret=1
 typeset -A opt_args
 
@@ -114,10 +114,18 @@
     if [[ $service = zip ]] && (( ! ${+opt_args[-d]} )); then
       _wanted files expl zfile _files -g '^(#i)*.(zip|xpi|[ejw]ar)(-.)' && return
     else
-      zipfile=( $~line[1](|.zip|.ZIP) )
-      [[ -z $zipfile[1] ]] && return 1
-      if [[ $zipfile[1] !=  $_zip_cache_list ]]; then
-	_zip_cache_name="$zipfile[1]"
+      testfile=${~${(Q)line[1]}}
+      if [[ -f $testfile ]]; then
+	zipfile=$testfile
+      elif [[ -f $testfile.zip ]]; then
+	zipfile=$testfile.zip
+      elif [[ -f $testfile.ZIP ]]; then
+	zipfile=$testfile.ZIP
+      else
+	return 1
+      fi
+      if [[ $zipfile !=  $_zip_cache_list ]]; then
+	_zip_cache_name="$zipfile"
 	_zip_cache_list=( ${(f)"$(zipinfo -1 $_zip_cache_name)"} )
       fi
      _wanted files expl 'file from archive' \
 
-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: Quoting problems with _zip (unzip) completer

[Benjamin R. Haskell]

On Thu, 4 Feb 2010, Peter Stephenson wrote:

> On Wed, 3 Feb 2010 22:09:58 +0000
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On Wed, 3 Feb 2010 02:16:15 +0100
> > Mikael Magnusson <mikachu@gmail.com> wrote:
> > > On 17 August 2009 21:58, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > > > On Tue, 04 Aug 2009 09:50:59 +0100
> > > > Peter Stephenson <pws@csr.com> wrote:
> > > >> Mikael Magnusson wrote:
> > > >> > % unzip test\[.zip <tab>
> > > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > > >> > _zip:117: bad pattern: test[.zip(|.zip|.ZIP)
> > 
> > The exact test above is currently working for me, with my default
> > completion setup.
> 
> It's working starting from zsh -f just loading compinit, too.  I tried 
> another vanilla version of zsh just to make sure.
> 
> If the other part of the thread doesn't yield anything, could you see 
> if you can find what's breaking it, in terms of shell settings or 
> environment?  (To be quite clear: I'm not going to be doing this 
> myself without some indication of what to do.)

I get brokenness with stock options.

$ zsh -f +d
host% mkdir /tmp/zshzip
host% cd /tmp/zshzip
host% mkdir foo
host% touch foo/bar
host% zip -r 'test[.zip' foo
  adding: foo/ (stored 0%)
  adding: foo/bar (stored 0%)
host% rm -rf foo
host% autoload compinit
host% compinit -d ./compinit
host% unzip 'test[.zip' <TAB>
_zip:117: bad pattern: test[.zip(|.zip|.ZIP)
host% unzip 'test[.zip' <cursor>

(commands only, for copypasting):
zsh -f +d
mkdir /tmp/zshzip
cd /tmp/zshzip
mkdir foo
touch foo/bar
zip -r 'test[.zip' foo
rm -rf foo
autoload compinit
compinit -d ./compinit


> Ben wrote:
> > Breaks for me w/ latest git.  Changing _zip line 117:
> > from      zipfile=( $~line[1](|.zip|.ZIP) )
> > to        zipfile=( $line[1](|.zip|.ZIP) )
> > 
> > fixes the 'test[.zip' case
> 
> This isn't a proper fix.  If you want to play along, read and digest 
> the description with my first patch.  The key point is to keep 
> ~-expansion working.

My bad.  Yep, I obviously glossed over that.


> However, this form does certainly have the globbing problems you 
> pointed out.  $~ is doing multiple things not all of which we want; I 
> wonder if this might be causing other problems, too.  It's annoyingly 
> hard to get the effect of file expansion and removal of quotes without 
> the effect of globbing, but a scalar assignment with $~ and an 
> explicit unquote seems to do the trick.  Does changing it along the 
> lines of this patch fix the original problem?

Yes.  That works for me for the Zsh pattern problem.  Still exploring 
the zipinfo problem.


> Index: Completion/Unix/Command/_zip
> +      if [[ $zipfile !=  $_zip_cache_list ]]; then
> +        _zip_cache_name="$zipfile"

Shouldn't the first line there be $_zip_cache_list be $_zip_cache_name?  
(The problem was there before your patch [line 119 prepatch, 128 post])  
Otherwise I don't think it'll ever cache the list.

-- 
Best,
Ben

Re: Quoting problems with _zip (unzip) completer

[Peter Stephenson]

"Benjamin R. Haskell" wrote:
> Yes.  That works for me for the Zsh pattern problem.  Still exploring 
> the zipinfo problem.

Good; I didn't look at the latter, it sounded less like it needed low
level hackery.

There's definitely room in the market for someone to write a zsh
completion function how-to, even on top of the documentation, the
existing functions, and From Bash to Z-Shell.  The use of quoted
strings direct from the command line vs. unquoted strings directly
representing file names could do with some description.

> > Index: Completion/Unix/Command/_zip
> > +      if [[ $zipfile !=  $_zip_cache_list ]]; then
> > +        _zip_cache_name="$zipfile"
> 
> Shouldn't the first line there be $_zip_cache_list be $_zip_cache_name?  
> (The problem was there before your patch [line 119 prepatch, 128 post])  
> Otherwise I don't think it'll ever cache the list.

Yes, I'll tweak that in case this change ever gets submitted.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom