Re: Another _path_files bug?

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Thu, 10 Feb 2011 09:28:20 -0800
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Feb 10,  8:42am, Bart Schaefer wrote:
> }
> } torch% setopt completeinword
> } torch% ls Test/
               ^completing here...
> } torch% ls Test//Test/Makefile
> } 
> } This only happens when there is only a single completion in the directory.
> } If the completion is ambiguous, it works correctly:
> } 
> } The first significant difference seems to be at around line 740 - 760 of
> } _path_files where, in the completeinword case, $mid gets assigned on line
> } 749.  I think, but am not yet sure, that when $tsuf contains but does not
> } begin with a "/", then at line 754 (and possibly also at line 752, but
> } again not sure) $cpre should NOT have a trailing slash appended.
> 
> That may or may not be the case, but I've narrowed this down to the compadd
> at line 783.  In the failing case, this does:
> 
>     compadd -Qf -J -default- -J -default- -p Test/ \
>             -s /Makefile -W Test// -M 'r:|/=* r:|=*' - Test

This is more an experiment than a suggested fix, but I wonder if the
problem is it's assuming there are three path sections: an initial path,
the middle bit, that the cursor's in the middle of, and then contents of
the directory at the end.  In the case here, we've only got two: the
"Test/" is being treated as both the initial part and the middle part.
So if we add a separate case for handling this it improves things.

Obvious we'll need to find out if the original problem was more general,
in which case splitting off this case may work but isn't the right fix,
or if this has repercussions in other simple cases.

The compquote stuff makes my hair stand on end, but I don't think
it comes in to play with simple file names like this.

By the way, we ought to take the opportunity to document further up that
this is handling completion in the middle of a path --- unless I'm the
only person in the world to feel that 'if [[ "$mid" = */ ]]' isn't
entirely self-explanatory.

Also, what does the 'if [[ -z "$tmp4" ]]' just above that signify?  I
think (from the comment above 729) it might just mean we've finished
collecting things and are about to add them to the completion ---
there's no 'else' in this case.  Even if we can't decide for sure a bit
of robust commentary speculation is probably better than the status quo.

I'm just imagining a Samuel Beckett character writing this file, so it's
time to stop and have supper.

Index: Completion/Unix/Type/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_path_files,v
retrieving revision 1.53
diff -p -u -r1.53 _path_files
--- Completion/Unix/Type/_path_files	6 Aug 2010 15:29:58 -0000	1.53
+++ Completion/Unix/Type/_path_files	11 Feb 2011 20:24:06 -0000
@@ -769,21 +769,35 @@ for prepath in "$prepaths[@]"; do
       SUFFIX="${osuf}"
 
       tmp4="${testpath#${mid}}"
-      tmp3="${mid%/*/}"
-      tmp2="${${mid%/}##*/}"
-      if [[ -n "$linepath" ]]; then
-        compquote -p tmp3
+      if [[ $mid = */*/* ]]; then
+	# Multiple levels of directory involved.
+	tmp3="${mid%/*/}"
+	tmp2="${${mid%/}##*/}"
+	if [[ -n "$linepath" ]]; then
+          compquote -p tmp3
+	else
+          compquote tmp3
+	fi
+	compquote tmp4 tmp2 tmp1
+	for i in "$tmp1[@]"; do
+	  _list_files tmp2 "$prepath$realpath${mid%/*/}"
+          compadd $Uopt -Qf "$mopts[@]" -p "${Uopt:+$IPREFIX}$linepath$tmp3/" \
+	    -s "/$tmp4$i${Uopt:+$ISUFFIX}" \
+            -W "$prepath$realpath${mid%/*/}/" \
+	    "$pfxsfx[@]" $Mopts $listopts - "$tmp2"
+	done
       else
-        compquote tmp3
+	# Simpler case with fewer directories: avoid double counting.
+	tmp2="${${mid%/}##*/}"
+	compquote tmp4 tmp2 tmp1
+	for i in "$tmp1[@]"; do
+	  _list_files tmp2 "$prepath$realpath${mid%/*/}"
+          compadd $Uopt -Qf "$mopts[@]" -p "${Uopt:+$IPREFIX}$linepath" \
+	    -s "/$tmp4$i${Uopt:+$ISUFFIX}" \
+            -W "$prepath$realpath" \
+	    "$pfxsfx[@]" $Mopts $listopts - "$tmp2"
+	done
       fi
-      compquote tmp4 tmp2 tmp1
-      for i in "$tmp1[@]"; do
-	_list_files tmp2 "$prepath$realpath${mid%/*/}"
-        compadd $Uopt -Qf "$mopts[@]" -p "${Uopt:+$IPREFIX}$linepath$tmp3/" \
-	        -s "/$tmp4$i${Uopt:+$ISUFFIX}" \
-                -W "$prepath$realpath${mid%/*/}/" \
-	        "$pfxsfx[@]" $Mopts $listopts - "$tmp2"
-      done
     else
       if [[ "$osuf" = */* ]]; then
         PREFIX="${opre}${osuf}"

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