file completion of ~user/... stats wrong directory

file completion of ~user/... stats wrong directory

[Greg Klanderman]

Hi,

In looking into some completion anomalies, I've noticed that strace
output shows it stat'ing the wrong path when you are completing a path
that starts with ~user/..

If you complete ~user/foo/bar, you see it stat the paths "foo/bar" and
then "foo" relative to the current directory.  This seems broken; even
if those paths exist relative to the current directory, it is
irrelevant to completing under ~user.  It seems to recover in most
cases and generate the right completions despite this, the only
problem this seems to be causing is with our netapp's .snapshot
directories.

Looking at _path_files, I'm guessing it has something to do with
setting prepaths=( '' ) in the logic dealing with ~ prefixed strings.
But I don't understand what prepaths is supposed to be doing.  It
worries me a bit that it is set to that same value in a number of
other cases.  Maybe it should be setting prepaths=() instead?

I'm running out of a nearly up-to-date cvs checkout (_path_files is
up to date at least).

thanks,
Greg

Re: file completion of ~user/... stats wrong directory

[Peter Stephenson]

On Thu, 9 Jul 2009 13:53:25 -0400
Greg Klanderman <gak@klanderman.net> wrote:
> Looking at _path_files, I'm guessing it has something to do with
> setting prepaths=( '' ) in the logic dealing with ~ prefixed strings.
> But I don't understand what prepaths is supposed to be doing.  It
> worries me a bit that it is set to that same value in a number of
> other cases.  Maybe it should be setting prepaths=() instead?

prepaths comes from the -W option (see the zparseopts call), which is used
when you want the file matcher to behave as if there was a path
prefix---for example, to complete folders in your mail directory, you might
use "-W ~/Mail".

I think the reason it's not an empty array is that matches are eventually
generated in a loop over all -W paths.  If there are none, we still want to
execute that loop once, but with an empty prepath (at line 340):

for prepath in "$prepaths[@]"; do

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

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

>>>>> Peter Stephenson <pws@csr.com> writes:

> prepaths comes from the -W option (see the zparseopts call), which is used
> when you want the file matcher to behave as if there was a path
> prefix---for example, to complete folders in your mail directory, you might
> use "-W ~/Mail".

Right, I'd kinda surmised it was something like that.

> I think the reason it's not an empty array is that matches are eventually
> generated in a loop over all -W paths.  If there are none, we still want to
> execute that loop once, but with an empty prepath (at line 340):

> for prepath in "$prepaths[@]"; do

So do you agree it's a bug that it's stat-ing the path with the ~user/
stripped off, effectively then relative to the current directory?

So, if it needs to go through that loop at least once, then maybe when
'prepath' (singular) is '' it should be prepending any stuff that got
stripped off the front back on?  Guess I'll have to grovel through
there and try to make sense of what's going on...

Greg

Re: file completion of ~user/... stats wrong directory

[Peter Stephenson]

Greg Klanderman wrote:
> So do you agree it's a bug that it's stat-ing the path with the ~user/
> stripped off, effectively then relative to the current directory?
> 
> So, if it needs to go through that loop at least once, then maybe when
> 'prepath' (singular) is '' it should be prepending any stuff that got
> stripped off the front back on?  Guess I'll have to grovel through
> there and try to make sense of what's going on...

Certainly sounds plausible.  It's odd this isn't an ever worse problem,
but _path_files has so many possibilities it's surprisingly hard to
trigger particular problems.

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

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

>>>>> On July 10, 2009 Peter Stephenson <pws@csr.com> wrote:

> Certainly sounds plausible.  It's odd this isn't an ever worse problem,
> but _path_files has so many possibilities it's surprisingly hard to
> trigger particular problems.

Looks like this is being caused by the 'accept-exact-dirs' logic in
_path_files:

|  if zstyle -t ":completion:${curcontext}:paths" accept-exact-dirs &&
|    [[ $pre = (#b)(*)/([^/]#) ]]; then
|    # We've been told that we can accept an exact directory
|    # prefix immediately.  Try this with the longest path prefix
|    # first:  this saves stats in the simple case and may get around
|    # automount behaviour if early components don't yet exist.
|    tmp1=$match[1]
|    tpre=$match[2]
|    while true; do
|      if [[ -d $donepath$tmp1 ]]; then
|        donepath=$donepath$tmp1/
|        pre=$tpre
|        break
|      elif [[ $tmp1 = (#b)(*)/([^/]#) ]]; then
|        tmp1=$match[1]
|        tpre=$match[2]/$tpre
|      else
|        break
|      fi
|    done
|  fi

When completing something of the form ~user/foo or $bar/foo, the test
[[ -d $donepath$tmp1 ]] is stat'ing the path 'foo' relative to the
current directory rather than with the stripped off prefix re-added.

greg

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

>>>>> On September 9, 2009 Greg Klanderman <gak@klanderman.net> wrote:

> Looks like this is being caused by the 'accept-exact-dirs' logic in
> _path_files:

Attached below is the one line fix.

greg

Index: Completion/Unix/Type/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_path_files,v
retrieving revision 1.47
diff -u -r1.47 _path_files
--- Completion/Unix/Type/_path_files	5 Aug 2009 00:46:45 -0000	1.47
+++ Completion/Unix/Type/_path_files	10 Sep 2009 15:04:23 -0000
@@ -355,7 +355,7 @@
     tmp1=$match[1]
     tpre=$match[2]
     while true; do
-      if [[ -d $donepath$tmp1 ]]; then
+      if [[ -d $prepath$realpath$donepath$tmp1 ]]; then
 	donepath=$donepath$tmp1/
 	pre=$tpre
 	break

Re: file completion of ~user/... stats wrong directory

[Bart Schaefer]

On Sep 10, 11:07am, Greg Klanderman wrote:
}
} Attached below is the one line fix.

I seem to recall having run into this before, attempting that same
change, and then finding that it breaks some OTHER more common case.

However, there are so many places like this in _path_files that I
may easily be confusing it with another "if" branch.

If I had copious free time, I'd rewrite _path_files from scratch.


} -      if [[ -d $donepath$tmp1 ]]; then
} +      if [[ -d $prepath$realpath$donepath$tmp1 ]]; then

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

>>>>> On September 10, 2009 Bart Schaefer <schaefer@brasslantern.com> wrote:

> I seem to recall having run into this before, attempting that same
> change, and then finding that it breaks some OTHER more common case.

I'm pretty sure this is right; every place $donepath (or $testpath) is
used it either needs $prepath$realpath or $linepath in front of it
depending on whether you're looking at file existence or adding a
completion.

greg

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

Hi Peter and Bart,

Do you guys need anything more from me on this to satisfy you
with the correctness of the patch I submitted?

thanks,
Greg

Re: file completion of ~user/... stats wrong directory

[Greg Klanderman]

thank you Peter!