* Quoting problems with _zip (unzip) completer @ 2009-08-03 20:15 Mikael Magnusson 2009-08-04 8:50 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Mikael Magnusson @ 2009-08-03 20:15 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-03 20:15 Quoting problems with _zip (unzip) completer Mikael Magnusson @ 2009-08-04 8:50 ` Peter Stephenson 2009-08-04 16:31 ` Mikael Magnusson 2009-08-17 20:58 ` Peter Stephenson 0 siblings, 2 replies; 16+ messages in thread From: Peter Stephenson @ 2009-08-04 8:50 UTC (permalink / raw) To: zsh-workers 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' ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-04 8:50 ` Peter Stephenson @ 2009-08-04 16:31 ` Mikael Magnusson 2009-08-17 20:58 ` Peter Stephenson 1 sibling, 0 replies; 16+ messages in thread From: Mikael Magnusson @ 2009-08-04 16:31 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-04 8:50 ` Peter Stephenson 2009-08-04 16:31 ` Mikael Magnusson @ 2009-08-17 20:58 ` Peter Stephenson 2009-08-17 21:49 ` Peter Stephenson 2010-02-03 1:16 ` Mikael Magnusson 1 sibling, 2 replies; 16+ messages in thread From: Peter Stephenson @ 2009-08-17 20:58 UTC (permalink / raw) To: zsh-workers 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/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-17 20:58 ` Peter Stephenson @ 2009-08-17 21:49 ` Peter Stephenson 2009-08-17 21:59 ` Nikolai Weibull 2010-02-03 1:16 ` Mikael Magnusson 1 sibling, 1 reply; 16+ messages in thread From: Peter Stephenson @ 2009-08-17 21:49 UTC (permalink / raw) To: zsh-workers 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/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-17 21:49 ` Peter Stephenson @ 2009-08-17 21:59 ` Nikolai Weibull 2009-08-18 9:10 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Nikolai Weibull @ 2009-08-17 21:59 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-17 21:59 ` Nikolai Weibull @ 2009-08-18 9:10 ` Peter Stephenson 0 siblings, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2009-08-18 9:10 UTC (permalink / raw) To: zsh-workers 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' ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2009-08-17 20:58 ` Peter Stephenson 2009-08-17 21:49 ` Peter Stephenson @ 2010-02-03 1:16 ` Mikael Magnusson 2010-02-03 22:09 ` Peter Stephenson 1 sibling, 1 reply; 16+ messages in thread From: Mikael Magnusson @ 2010-02-03 1:16 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 1:16 ` Mikael Magnusson @ 2010-02-03 22:09 ` Peter Stephenson 2010-02-03 22:43 ` Mikael Magnusson 2010-02-04 11:37 ` Peter Stephenson 0 siblings, 2 replies; 16+ messages in thread From: Peter Stephenson @ 2010-02-03 22:09 UTC (permalink / raw) To: zsh workers 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/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 22:09 ` Peter Stephenson @ 2010-02-03 22:43 ` Mikael Magnusson 2010-02-03 23:11 ` Benjamin R. Haskell 2010-02-04 10:03 ` Peter Stephenson 2010-02-04 11:37 ` Peter Stephenson 1 sibling, 2 replies; 16+ messages in thread From: Mikael Magnusson @ 2010-02-03 22:43 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh workers >> > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 22:43 ` Mikael Magnusson @ 2010-02-03 23:11 ` Benjamin R. Haskell 2010-02-03 23:21 ` Benjamin R. Haskell 2010-02-04 10:03 ` Peter Stephenson 1 sibling, 1 reply; 16+ messages in thread From: Benjamin R. Haskell @ 2010-02-03 23:11 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Peter Stephenson, zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 23:11 ` Benjamin R. Haskell @ 2010-02-03 23:21 ` Benjamin R. Haskell 0 siblings, 0 replies; 16+ messages in thread From: Benjamin R. Haskell @ 2010-02-03 23:21 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Peter Stephenson, zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 22:43 ` Mikael Magnusson 2010-02-03 23:11 ` Benjamin R. Haskell @ 2010-02-04 10:03 ` Peter Stephenson 1 sibling, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2010-02-04 10:03 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-03 22:09 ` Peter Stephenson 2010-02-03 22:43 ` Mikael Magnusson @ 2010-02-04 11:37 ` Peter Stephenson 2010-02-04 14:15 ` Benjamin R. Haskell 1 sibling, 1 reply; 16+ messages in thread From: Peter Stephenson @ 2010-02-04 11:37 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-04 11:37 ` Peter Stephenson @ 2010-02-04 14:15 ` Benjamin R. Haskell 2010-02-04 14:22 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Benjamin R. Haskell @ 2010-02-04 14:15 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh workers 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Quoting problems with _zip (unzip) completer 2010-02-04 14:15 ` Benjamin R. Haskell @ 2010-02-04 14:22 ` Peter Stephenson 0 siblings, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2010-02-04 14:22 UTC (permalink / raw) To: zsh workers "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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-02-04 14:22 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-08-03 20:15 Quoting problems with _zip (unzip) completer Mikael Magnusson 2009-08-04 8:50 ` Peter Stephenson 2009-08-04 16:31 ` Mikael Magnusson 2009-08-17 20:58 ` Peter Stephenson 2009-08-17 21:49 ` Peter Stephenson 2009-08-17 21:59 ` Nikolai Weibull 2009-08-18 9:10 ` Peter Stephenson 2010-02-03 1:16 ` Mikael Magnusson 2010-02-03 22:09 ` Peter Stephenson 2010-02-03 22:43 ` Mikael Magnusson 2010-02-03 23:11 ` Benjamin R. Haskell 2010-02-03 23:21 ` Benjamin R. Haskell 2010-02-04 10:03 ` Peter Stephenson 2010-02-04 11:37 ` Peter Stephenson 2010-02-04 14:15 ` Benjamin R. Haskell 2010-02-04 14:22 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).