Bug in _path_files

Bug in _path_files

[Vin Shelton]

Hello,

Running from the latest CVS sources, the following gives me an error:

Wed 7 15:10; ls $SRCROOT/<TAB>
_path_files:211: bad pattern: [^\

Apparently, this was caused by Peter's change to subst.c.  I tried to
modify _path_files by taking out the double backslash at line 211, but
that did not produce the desired results.

  - Vin

-- 
In a minute there is time
For decisions and revisions which a minute will reverse.         T.S. Eliot
[URL: http://www.cs.amherst.edu/~ccm/prufrock.html]

Re: Bug in _path_files

[Peter Stephenson]

Vin Shelton wrote:
> Hello,
> 
> Running from the latest CVS sources, the following gives me an error:
> 
> Wed 7 15:10; ls $SRCROOT/<TAB>
> _path_files:211: bad pattern: [^\
> 
> Apparently, this was caused by Peter's change to subst.c.  I tried to
> modify _path_files by taking out the double backslash at line 211, but
> that did not produce the desired results.

(You would need to replace the two backslashes with a single one.)

Whoops, it seems the old fudge was documented and used in various
places.  Since it doesn't work properly I still think we need to change
it, however.

I've replaced the completion code in such a way that it doesn't matter,
since the slashes all appear inside parameters.

Not quite sure what to do about 4.0 --- I applied the first patch but
maybe it's better to leave it and document the bug.

Index: README
===================================================================
RCS file: /cvsroot/zsh/zsh/README,v
retrieving revision 1.11
diff -u -r1.11 README
--- README	2 May 2003 10:58:52 -0000	1.11
+++ README	8 May 2003 09:59:44 -0000
@@ -39,6 +39,15 @@
 option is enabled, as it is by default) no longer expands aliases.  It was
 felt this feature was underused and confusing.
 
+In 4.0, a literal `/' was quoted in the `src' text of a substitution of the
+form `${foo/src/rep}' or ${foo//src/rep} with two backslashes.  This was
+documented, but inconsistent with normal quoting conventions and poorly
+implemented.  The `/' now requires only one backslash to quote it whether
+or not the expression occurs in double quotes.  For example:
+  % foo=word/bird
+  % print ${foo/\//-} "${foo/\//+}"
+  word-bird word+bird
+
 Documentation
 -------------
 
Index: Completion/Unix/Command/_cvs
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_cvs,v
retrieving revision 1.18
diff -u -r1.18 _cvs
--- Completion/Unix/Command/_cvs	12 Mar 2003 04:43:50 -0000	1.18
+++ Completion/Unix/Command/_cvs	8 May 2003 09:59:44 -0000
@@ -611,7 +611,7 @@
 
 (( $+functions[_cvs_root] )) ||
 _cvs_root () {
-  local cvspassfile id
+  local cvspassfile id slash
 
   typeset -gU _cvs_roots
 
@@ -622,7 +622,8 @@
       id="$(LC_ALL=C ls -l "$cvspassfile")"
     fi
     if [[ "$id" != "$_cvs_pass_id" ]]; then
-      _cvs_roots=($_cvs_roots ${${${${(f)"$(<$cvspassfile)"}#/1 }%% *}/:2401\\//:/})
+      slash=/
+      _cvs_roots=($_cvs_roots ${${${${(f)"$(<$cvspassfile)"}#/1 }%% *}/:2401${slash}/:/})
       _cvs_pass_id="$id"
     fi
   fi
@@ -918,13 +919,13 @@
 (( $+functions[_cvs_modified_entries] )) ||
 _cvs_modified_entries () {
   if _cvs_loadstat; then
-    local expl match linedir realdir pat
+    local expl match linedir realdir pat slash=/
     match=()
     : ${PREFIX:#(#b)(*/)(*)}
     linedir="$match[1]"
     realdir=${(e)~linedir}
     [[ -f "$realdir"CVS/Entries ]] &&
-    [[ -n ${pat::="${(@j:|:)${(@)${(@)${(@)${(@)${(@)${(@M)${(@f)"$(<"$realdir"CVS/Entries)"}:#/*}#/}/\\/[^\\/]#\\///}%/[^/]#/[^/]#}:#${(j:|:)~${${${${(f)"$(LC_ALL=C builtin stat -gn +mtime -F '%a %b %e %T %Y' ${realdir}*(D) 2>/dev/null)"}##*/}/ //}//(#m)[][*?()<|^~#\\]/\\$MATCH}}}%%/*}//(#m)[][*?()<|^~#\\]/\\$MATCH}"} ]] &&
+    [[ -n ${pat::="${(@j:|:)${(@)${(@)${(@)${(@)${(@)${(@M)${(@f)"$(<"$realdir"CVS/Entries)"}:#/*}#/}/${slash}[^${slash}]#${slash}//}%/[^/]#/[^/]#}:#${(j:|:)~${${${${(f)"$(LC_ALL=C builtin stat -gn +mtime -F '%a %b %e %T %Y' ${realdir}*(D) 2>/dev/null)"}##*/}/ //}//(#m)[][*?()<|^~#\\]/\\$MATCH}}}%%/*}//(#m)[][*?()<|^~#\\]/\\$MATCH}"} ]] &&
     _wanted files expl 'modified file' _path_files -g "$pat"
   else
     _cvs_existing_entries
Index: Completion/Unix/Command/_perforce
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_perforce,v
retrieving revision 1.6
diff -u -r1.6 _perforce
--- Completion/Unix/Command/_perforce	25 Apr 2003 11:19:09 -0000	1.6
+++ Completion/Unix/Command/_perforce	8 May 2003 09:59:44 -0000
@@ -906,7 +906,7 @@
     # by various logical operators.  The `=' could be a comparison,
     # but we don't currently bother with that here; it's a bit cumbersome
     # to complete.
-    local line type oldifs=$IFS IFS= key value
+    local line type oldifs=$IFS IFS= key value slash=/
     local match mbegin mend
     # This is simply to split out two space-delimited words a backreferences.
     local m2words
@@ -962,7 +962,7 @@
 
     for key in ${(k)p4fields}; do
 	if [[ -n ${p4values[$key]} ]]; then
-	    valuespec+=("${key}:${p4fields[$key]}:(${p4values[$key]//\\// })")
+	    valuespec+=("${key}:${p4fields[$key]}:(${p4values[$key]//$slash/ })")
 	elif [[ $key = job ]]; then
 	    # Nothing special for jobs; add our own completion.
 	    valuespec+=("${key}:Perforce job:_perforce_jobs")
Index: Completion/Unix/Command/_rsync
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_rsync,v
retrieving revision 1.5
diff -u -r1.5 _rsync
--- Completion/Unix/Command/_rsync	21 May 2002 07:44:58 -0000	1.5
+++ Completion/Unix/Command/_rsync	8 May 2003 09:59:44 -0000
@@ -31,13 +31,14 @@
   _describe "remote modules" remmodules -S/
 
 elif [[ -prefix 1 *: ]]; then
-  local remfiles remdispf remdispd
+  local remfiles remdispf remdispd slash
 
   compset -P 1 '*:'
 
   if zstyle -T ":completion:${curcontext}:" remote-access; then
+    slash=/
     remfiles=(${(f)"$(ssh -a -x ${words[CURRENT]%:*} ls -d1F ${${${words[CURRENT
-]#*:}:h}/\\/(#e)/}/\* 2>/dev/null)"})
+]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null)"})
 
     remdispf=(${remfiles:#*/})
     remdispd=(${(M)remfiles:#*/})
@@ -46,7 +47,7 @@
       compadd -d remdispf ${${remfiles:#*/}/[*=@|](#e)/}
       
     _wanted files expl 'remote files and directories' \
-      compadd -S/ -d remdispd ${${(M)remfiles:#*/}/\\/(#e)/}
+      compadd -S/ -d remdispd ${${(M)remfiles:#*/}/${slash}(#e)/}
   else
     _message -e remote-files 'remote files'
   fi
Index: Completion/Unix/Type/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_path_files,v
retrieving revision 1.22
diff -u -r1.22 _path_files
--- Completion/Unix/Type/_path_files	26 Feb 2003 14:32:50 -0000	1.22
+++ Completion/Unix/Type/_path_files	8 May 2003 09:59:45 -0000
@@ -208,7 +208,8 @@
   eval 'realpath=${(e)~linepath}' 2>/dev/null
   [[ -z "$realpath" || "$realpath" = "$linepath" ]] && return 1
   pre="${pre#${linepath}}"
-  i="${#linepath//[^\\/]}"
+  i='[^/]'
+  i="${#linepath//$i}"
   orig="${orig[1,(in:i:)/][1,-2]}"
   donepath=
   prepaths=( '' )
Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.45
diff -u -r1.45 expn.yo
--- Doc/Zsh/expn.yo	2 May 2003 10:25:32 -0000	1.45
+++ Doc/Zsh/expn.yo	8 May 2003 09:59:45 -0000
@@ -510,8 +510,8 @@
 var(pattern) must match at the start of the string, or `tt(%)', in
 which case it must match at the end of the string.  The var(repl) may
 be an empty string, in which case the final `tt(/)' may also be omitted.
-To quote the final `tt(/)' in other cases it should be preceded by two
-backslashes (i.e., a quoted backslash); this is not necessary if the
+To quote the final `tt(/)' in other cases it should be preceded by a
+single backslash; this is not necessary if the
 `tt(/)' occurs inside a substituted parameter.  Note also that the `tt(#)'
 and `tt(%)' are not active if they occur inside a substituted parameter,
 even at the start.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Bug in _path_files

[Bart Schaefer]

On May 8, 11:06am, Peter Stephenson wrote:
}
} Whoops, it seems the old fudge was documented and used in various
} places.

Dang.  I grep'd for that the day you posted the patch, thinking it was
probably going to break something, but forgot to hit all the completion
subdirectories ...

} Not quite sure what to do about 4.0 --- I applied the first patch but
} maybe it's better to leave it and document the bug.

There was a time I'd have said that was the way to go ... but now we have
assorted Linux distributions and the like bundling the "stable" releases,
so the longer we leave these kinds of discrepancies the worse the damage
is likely to be when 4.2 or 5.0 or whatever it will be, comes out.

So I'd recommend that you apply both patches to the 4.0 branch, and that
we (you) think about doing a 4.0.7 sometime soon.

Re: Bug in _path_files

[Peter Stephenson]

"Bart Schaefer" wrote:
> } Not quite sure what to do about 4.0 --- I applied the first patch but
> } maybe it's better to leave it and document the bug.
> 
> There was a time I'd have said that was the way to go ... but now we have
> assorted Linux distributions and the like bundling the "stable" releases,
> so the longer we leave these kinds of discrepancies the worse the damage
> is likely to be when 4.2 or 5.0 or whatever it will be, comes out.
> 
> So I'd recommend that you apply both patches to the 4.0 branch, and that
> we (you) think about doing a 4.0.7 sometime soon.

Committed with changes, thanks for the feedback.

By the way, there's still a test failing in 4.0; it appears that the
patch for B02typeset.ztst needed as a consequence of another parameter
fix hasn't been applied to the 4.0 branch.

I will have a grand release fest when I can summon the time and energy.
To answer Geoff's question of a few days ago, I won't be going away
again until late June.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************