_canonical_path not working on *BSD

_canonical_path not working on *BSD

[Baptiste Daroussin]

Hi,

_canonical_path is not working on freebsd, it rely on readlink which  
does work the same on BSD :
on openbsd -q options doesn't exist : umount[tab] gives readlink:  
unknown option -- q
on freebsd readlink: illegal option -- q

there is on patch on freebsd ports which uses /usr/sbin/stat that  
works but then the completion is buggy
http://www.freebsd.org/cgi/cvsweb.cgi/~checkout~/ports/shells/zsh/files/patch-Completion-Unix-Type-_canonical_paths?rev=1.1;content-type=text%2Fplain


umount /h[tab]
gives me umount /h/
then [tab] again gives me umount /h//

so currently the work around is to replace _canonical_path by compadd  
-a in _mount (udevordir) and it works

I can't find any clue to correct the bug myself.

Thanks
Bapt

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

Debian bugs (Re: _canonical_path not working on *BSD)

[Bart Schaefer]

On Mar 26, 11:44am, Baptiste Daroussin wrote:
}
} _canonical_path is not working on freebsd, it rely on readlink

Clint, et al., please don't take this the wrong way, but:  This is
the second path-resolution-related change made for Debian and then
committed into the zsh dist that has caused portability issues.  I
think we need to start actively seeking out cross-platform testers
before accepting completion patches via the Debian bugs list.

Patches either in C code or using only built-in zsh features are
probably still OK.  So would be changes that go only into the
Completion/Debian/ directory.

Re: Debian bugs (Re: _canonical_path not working on *BSD)

[Clint Adams]

On Wed, Mar 26, 2008 at 08:01:17AM -0700, Bart Schaefer wrote:
> Clint, et al., please don't take this the wrong way, but:  This is
> the second path-resolution-related change made for Debian and then
> committed into the zsh dist that has caused portability issues.  I
> think we need to start actively seeking out cross-platform testers
> before accepting completion patches via the Debian bugs list.

You're right; I should have spent more time reviewing.

Re: _canonical_path not working on *BSD

[Peter Stephenson]

On Wed, 26 Mar 2008 11:44:13 +0100
Baptiste Daroussin <baptiste.daroussin@gmail.com> wrote:
> Hi,
> 
> _canonical_path is not working on freebsd, it rely on readlink which  
> does work the same on BSD :
> on openbsd -q options doesn't exist : umount[tab] gives readlink:  
> unknown option -- q
> on freebsd readlink: illegal option -- q

Pea suggested just removing the -q, but probably we should also redirect
stderr:

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.1
diff -u -r1.1 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
+++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 15:05:05 -0000
@@ -38,7 +38,8 @@
   files=($@)
 else
   for __index in $@; do
-    files+=$(readlink -qf $__index)
+    # BSD doesn't have -q, so redirect stderr.
+    files+=$(readlink -f $__index 2>/dev/null)
   done
 fi
 
@@ -48,7 +49,7 @@
   expref=${~origpref}
   [[ $origpref == (|*/). ]] && rltrim=.
   curpref=${${expref%$rltrim}:-./}
-  canpref=$(readlink -qf $curpref)
+  canpref=$(readlink -f $curpref 2>/dev/null)
   if [[ $? -eq 0 ]]; then
     [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
     canpref+=$rltrim


-- 
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: _canonical_path not working on *BSD

[Baptiste Daroussin]

Quoting Peter Stephenson <pws@csr.com>:

>
> Pea suggested just removing the -q, but probably we should also redirect
> stderr:
>
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.1
> diff -u -r1.1 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
> +++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 15:05:05 -0000
> @@ -38,7 +38,8 @@
>    files=($@)
>  else
>    for __index in $@; do
> -    files+=$(readlink -qf $__index)
> +    # BSD doesn't have -q, so redirect stderr.
> +    files+=$(readlink -f $__index 2>/dev/null)
>    done
>  fi
>
> @@ -48,7 +49,7 @@
>    expref=${~origpref}
>    [[ $origpref == (|*/). ]] && rltrim=.
>    curpref=${${expref%$rltrim}:-./}
> -  canpref=$(readlink -qf $curpref)
> +  canpref=$(readlink -f $curpref 2>/dev/null)
>    if [[ $? -eq 0 ]]; then
>      [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
>      canpref+=$rltrim
>
>
> --
> 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
>

This only work on OpenBSD, readlink on FreeBSD doesn't allow -f.

here is the manpage on FreeBSD :  
http://www.freebsd.org/cgi/man.cgi?query=readlink&apropos=0&sektion=0&manpath=FreeBSD+6.3-RELEASE&format=html
it seems to be ok on NetBSD, but I don't have tried (the manpage says  
-f is ok for readlink)

Darwin should have the same problem, the man page is the same as the  
freebsd one :
http://developer.apple.com/documentation/Darwin/Reference/Manpages/man1/readlink.1.html

I will also see with the freebsd folks if it is not possible to add -f  
to readlink.

---
Baptiste

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

Re: _canonical_path not working on *BSD

[Peter Stephenson]

Baptiste Daroussin wrote:
> This only work on OpenBSD, readlink on FreeBSD doesn't allow -f.

I think that means the function won't do what it's designed to do,
right?  So in that case we should test if readlink accepts -f and if it
doesn't, then return immediately.  However, that may not be a trivial
test.  How about something like

[[ -n $(readlink -f / 2>&1 >/dev/null) ]]

(for which we could cache the result)?  Or is testing that
"readlink -f / >&/dev/null" returns status 0 good enough?

-- 
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: _canonical_path not working on *BSD

[Pea]

Le Wed, 26 Mar 2008 15:34:09 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> Baptiste Daroussin wrote:
> > This only work on OpenBSD, readlink on FreeBSD doesn't allow -f.
> 
> I think that means the function won't do what it's designed to do,
> right?  So in that case we should test if readlink accepts -f and if
> it doesn't, then return immediately.  However, that may not be a
> trivial test.  How about something like
> 
> [[ -n $(readlink -f / 2>&1 >/dev/null) ]]
> 
> (for which we could cache the result)?  Or is testing that
> "readlink -f / >&/dev/null" returns status 0 good enough?
> 

On OpenBSD:

[pea@coredump:~]% readlink -f / >&/dev/null ; echo $?
0

But readlink -f exists on netbsd only since release 4.0. Older release
doesn't have this option.

Regards.

Re: _canonical_path not working on *BSD

[Clint Adams]

On Wed, Mar 26, 2008 at 03:34:09PM +0000, Peter Stephenson wrote:
> I think that means the function won't do what it's designed to do,
> right?  So in that case we should test if readlink accepts -f and if it
> doesn't, then return immediately.  However, that may not be a trivial
> test.  How about something like

/bin/realpath on FreeBSD does follow symlinks.

Re: _canonical_path not working on *BSD

[Pea]

Le Wed, 26 Mar 2008 11:44:13 +0100,
Baptiste Daroussin <baptiste.daroussin@gmail.com> a écrit :

> Hi,
> 
> _canonical_path is not working on freebsd, it rely on readlink which  
> does work the same on BSD :
> on openbsd -q options doesn't exist : umount[tab] gives readlink:  
> unknown option -- q
> on freebsd readlink: illegal option -- q
> 
> there is on patch on freebsd ports which uses /usr/sbin/stat that  
> works but then the completion is buggy
> http://www.freebsd.org/cgi/cvsweb.cgi/~checkout~/ports/shells/zsh/files/patch-Completion-Unix-Type-_canonical_paths?rev=1.1;content-type=text%2Fplain
> 
> 
> umount /h[tab]
> gives me umount /h/
> then [tab] again gives me umount /h//
> 
> so currently the work around is to replace _canonical_path by
> compadd -a in _mount (udevordir) and it works
> 
> I can't find any clue to correct the bug myself.
> 
> Thanks
> Bapt
> 
> ----------------------------------------------------------------
> This message was sent using IMP, the Internet Messaging Program.
> 
> 
> 

Hi,

I confirm the error on OpenBSD.
The solution provided by baptiste works. 
Second solution (for OpenBSD), change the call to readlink -f (in
Completion/Unix/Type/_canonical_paths).

Regards.

Pea

Re: _canonical_path not working on *BSD

[Bart Schaefer]

On Mar 26, 11:44am, Baptiste Daroussin wrote:
}
} _canonical_path is not working on freebsd, it rely on readlink

Try this.  I don't actually have any mount points that are symlink
targets so it's hard to test, but I think this is right:

--- ../zsh-forge/current/Completion/Unix/Type/_canonical_paths  2006-05-28
11:36:06.000000000 -0700
+++ ../zsh-4.0/Completion/Unix/Type/_canonical_paths    2008-03-26
08:31:42.000000000 -0700
@@ -27,7 +27,7 @@
 
 shift 2
 
-if (( ! $+commands[readlink] )); then
+if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
   _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
   return ret
 fi
@@ -38,7 +38,7 @@
   files=($@)
 else
   for __index in $@; do
-    files+=$(readlink -qf $__index)
+    files+=$(zstat +link $__index)
   done
 fi
 
@@ -48,8 +48,8 @@
   expref=${~origpref}
   [[ $origpref == (|*/). ]] && rltrim=.
   curpref=${${expref%$rltrim}:-./}
-  canpref=$(readlink -qf $curpref)
-  if [[ $? -eq 0 ]]; then
+  canpref=$(zstat +link $curpref)
+  if [[ -n "$canpref" ]]; then
     [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
     canpref+=$rltrim
     [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/

Re: _canonical_path not working on *BSD

[Peter Stephenson]

Bart Schaefer wrote:
> On Mar 26, 11:44am, Baptiste Daroussin wrote:
> }
> } _canonical_path is not working on freebsd, it rely on readlink
> 
> Try this.  I don't actually have any mount points that are symlink
> targets so it's hard to test, but I think this is right:
> 
> -if (( ! $+commands[readlink] )); then
> +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then

That's probably a better solution; it doesn't necessarily work on all
systems, particularly if the shell is statically linked, but it should
be at least as widely applicable as, and easier to test for than,
anything involving readlink.

-- 
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: _canonical_path not working on *BSD

[Peter Stephenson]

Peter Stephenson wrote:
> Bart Schaefer wrote:
> > On Mar 26, 11:44am, Baptiste Daroussin wrote:
> > }
> > } _canonical_path is not working on freebsd, it rely on readlink
> > 
> > Try this.  I don't actually have any mount points that are symlink
> > targets so it's hard to test, but I think this is right:
> > 
> > -if (( ! $+commands[readlink] )); then
> > +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
> 
> That's probably a better solution; it doesn't necessarily work on all
> systems, particularly if the shell is statically linked, but it should
> be at least as widely applicable as, and easier to test for than,
> anything involving readlink.

Hmm... sorry about all the traffic... actually, it still doesn't
guarantee to give a canonical path as "readlink -f" does, since it
doesn't check if the value returned is itself a symbolic link, and also
it returns empty instead of the original file if it wasn't a link.  We would
need to do something like the following... if I've correctly divined
that the intention in both cases is that if a file exists at all we
should always use the name, but converted to the canonical form.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.1
diff -u -r1.1 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
+++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 15:58:52 -0000
@@ -27,18 +27,28 @@
 
 shift 2
 
-if (( ! $+commands[readlink] )); then
+if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
   _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
   return ret
 fi
 
+typeset addfile newfile
 typeset -a matches files
 
 if (( $__opts[(I)-N] )); then
   files=($@)
 else
   for __index in $@; do
-    files+=$(readlink -qf $__index)
+    addfile=$__index
+    while true; do
+      newfile=$(zstat +link $addfile)
+      if [[ -n $newfile ]]; then
+	addfile=$newfile
+      else
+	break
+      fi
+    done
+    files+=($addfile)
   done
 fi
 
@@ -48,8 +58,18 @@
   expref=${~origpref}
   [[ $origpref == (|*/). ]] && rltrim=.
   curpref=${${expref%$rltrim}:-./}
-  canpref=$(readlink -qf $curpref)
-  if [[ $? -eq 0 ]]; then
+  if zstat $curpref >&/dev/null; then
+    canpref=$curpref
+    while true; do
+      newfile=$(zstat +link $canpref)
+      if [[ -n $newfile ]]; then
+	canpref=$newfile
+      else
+	break
+      fi
+    done
+  fi
+  if [[ -n "$canpref" ]]; then
     [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
     canpref+=$rltrim
     [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/


-- 
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: _canonical_path not working on *BSD

[Bart Schaefer]

On Mar 26,  4:04pm, Peter Stephenson wrote:
}
} Hmm... sorry about all the traffic... actually, it still doesn't
} guarantee to give a canonical path as "readlink -f" does, since it
} doesn't check if the value returned is itself a symbolic link

Using a while-loop in shell code leads to a potentially infinite loop
on a symlink cycle.  I think readlink and realpath have some sort of
defense against that.

Re: _canonical_path not working on *BSD

[Peter Stephenson]

Peter Stephenson wrote:
> Hmm... sorry about all the traffic... actually, it still doesn't
> guarantee to give a canonical path as "readlink -f" does, since it
> doesn't check if the value returned is itself a symbolic link, and also
> it returns empty instead of the original file if it wasn't a link.  We would
> need to do something like the following... if I've correctly divined
> that the intention in both cases is that if a file exists at all we
> should always use the name, but converted to the canonical form.

Sigh.  Or even this.  This time, I've guarded against loops in the
symbolic links.

I'm still not quite sure what the test of the status from readlink in
the second case was for, seeing as "readlink -qf /nonexistent" returns
status 0.  I've interpreted it as a test for a file that actually
exists.  In the present context it may not matter.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.1
diff -u -r1.1 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
+++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 16:16:14 -0000
@@ -27,18 +27,38 @@
 
 shift 2
 
-if (( ! $+commands[readlink] )); then
+if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
   _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
   return ret
 fi
 
+typeset REPLY
 typeset -a matches files
 
+_canonical_paths_get_canonical_path() {
+  typeset newfile
+  typeset -A seen
+
+  REPLY=$1
+  # Guard against loops.
+  while [[ -z ${seen[$REPLY]} ]]; do
+    seen[$REPLY]=1
+    newfile=$(zstat +link $REPLY 2>/dev/null)
+    if [[ -n $newfile ]]; then
+      REPLY=$newfile
+    else
+      break
+    fi
+  done
+}
+
+
 if (( $__opts[(I)-N] )); then
   files=($@)
 else
   for __index in $@; do
-    files+=$(readlink -qf $__index)
+    _canonical_paths_get_canonical_path $__index
+    files+=($REPLY)
   done
 fi
 
@@ -48,8 +68,9 @@
   expref=${~origpref}
   [[ $origpref == (|*/). ]] && rltrim=.
   curpref=${${expref%$rltrim}:-./}
-  canpref=$(readlink -qf $curpref)
-  if [[ $? -eq 0 ]]; then
+  if zstat $curpref >&/dev/null; then
+    _canonical_paths_get_canonical_path $curpref
+    canpref=$REPLY
     [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
     canpref+=$rltrim
     [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/


-- 
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: _canonical_path not working on *BSD

[Pea]

Le Wed, 26 Mar 2008 16:21:51 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> Peter Stephenson wrote:
> > Hmm... sorry about all the traffic... actually, it still doesn't
> > guarantee to give a canonical path as "readlink -f" does, since it
> > doesn't check if the value returned is itself a symbolic link, and
> > also it returns empty instead of the original file if it wasn't a
> > link.  We would need to do something like the following... if I've
> > correctly divined that the intention in both cases is that if a
> > file exists at all we should always use the name, but converted to
> > the canonical form.
> 
> Sigh.  Or even this.  This time, I've guarded against loops in the
> symbolic links.
> 
> I'm still not quite sure what the test of the status from readlink in
> the second case was for, seeing as "readlink -qf /nonexistent" returns
> status 0.  I've interpreted it as a test for a file that actually
> exists.  In the present context it may not matter.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.1
> diff -u -r1.1 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06
> -0000	1.1 +++ Completion/Unix/Type/_canonical_paths	26
> Mar 2008 16:16:14 -0000 @@ -27,18 +27,38 @@
>  
>  shift 2
>  
> -if (( ! $+commands[readlink] )); then
> +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
>    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
>    return ret
>  fi
>  
> +typeset REPLY
>  typeset -a matches files
>  
> +_canonical_paths_get_canonical_path() {
> +  typeset newfile
> +  typeset -A seen
> +
> +  REPLY=$1
> +  # Guard against loops.
> +  while [[ -z ${seen[$REPLY]} ]]; do
> +    seen[$REPLY]=1
> +    newfile=$(zstat +link $REPLY 2>/dev/null)
> +    if [[ -n $newfile ]]; then
> +      REPLY=$newfile
> +    else
> +      break
> +    fi
> +  done
> +}
> +
> +
>  if (( $__opts[(I)-N] )); then
>    files=($@)
>  else
>    for __index in $@; do
> -    files+=$(readlink -qf $__index)
> +    _canonical_paths_get_canonical_path $__index
> +    files+=($REPLY)
>    done
>  fi
>  
> @@ -48,8 +68,9 @@
>    expref=${~origpref}
>    [[ $origpref == (|*/). ]] && rltrim=.
>    curpref=${${expref%$rltrim}:-./}
> -  canpref=$(readlink -qf $curpref)
> -  if [[ $? -eq 0 ]]; then
> +  if zstat $curpref >&/dev/null; then
> +    _canonical_paths_get_canonical_path $curpref
> +    canpref=$REPLY
>      [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
>      canpref+=$rltrim
>      [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> 
> 

This almost works:

umount [TAB] gives me:
[pea@coredump:~]% umount
             /            /dev/sd0a    /dev/svnd0a  /home/pea


umout /[TAB] gives me:
[pea@coredump:~]% umount /
/            /dev/sd0a    /dev/svnd0a  /home/pea


But umount /d[TAB] or umount /de[TAB] or umount /h[TAB] give me
nothing...
It worked with readlinks

Re: _canonical_path not working on *BSD

[Peter Stephenson]

On Wed, 26 Mar 2008 17:38:24 +0100
Pea <zsh@raveland.org> wrote:
> This almost works:
> 
> umount [TAB] gives me:
> [pea@coredump:~]% umount
>              /            /dev/sd0a    /dev/svnd0a  /home/pea
> 
> 
> umout /[TAB] gives me:
> [pea@coredump:~]% umount /
> /            /dev/sd0a    /dev/svnd0a  /home/pea
> 
> 
> But umount /d[TAB] or umount /de[TAB] or umount /h[TAB] give me
> nothing...
> It worked with readlinks

Thanks for trying...  looks like my assumption about the status test
was wrong.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.1
diff -u -r1.1 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
+++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 16:43:46 -0000
@@ -27,18 +27,38 @@
 
 shift 2
 
-if (( ! $+commands[readlink] )); then
+if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
   _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
   return ret
 fi
 
+typeset REPLY
 typeset -a matches files
 
+_canonical_paths_get_canonical_path() {
+  typeset newfile
+  typeset -A seen
+
+  REPLY=$1
+  # Guard against loops.
+  while [[ -z ${seen[$REPLY]} ]]; do
+    seen[$REPLY]=1
+    newfile=$(zstat +link $REPLY 2>/dev/null)
+    if [[ -n $newfile ]]; then
+      REPLY=$newfile
+    else
+      break
+    fi
+  done
+}
+
+
 if (( $__opts[(I)-N] )); then
   files=($@)
 else
   for __index in $@; do
-    files+=$(readlink -qf $__index)
+    _canonical_paths_get_canonical_path $__index
+    files+=($REPLY)
   done
 fi
 
@@ -48,13 +68,16 @@
   expref=${~origpref}
   [[ $origpref == (|*/). ]] && rltrim=.
   curpref=${${expref%$rltrim}:-./}
-  canpref=$(readlink -qf $curpref)
-  if [[ $? -eq 0 ]]; then
-    [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
-    canpref+=$rltrim
-    [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
-    matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
+  if zstat $curpref >&/dev/null; then
+    _canonical_paths_get_canonical_path $curpref
+    canpref=$REPLY
+  else
+    canpref=$curpref
   fi
+  [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
+  canpref+=$rltrim
+  [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
+  matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
   for subdir in $expref?*(@); do
     _canonical_paths_add_paths ${subdir/$expref/$origpref} add
   done


-- 
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: _canonical_path not working on *BSD

[Pea]

Le Wed, 26 Mar 2008 16:46:15 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> On Wed, 26 Mar 2008 17:38:24 +0100
> Pea <zsh@raveland.org> wrote:
> > This almost works:
> > 
> > umount [TAB] gives me:
> > [pea@coredump:~]% umount
> >              /            /dev/sd0a    /dev/svnd0a  /home/pea
> > 
> > 
> > umout /[TAB] gives me:
> > [pea@coredump:~]% umount /
> > /            /dev/sd0a    /dev/svnd0a  /home/pea
> > 
> > 
> > But umount /d[TAB] or umount /de[TAB] or umount /h[TAB] give me
> > nothing...
> > It worked with readlinks
> 
> Thanks for trying...  looks like my assumption about the status test
> was wrong.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.1
> diff -u -r1.1 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06
> -0000	1.1 +++ Completion/Unix/Type/_canonical_paths	26
> Mar 2008 16:43:46 -0000 @@ -27,18 +27,38 @@
>  
>  shift 2
>  
> -if (( ! $+commands[readlink] )); then
> +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
>    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
>    return ret
>  fi
>  
> +typeset REPLY
>  typeset -a matches files
>  
> +_canonical_paths_get_canonical_path() {
> +  typeset newfile
> +  typeset -A seen
> +
> +  REPLY=$1
> +  # Guard against loops.
> +  while [[ -z ${seen[$REPLY]} ]]; do
> +    seen[$REPLY]=1
> +    newfile=$(zstat +link $REPLY 2>/dev/null)
> +    if [[ -n $newfile ]]; then
> +      REPLY=$newfile
> +    else
> +      break
> +    fi
> +  done
> +}
> +
> +
>  if (( $__opts[(I)-N] )); then
>    files=($@)
>  else
>    for __index in $@; do
> -    files+=$(readlink -qf $__index)
> +    _canonical_paths_get_canonical_path $__index
> +    files+=($REPLY)
>    done
>  fi
>  
> @@ -48,13 +68,16 @@
>    expref=${~origpref}
>    [[ $origpref == (|*/). ]] && rltrim=.
>    curpref=${${expref%$rltrim}:-./}
> -  canpref=$(readlink -qf $curpref)
> -  if [[ $? -eq 0 ]]; then
> -    [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
> -    canpref+=$rltrim
> -    [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> -    matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
> +  if zstat $curpref >&/dev/null; then
> +    _canonical_paths_get_canonical_path $curpref
> +    canpref=$REPLY
> +  else
> +    canpref=$curpref
>    fi
> +  [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
> +  canpref+=$rltrim
> +  [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> +  matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
>    for subdir in $expref?*(@); do
>      _canonical_paths_add_paths ${subdir/$expref/$origpref} add
>    done
> 
> 

This patch works fine for me on OpenBSD !!
Thanks.
-- 
Pierre-Emmanuel André <pea at raveland.org>
GPG key: 0x7AE329DC

Re: _canonical_path not working on *BSD

[Baptiste Daroussin]

Quoting Peter Stephenson <pws@csr.com>:

> On Wed, 26 Mar 2008 17:38:24 +0100
> Pea <zsh@raveland.org> wrote:
>> This almost works:
>>
>> umount [TAB] gives me:
>> [pea@coredump:~]% umount
>>              /            /dev/sd0a    /dev/svnd0a  /home/pea
>>
>>
>> umout /[TAB] gives me:
>> [pea@coredump:~]% umount /
>> /            /dev/sd0a    /dev/svnd0a  /home/pea
>>
>>
>> But umount /d[TAB] or umount /de[TAB] or umount /h[TAB] give me
>> nothing...
>> It worked with readlinks
>
> Thanks for trying...  looks like my assumption about the status test
> was wrong.
>
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.1
> diff -u -r1.1 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06 -0000	1.1
> +++ Completion/Unix/Type/_canonical_paths	26 Mar 2008 16:43:46 -0000
> @@ -27,18 +27,38 @@
>
>  shift 2
>
> -if (( ! $+commands[readlink] )); then
> +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
>    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
>    return ret
>  fi
>
> +typeset REPLY
>  typeset -a matches files
>
> +_canonical_paths_get_canonical_path() {
> +  typeset newfile
> +  typeset -A seen
> +
> +  REPLY=$1
> +  # Guard against loops.
> +  while [[ -z ${seen[$REPLY]} ]]; do
> +    seen[$REPLY]=1
> +    newfile=$(zstat +link $REPLY 2>/dev/null)
> +    if [[ -n $newfile ]]; then
> +      REPLY=$newfile
> +    else
> +      break
> +    fi
> +  done
> +}
> +
> +
>  if (( $__opts[(I)-N] )); then
>    files=($@)
>  else
>    for __index in $@; do
> -    files+=$(readlink -qf $__index)
> +    _canonical_paths_get_canonical_path $__index
> +    files+=($REPLY)
>    done
>  fi
>
> @@ -48,13 +68,16 @@
>    expref=${~origpref}
>    [[ $origpref == (|*/). ]] && rltrim=.
>    curpref=${${expref%$rltrim}:-./}
> -  canpref=$(readlink -qf $curpref)
> -  if [[ $? -eq 0 ]]; then
> -    [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
> -    canpref+=$rltrim
> -    [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> -    matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
> +  if zstat $curpref >&/dev/null; then
> +    _canonical_paths_get_canonical_path $curpref
> +    canpref=$REPLY
> +  else
> +    canpref=$curpref
>    fi
> +  [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
> +  canpref+=$rltrim
> +  [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> +  matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
>    for subdir in $expref?*(@); do
>      _canonical_paths_add_paths ${subdir/$expref/$origpref} add
>    done
>
>
> --
> 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
>

Wonderful, this one works great on FreeBSD, hope it will be ok for others.

Thanks.
Baptiste.

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

Re: _canonical_path not working on *BSD

[Peter Stephenson]

On Wed, 26 Mar 2008 16:46:15 +0000
Peter Stephenson <pws@csr.com> wrote:
> +_canonical_paths_get_canonical_path() {
> +  typeset newfile
> +  typeset -A seen
> +
> +  REPLY=$1
> +  # Guard against loops.
> +  while [[ -z ${seen[$REPLY]} ]]; do
> +    seen[$REPLY]=1
> +    newfile=$(zstat +link $REPLY 2>/dev/null)
> +    if [[ -n $newfile ]]; then
> +      REPLY=$newfile
> +    else
> +      break
> +    fi
> +  done
> +}

I should make one more point about this before leaving the subject: this
doesn't do everything that "readlink -f" does, in particular it doesn't
remove .. path segments, strip multiple /'s or remove symbolic link
references in intervening directories.

Luckily, we have the technology, I think.

We don't need the $(...) for the zstat.  It would be quite nice to
be able to get the directory canonicalization without a fork, too.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.2
diff -u -r1.2 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	26 Mar 2008 17:22:25 -0000	1.2
+++ Completion/Unix/Type/_canonical_paths	27 Mar 2008 10:19:10 -0000
@@ -36,20 +36,41 @@
 typeset -a matches files
 
 _canonical_paths_get_canonical_path() {
-  typeset newfile
+  typeset newfile dir
   typeset -A seen
 
   REPLY=$1
-  # Guard against loops.
+  # Resolve any trailing symbolic links, guarding against loops.
   while [[ -z ${seen[$REPLY]} ]]; do
     seen[$REPLY]=1
-    newfile=$(zstat +link $REPLY 2>/dev/null)
-    if [[ -n $newfile ]]; then
-      REPLY=$newfile
+    newfile=()
+    zstat -A newfile +link $REPLY 2>/dev/null
+    if [[ -n $newfile[1] ]]; then
+      REPLY=$newfile[1]
     else
       break
     fi
   done
+
+  # Canonicalise the directory path.  We may not be able to
+  # do this if we can't read all components.
+  if [[ -d $REPLY ]]; then
+    dir="$(unfunction chpwd
+           setopt CHASE_LINKS
+           cd $REPLY 2>/dev/null && pwd)"
+    if [[ -n $dir ]]; then
+      REPLY=$dir
+    fi
+  elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
+    # Don't try this if there's a trailing slash or we're in
+    # the root directory.
+    dir="$(unfunction chpwd
+           setopt CHASE_LINKS
+           cd ${REPLY%/*} 2>/dev/null && pwd)"
+    if [[ -n $dir ]]; then
+      REPLY=$dir/${REPLY##*/}
+    fi
+  fi
 }
 
 

Re: _canonical_path not working on *BSD

[Pea]

Le Thu, 27 Mar 2008 10:23:25 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> On Wed, 26 Mar 2008 16:46:15 +0000
> Peter Stephenson <pws@csr.com> wrote:
> > +_canonical_paths_get_canonical_path() {
> > +  typeset newfile
> > +  typeset -A seen
> > +
> > +  REPLY=$1
> > +  # Guard against loops.
> > +  while [[ -z ${seen[$REPLY]} ]]; do
> > +    seen[$REPLY]=1
> > +    newfile=$(zstat +link $REPLY 2>/dev/null)
> > +    if [[ -n $newfile ]]; then
> > +      REPLY=$newfile
> > +    else
> > +      break
> > +    fi
> > +  done
> > +}
> 
> I should make one more point about this before leaving the subject:
> this doesn't do everything that "readlink -f" does, in particular it
> doesn't remove .. path segments, strip multiple /'s or remove
> symbolic link references in intervening directories.
> 
> Luckily, we have the technology, I think.
> 
> We don't need the $(...) for the zstat.  It would be quite nice to
> be able to get the directory canonicalization without a fork, too.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.2
> diff -u -r1.2 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	26 Mar 2008 17:22:25
> -0000	1.2 +++ Completion/Unix/Type/_canonical_paths	27
> Mar 2008 10:19:10 -0000 @@ -36,20 +36,41 @@
>  typeset -a matches files
>  
>  _canonical_paths_get_canonical_path() {
> -  typeset newfile
> +  typeset newfile dir
>    typeset -A seen
>  
>    REPLY=$1
> -  # Guard against loops.
> +  # Resolve any trailing symbolic links, guarding against loops.
>    while [[ -z ${seen[$REPLY]} ]]; do
>      seen[$REPLY]=1
> -    newfile=$(zstat +link $REPLY 2>/dev/null)
> -    if [[ -n $newfile ]]; then
> -      REPLY=$newfile
> +    newfile=()
> +    zstat -A newfile +link $REPLY 2>/dev/null
> +    if [[ -n $newfile[1] ]]; then
> +      REPLY=$newfile[1]
>      else
>        break
>      fi
>    done
> +
> +  # Canonicalise the directory path.  We may not be able to
> +  # do this if we can't read all components.
> +  if [[ -d $REPLY ]]; then
> +    dir="$(unfunction chpwd
> +           setopt CHASE_LINKS
> +           cd $REPLY 2>/dev/null && pwd)"
> +    if [[ -n $dir ]]; then
> +      REPLY=$dir
> +    fi
> +  elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
> +    # Don't try this if there's a trailing slash or we're in
> +    # the root directory.
> +    dir="$(unfunction chpwd
> +           setopt CHASE_LINKS
> +           cd ${REPLY%/*} 2>/dev/null && pwd)"
> +    if [[ -n $dir ]]; then
> +      REPLY=$dir/${REPLY##*/}
> +    fi
> +  fi
>  }
>  
>  
> 

Doesn't work on OpenBSD (-current).
umount [TAB] gives me:

[pea@pea-dsktp:~]% umount _canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
_canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
_canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
_canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
_canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
_canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
                   umount                                                             
             /            /dev/svnd0c  /dev/wd0a    /home/pea



-- 
Pierre-Emmanuel André <pea at raveland.org>
GPG key: 0x7AE329DC

Re: _canonical_path not working on *BSD

[Peter Stephenson]

On Thu, 27 Mar 2008 12:08:07 +0100
Pea <zsh@raveland.org> wrote:
> [pea@pea-dsktp:~]% umount _canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd
> _canonical_paths_get_canonical_path:unfunction:1: no such hash table element: chpwd

I thought unfunction was silent if a function didn't exist, as is now
true of (and required for) unset, but apparently not.

Since I can't seem to get away from this, I've rewritten the function
more canonically so that the helper functions only get loaded once
and aren't buried in the body of the main function that gets stored.
Most of the patch is consequent rearrangement.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.3
diff -u -r1.3 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	27 Mar 2008 10:35:21 -0000	1.3
+++ Completion/Unix/Type/_canonical_paths	27 Mar 2008 11:20:34 -0000
@@ -13,27 +13,13 @@
 # case they are already so. `tag' and `desc' arguments are well, obvious :) In
 # addition, the options -M, -J, -V, -1, -2, -n, -F, -X are passed to compadd.
 
-local __index
-typeset -a __gopts __opts
-
-zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
-
-: ${1:=canonical-paths} ${2:=path}
-
-__index=$__opts[(I)-A]
-(( $__index )) && set -- $@ ${(P)__opts[__index+1]}
-
-local expl ret=1 tag=$1 desc=$2
-
-shift 2
-
-if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
-  _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
-  return ret
-fi
-
-typeset REPLY
-typeset -a matches files
+_canonical_paths_pwd() {
+  # Get the canonical directory name by changing to it.
+  # To be run in a subshell.
+  (( ${+functions[chpwd]} )) && unfunction chpwd
+  setopt CHASE_LINKS
+  cd $1 2>/dev/null && pwd
+}
 
 _canonical_paths_get_canonical_path() {
   typeset newfile dir
@@ -55,34 +41,20 @@
   # Canonicalise the directory path.  We may not be able to
   # do this if we can't read all components.
   if [[ -d $REPLY ]]; then
-    dir="$(unfunction chpwd
-           setopt CHASE_LINKS
-           cd $REPLY 2>/dev/null && pwd)"
+    dir="$(_canonical_paths_pwd $REPLY)"
     if [[ -n $dir ]]; then
       REPLY=$dir
     fi
   elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
     # Don't try this if there's a trailing slash or we're in
     # the root directory.
-    dir="$(unfunction chpwd
-           setopt CHASE_LINKS
-           cd ${REPLY%/*} 2>/dev/null && pwd)"
+    dir="$(_canonical_paths_pwd ${REPLY%/*})"
     if [[ -n $dir ]]; then
       REPLY=$dir/${REPLY##*/}
     fi
   fi
 }
 
-
-if (( $__opts[(I)-N] )); then
-  files=($@)
-else
-  for __index in $@; do
-    _canonical_paths_get_canonical_path $__index
-    files+=($REPLY)
-  done
-fi
-
 _canonical_paths_add_paths () {
   local origpref=$1 expref rltrim curpref canpref subdir
   [[ $2 != add ]] && matches=()
@@ -104,35 +76,70 @@
   done
 }
 
-local base=$PREFIX
-typeset -i blimit
+_canonical_paths() {
+  local __index
+  typeset -a __gopts __opts
+
+  zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
+
+  : ${1:=canonical-paths} ${2:=path}
+
+  __index=$__opts[(I)-A]
+  (( $__index )) && set -- $@ ${(P)__opts[__index+1]}
 
-_canonical_paths_add_paths $base
+  local expl ret=1 tag=$1 desc=$2
 
-if [[ -z $base ]]; then
-  _canonical_paths_add_paths / add
-elif [[ $base == ..(/.(|.))#(|/) ]]; then
-
-  # This style controls how many parent directory links (..) to chase searching
-  # for possible completions. The default is 8. Note that this chasing is
-  # triggered only when the user enters atleast a .. and the path completed
-  # contains only . or .. components. A value of 0 turns off .. link chasing
-  # altogether.
-
-  zstyle -s ":completion:${curcontext}:$tag" \
-    canonical-paths-back-limit blimit || blimit=8
-
-  if [[ $base != */ ]]; then
-    [[ $base != *.. ]] && base+=.
-    base+=/
+  shift 2
+
+  if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
+    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
+    return ret
   fi
-  until [[ $base.. -ef $base || blimit -le 0 ]]; do
-    base+=../
-    _canonical_paths_add_paths $base add
-    blimit+=-1
-  done
-fi
 
-_wanted "$tag" expl "$desc" compadd $__gopts -Q -U -a matches && ret=0
+  typeset REPLY
+  typeset -a matches files
+
+  if (( $__opts[(I)-N] )); then
+    files=($@)
+  else
+    for __index in $@; do
+      _canonical_paths_get_canonical_path $__index
+      files+=($REPLY)
+    done
+  fi
+
+  local base=$PREFIX
+  typeset -i blimit
+
+  _canonical_paths_add_paths $base
+
+  if [[ -z $base ]]; then
+    _canonical_paths_add_paths / add
+  elif [[ $base == ..(/.(|.))#(|/) ]]; then
+
+    # This style controls how many parent directory links (..) to chase searching
+    # for possible completions. The default is 8. Note that this chasing is
+    # triggered only when the user enters atleast a .. and the path completed
+    # contains only . or .. components. A value of 0 turns off .. link chasing
+    # altogether.
+
+    zstyle -s ":completion:${curcontext}:$tag" \
+      canonical-paths-back-limit blimit || blimit=8
+
+    if [[ $base != */ ]]; then
+      [[ $base != *.. ]] && base+=.
+      base+=/
+    fi
+    until [[ $base.. -ef $base || blimit -le 0 ]]; do
+      base+=../
+      _canonical_paths_add_paths $base add
+      blimit+=-1
+    done
+  fi
+
+  _wanted "$tag" expl "$desc" compadd $__gopts -Q -U -a matches && ret=0
+
+  return ret
+}
 
-return ret
+_canonical_paths "$@"

-- 
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

PATCH: cd -q (was Re: _canonical_path ...)

[Peter Stephenson]

On Thu, 27 Mar 2008 11:25:06 +0000
Peter Stephenson <pws@csr.com> wrote:
> I thought unfunction was silent if a function didn't exist, as is now
> true of (and required for) unset, but apparently not.
> +_canonical_paths_pwd() {
> +  # Get the canonical directory name by changing to it.
> +  # To be run in a subshell.
> +  (( ${+functions[chpwd]} )) && unfunction chpwd
> +  setopt CHASE_LINKS
> +  cd $1 2>/dev/null && pwd
> +}

The more I think about this (and I'd really like to stop now), the more
wrong it seems to me that it's so hard to get the shell to cd without
side effects.  Indeed, the patch above is incomplete since it omits
the new chpwd_functions array, and I haven't work out a foolproof
way of suppressing the effect of chpwd without forking the shell.
chpwd will typically print a bogus directory to the terminal title
bar, which is to be avoided; but it can in principle do anything
and if we're not sticking around in the directory the interactive
environment really doesn't need to care about the cd.

Any comments about the following proposal, which adds the -q option
to all relevant functions?  If it looks OK, I'll change the function
above to unsetopt PUSHD_NO_DUPS and use pushd -q/popd -q to sanitize
the directory with REPLY=$PWD and remove the fork.

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.103
diff -u -r1.103 builtins.yo
--- Doc/Zsh/builtins.yo	1 Feb 2008 19:59:45 -0000	1.103
+++ Doc/Zsh/builtins.yo	27 Mar 2008 11:49:30 -0000
@@ -152,9 +152,9 @@
 module(cap)(zsh/cap)
 findex(cd)
 cindex(directories, changing)
-xitem(tt(cd) [ tt(-sLP) ] [ var(arg) ])
-xitem(tt(cd) [ tt(-sLP) ] var(old) var(new))
-item(tt(cd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
+xitem(tt(cd) [ tt(-qsLP) ] [ var(arg) ])
+xitem(tt(cd) [ tt(-qsLP) ] var(old) var(new))
+item(tt(cd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
 Change the current directory.  In the first form, change the
 current directory to var(arg), or to the value of tt($HOME) if
 var(arg) is not specified.  If var(arg) is `tt(-)', change to the
@@ -189,6 +189,11 @@
 If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())'
 and `tt(-)' in this context are swapped.
 
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt(chpwd_functions) are not called.
+This is useful for calls to tt(cd) that do not change the environment
+seen by an interactive user.
+
 If the tt(-s) option is specified, tt(cd) refuses to change the current
 directory if the given pathname contains symlinks.  If the tt(-P) option
 is given or the tt(CHASE_LINKS) option is set, symbolic links are resolved
@@ -795,7 +800,7 @@
 )
 prefix(noglob)
 findex(popd)
-item(tt(popd) [ {tt(PLUS())|tt(-)}var(n) ])(
+item(tt(popd) [ [-q] {tt(PLUS())|tt(-)}var(n) ])(
 Remove an entry from the directory stack, and perform a tt(cd) to
 the new top directory.  With no argument, the current top entry is
 removed.  An argument of the form `tt(PLUS())var(n)' identifies a stack
@@ -804,6 +809,11 @@
 pindex(PUSHD_MINUS, use of)
 If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())' and
 `tt(-)' in this context are swapped.
+
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt($chpwd_functions) are not called,
+and the new directory stack is not printed.  This is useful for calls to
+tt(popd) that do not change the environment seen by an interactive user.
 )
 findex(print)
 xitem(tt(print) [ tt(-abcDilmnNoOpPrsz) ] [ tt(-u) var(n) ] [ tt(-f) var(format) ] [ tt(-C) var(cols) ])
@@ -935,9 +945,9 @@
 pindex(PUSHD_MINUS, use of)
 pindex(CDABLE_VARS, use of)
 pindex(PUSHD_SILENT, use of)
-xitem(tt(pushd) [ tt(-sLP) ] [ var(arg) ])
-xitem(tt(pushd) [ tt(-sLP) ] var(old) var(new))
-item(tt(pushd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
+xitem(tt(pushd) [ tt(-qsLP) ] [ var(arg) ])
+xitem(tt(pushd) [ tt(-qsLP) ] var(old) var(new))
+item(tt(pushd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
 Change the current directory, and push the old current directory
 onto the directory stack.  In the first form, change the
 current directory to var(arg).
@@ -956,8 +966,14 @@
 from the right.  If the tt(PUSHD_MINUS) option is set, the meanings
 of `tt(PLUS())' and `tt(-)' in this context are swapped.
 
-If the option tt(PUSHD_SILENT) is not set, the directory
-stack will be printed after a tt(pushd) is performed.
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt($chpwd_functions) are not called,
+and the new directory stack is not printed.  This is useful for calls to
+tt(pushd) that do not change the environment seen by an interactive user.
+
+If the option tt(-q) is not specified and the shell option tt(PUSHD_SILENT)
+is not set, the directory stack will be printed after a tt(pushd) is
+performed.
 
 The options tt(-s), tt(-L) and tt(-P) have the same meanings as for the
 tt(cd) builtin.
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.188
diff -u -r1.188 builtin.c
--- Src/builtin.c	2 Mar 2008 21:21:53 -0000	1.188
+++ Src/builtin.c	27 Mar 2008 11:49:31 -0000
@@ -50,8 +50,8 @@
     BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
     BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK, NULL, NULL),
     BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
-    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
-    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
+    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
+    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
     BUILTIN("continue", BINF_PSPECIAL, bin_break, 0, 1, BIN_CONTINUE, NULL, NULL),
     BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL, bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klmprtuxz", NULL),
     BUILTIN("dirs", 0, bin_dirs, 0, -1, 0, "clpv", NULL),
@@ -98,10 +98,10 @@
     BUILTIN("patdebug", 0, bin_patdebug, 1, -1, 0, "p", NULL),
 #endif
 
-    BUILTIN("popd", 0, bin_cd, 0, 1, BIN_POPD, NULL, NULL),
+    BUILTIN("popd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 1, BIN_POPD, "q", NULL),
     BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, "abcC:Df:ilmnNoOpPrRsu:z-", NULL),
     BUILTIN("printf", 0, bin_print, 1, -1, BIN_PRINTF, NULL, NULL),
-    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "sPL", NULL),
+    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "qsPL", NULL),
     BUILTIN("pushln", 0, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
     BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL),
     BUILTIN("r", 0, bin_fc, 0, -1, BIN_R, "nrl", NULL),
@@ -788,7 +788,7 @@
 	unqueue_signals();
 	return 1;
     }
-    cd_new_pwd(func, dir);
+    cd_new_pwd(func, dir, OPT_ISSET(ops, 'q'));
 
     if (stat(unmeta(pwd), &st1) < 0) {
 	setjobpwd();
@@ -1087,7 +1087,7 @@
 
 /**/
 static void
-cd_new_pwd(int func, LinkNode dir)
+cd_new_pwd(int func, LinkNode dir, int quiet)
 {
     char *new_pwd, *s;
     int dirstacksize;
@@ -1127,7 +1127,7 @@
 
     if (isset(INTERACTIVE)) {
 	if (func != BIN_CD) {
-            if (unset(PUSHDSILENT))
+            if (unset(PUSHDSILENT) && !quiet)
 	        printdirstack();
         } else if (doprintdir) {
 	    fprintdir(pwd, stdout);
@@ -1138,7 +1138,8 @@
     /* execute the chpwd function */
     fflush(stdout);
     fflush(stderr);
-    callhookfunc("chpwd", NULL, 1);
+    if (!quiet)
+	callhookfunc("chpwd", NULL, 1);
 
     dirstacksize = getiparam("DIRSTACKSIZE");
     /* handle directory stack sizes out of range */

-- 
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: PATCH: cd -q (was Re: _canonical_path ...)

[Stephane Chazelas]

On Thu, Mar 27, 2008 at 12:15:25PM +0000, Peter Stephenson wrote:
> On Thu, 27 Mar 2008 11:25:06 +0000
> Peter Stephenson <pws@csr.com> wrote:
> > I thought unfunction was silent if a function didn't exist, as is now
> > true of (and required for) unset, but apparently not.
> > +_canonical_paths_pwd() {
> > +  # Get the canonical directory name by changing to it.
> > +  # To be run in a subshell.
> > +  (( ${+functions[chpwd]} )) && unfunction chpwd
> > +  setopt CHASE_LINKS
> > +  cd $1 2>/dev/null && pwd
> > +}
[...]

You need cd -- "$1"
you might also want cd -P.

With "emulate sh", cd -P -- "$1" && pwd -P should return an
absolute path.

-- 
Stéphane

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Mikael Magnusson]

On 27/03/2008, Peter Stephenson <pws@csr.com> wrote:
>  The more I think about this (and I'd really like to stop now), the more
>  wrong it seems to me that it's so hard to get the shell to cd without
>  side effects.  Indeed, the patch above is incomplete since it omits
>  the new chpwd_functions array, and I haven't work out a foolproof
>  way of suppressing the effect of chpwd without forking the shell.
>  chpwd will typically print a bogus directory to the terminal title
>  bar, which is to be avoided; but it can in principle do anything
>  and if we're not sticking around in the directory the interactive
>  environment really doesn't need to care about the cd.
>
>  Any comments about the following proposal, which adds the -q option
>  to all relevant functions?  If it looks OK, I'll change the function
>  above to unsetopt PUSHD_NO_DUPS and use pushd -q/popd -q to sanitize
>  the directory with REPLY=$PWD and remove the fork.

Nice, I've wanted this on several occasions. Especially since
sometimes my chpwd function breaks constructs like (cd /foo; tar c .)
| tar x. I worked around that by printing everything to stderr instead
but it seems icky.

Also, I realized I didn't know what the existing options -sLP did, so
I looked at the manpage and
1) there's a typo near the beginning
Otherwise, if var(arg) begins with a slash, attempt to change to the
director<missing a y here> given by var(arg).
2)
    If the -P option is given or the CHASE_LINKS option is set,
symbolic links are resolved to their true  values.   If the -L option
is given symbolic links are followed regardless of the state of the
CHASE_LINKS option.
Intuitively this paragraph doesn't make any sense. Both options seem
to do the same thing? (-P || CHASE_LINKS) || -L == -P || -L ||
CHASE_LINKS

-- 
Mikael Magnusson

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Peter Stephenson]

"Mikael Magnusson" wrote:
>     If the -P option is given or the CHASE_LINKS option is set,
> symbolic links are resolved to their true  values.   If the -L option
> is given symbolic links are followed regardless of the state of the
> CHASE_LINKS option.
> Intuitively this paragraph doesn't make any sense. Both options seem
> to do the same thing? (-P || CHASE_LINKS) || -L == -P || -L ||
> CHASE_LINKS

-L is the opposite of the -P (logical rather than physical).  "Followed"
 is a little unclear; it should be something like "retained", unless
someone can express it better.

-- 
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: PATCH: cd -q (was Re: _canonical_path ...)

[Mikael Magnusson]

On 27/03/2008, Peter Stephenson <pws@csr.com> wrote:
> "Mikael Magnusson" wrote:
>  >     If the -P option is given or the CHASE_LINKS option is set,
>  > symbolic links are resolved to their true  values.   If the -L option
>  > is given symbolic links are followed regardless of the state of the
>  > CHASE_LINKS option.
>  > Intuitively this paragraph doesn't make any sense. Both options seem
>  > to do the same thing? (-P || CHASE_LINKS) || -L == -P || -L ||
>  > CHASE_LINKS
>
>
> -L is the opposite of the -P (logical rather than physical).  "Followed"
>   is a little unclear; it should be something like "retained", unless
>  someone can express it better.

I see. "follow" is probably the worst possible word to use for this
given for example this entry in the man page for "cp"
       -L, --dereference
              always follow symbolic links in SOURCE
I would suggest changing to "symbolic links are not resolved
regardless" or even "symbolic links are not followed regardless" :).
-- 
Mikael Magnusson

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Peter Stephenson]

On Thu, 27 Mar 2008 12:15:25 +0000
Peter Stephenson <pws@csr.com> wrote:
> Any comments about the following proposal, which adds the -q option
> to all relevant functions?  If it looks OK, I'll change the function
> above to unsetopt PUSHD_NO_DUPS and use pushd -q/popd -q to sanitize
> the directory with REPLY=$PWD and remove the fork.

Here is what I'm likely to commit.  It includes various related
suggestions.  It doesn't include a new style for the canonical stuff.

Index: Completion/Unix/Type/_canonical_paths
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
retrieving revision 1.4
diff -u -r1.4 _canonical_paths
--- Completion/Unix/Type/_canonical_paths	27 Mar 2008 11:29:20 -0000	1.4
+++ Completion/Unix/Type/_canonical_paths	27 Mar 2008 18:42:17 -0000
@@ -15,42 +15,45 @@
 
 _canonical_paths_pwd() {
   # Get the canonical directory name by changing to it.
-  # To be run in a subshell.
-  (( ${+functions[chpwd]} )) && unfunction chpwd
-  setopt CHASE_LINKS
-  cd $1 2>/dev/null && pwd
+  integer chaselinks
+  [[ -o chaselinks ]] && (( chaselinks = 1 ))
+  setopt localoptions nopushdignoredups chaselinks
+  if builtin pushd -q -- $1 2>/dev/null; then
+    REPLY=$PWD
+    (( chaselinks )) || unsetopt chaselinks
+    builtin popd -q
+  else
+    REPLY=$1
+  fi
 }
 
 _canonical_paths_get_canonical_path() {
-  typeset newfile dir
+  typeset newfile nondir
   typeset -A seen
 
   REPLY=$1
-  # Resolve any trailing symbolic links, guarding against loops.
-  while [[ -z ${seen[$REPLY]} ]]; do
-    seen[$REPLY]=1
-    newfile=()
-    zstat -A newfile +link $REPLY 2>/dev/null
-    if [[ -n $newfile[1] ]]; then
-      REPLY=$newfile[1]
-    else
-      break
-    fi
-  done
-
   # Canonicalise the directory path.  We may not be able to
   # do this if we can't read all components.
   if [[ -d $REPLY ]]; then
-    dir="$(_canonical_paths_pwd $REPLY)"
-    if [[ -n $dir ]]; then
-      REPLY=$dir
-    fi
-  elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
-    # Don't try this if there's a trailing slash or we're in
-    # the root directory.
-    dir="$(_canonical_paths_pwd ${REPLY%/*})"
-    if [[ -n $dir ]]; then
-      REPLY=$dir/${REPLY##*/}
+    _canonical_paths_pwd $REPLY
+  else
+    # Resolve any trailing symbolic links, guarding against loops.
+    while [[ -z ${seen[$REPLY]} ]]; do
+      seen[$REPLY]=1
+      newfile=()
+      zstat -A newfile +link $REPLY 2>/dev/null
+      if [[ -n $newfile[1] ]]; then
+	REPLY=$newfile[1]
+      else
+	break
+      fi
+    done
+    if [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
+      # Don't try this if there's a trailing slash or we're in
+      # the root directory.
+      nondir=${REPLY##*/#}
+      _canonical_paths_pwd ${REPLY%/#*}
+      REPLY+="/$nondir"
     fi
   fi
 }
Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.103
diff -u -r1.103 builtins.yo
--- Doc/Zsh/builtins.yo	1 Feb 2008 19:59:45 -0000	1.103
+++ Doc/Zsh/builtins.yo	27 Mar 2008 18:42:17 -0000
@@ -152,16 +152,16 @@
 module(cap)(zsh/cap)
 findex(cd)
 cindex(directories, changing)
-xitem(tt(cd) [ tt(-sLP) ] [ var(arg) ])
-xitem(tt(cd) [ tt(-sLP) ] var(old) var(new))
-item(tt(cd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
+xitem(tt(cd) [ tt(-qsLP) ] [ var(arg) ])
+xitem(tt(cd) [ tt(-qsLP) ] var(old) var(new))
+item(tt(cd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
 Change the current directory.  In the first form, change the
 current directory to var(arg), or to the value of tt($HOME) if
 var(arg) is not specified.  If var(arg) is `tt(-)', change to the
 value of tt($OLDPWD), the previous directory.
 
 Otherwise, if var(arg) begins with a slash, attempt to change to the
-director given by var(arg).
+directory given by var(arg).
 
 If var(arg) does not begin with a slash, the behaviour depends on whether
 the current directory `tt(.)' occurs in the list of directories contained
@@ -189,11 +189,17 @@
 If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())'
 and `tt(-)' in this context are swapped.
 
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt(chpwd_functions) are not called.
+This is useful for calls to tt(cd) that do not change the environment
+seen by an interactive user.
+
 If the tt(-s) option is specified, tt(cd) refuses to change the current
 directory if the given pathname contains symlinks.  If the tt(-P) option
 is given or the tt(CHASE_LINKS) option is set, symbolic links are resolved
 to their true values.  If the tt(-L) option is given symbolic links are
-followed regardless of the state of the tt(CHASE_LINKS) option.
+retained in the directory (and not resolved) regardless of the state of
+the tt(CHASE_LINKS) option.
 )
 alias(chdir)(cd)
 module(clone)(zsh/clone)
@@ -795,7 +801,7 @@
 )
 prefix(noglob)
 findex(popd)
-item(tt(popd) [ {tt(PLUS())|tt(-)}var(n) ])(
+item(tt(popd) [ [-q] {tt(PLUS())|tt(-)}var(n) ])(
 Remove an entry from the directory stack, and perform a tt(cd) to
 the new top directory.  With no argument, the current top entry is
 removed.  An argument of the form `tt(PLUS())var(n)' identifies a stack
@@ -804,6 +810,11 @@
 pindex(PUSHD_MINUS, use of)
 If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())' and
 `tt(-)' in this context are swapped.
+
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt($chpwd_functions) are not called,
+and the new directory stack is not printed.  This is useful for calls to
+tt(popd) that do not change the environment seen by an interactive user.
 )
 findex(print)
 xitem(tt(print) [ tt(-abcDilmnNoOpPrsz) ] [ tt(-u) var(n) ] [ tt(-f) var(format) ] [ tt(-C) var(cols) ])
@@ -935,9 +946,9 @@
 pindex(PUSHD_MINUS, use of)
 pindex(CDABLE_VARS, use of)
 pindex(PUSHD_SILENT, use of)
-xitem(tt(pushd) [ tt(-sLP) ] [ var(arg) ])
-xitem(tt(pushd) [ tt(-sLP) ] var(old) var(new))
-item(tt(pushd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
+xitem(tt(pushd) [ tt(-qsLP) ] [ var(arg) ])
+xitem(tt(pushd) [ tt(-qsLP) ] var(old) var(new))
+item(tt(pushd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
 Change the current directory, and push the old current directory
 onto the directory stack.  In the first form, change the
 current directory to var(arg).
@@ -956,8 +967,14 @@
 from the right.  If the tt(PUSHD_MINUS) option is set, the meanings
 of `tt(PLUS())' and `tt(-)' in this context are swapped.
 
-If the option tt(PUSHD_SILENT) is not set, the directory
-stack will be printed after a tt(pushd) is performed.
+If the tt(-q) (quiet) option is specified, the hook function tt(chpwd)
+and the functions in the array tt($chpwd_functions) are not called,
+and the new directory stack is not printed.  This is useful for calls to
+tt(pushd) that do not change the environment seen by an interactive user.
+
+If the option tt(-q) is not specified and the shell option tt(PUSHD_SILENT)
+is not set, the directory stack will be printed after a tt(pushd) is
+performed.
 
 The options tt(-s), tt(-L) and tt(-P) have the same meanings as for the
 tt(cd) builtin.
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.188
diff -u -r1.188 builtin.c
--- Src/builtin.c	2 Mar 2008 21:21:53 -0000	1.188
+++ Src/builtin.c	27 Mar 2008 18:42:18 -0000
@@ -50,8 +50,8 @@
     BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
     BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK, NULL, NULL),
     BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
-    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
-    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
+    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
+    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
     BUILTIN("continue", BINF_PSPECIAL, bin_break, 0, 1, BIN_CONTINUE, NULL, NULL),
     BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL, bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klmprtuxz", NULL),
     BUILTIN("dirs", 0, bin_dirs, 0, -1, 0, "clpv", NULL),
@@ -98,10 +98,10 @@
     BUILTIN("patdebug", 0, bin_patdebug, 1, -1, 0, "p", NULL),
 #endif
 
-    BUILTIN("popd", 0, bin_cd, 0, 1, BIN_POPD, NULL, NULL),
+    BUILTIN("popd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 1, BIN_POPD, "q", NULL),
     BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, "abcC:Df:ilmnNoOpPrRsu:z-", NULL),
     BUILTIN("printf", 0, bin_print, 1, -1, BIN_PRINTF, NULL, NULL),
-    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "sPL", NULL),
+    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "qsPL", NULL),
     BUILTIN("pushln", 0, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
     BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL),
     BUILTIN("r", 0, bin_fc, 0, -1, BIN_R, "nrl", NULL),
@@ -788,7 +788,7 @@
 	unqueue_signals();
 	return 1;
     }
-    cd_new_pwd(func, dir);
+    cd_new_pwd(func, dir, OPT_ISSET(ops, 'q'));
 
     if (stat(unmeta(pwd), &st1) < 0) {
 	setjobpwd();
@@ -1087,7 +1087,7 @@
 
 /**/
 static void
-cd_new_pwd(int func, LinkNode dir)
+cd_new_pwd(int func, LinkNode dir, int quiet)
 {
     char *new_pwd, *s;
     int dirstacksize;
@@ -1127,7 +1127,7 @@
 
     if (isset(INTERACTIVE)) {
 	if (func != BIN_CD) {
-            if (unset(PUSHDSILENT))
+            if (unset(PUSHDSILENT) && !quiet)
 	        printdirstack();
         } else if (doprintdir) {
 	    fprintdir(pwd, stdout);
@@ -1138,7 +1138,8 @@
     /* execute the chpwd function */
     fflush(stdout);
     fflush(stderr);
-    callhookfunc("chpwd", NULL, 1);
+    if (!quiet)
+	callhookfunc("chpwd", NULL, 1);
 
     dirstacksize = getiparam("DIRSTACKSIZE");
     /* handle directory stack sizes out of range */

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Pea]

Le Thu, 27 Mar 2008 18:45:02 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> On Thu, 27 Mar 2008 12:15:25 +0000
> Peter Stephenson <pws@csr.com> wrote:
> > Any comments about the following proposal, which adds the -q option
> > to all relevant functions?  If it looks OK, I'll change the function
> > above to unsetopt PUSHD_NO_DUPS and use pushd -q/popd -q to sanitize
> > the directory with REPLY=$PWD and remove the fork.
> 
> Here is what I'm likely to commit.  It includes various related
> suggestions.  It doesn't include a new style for the canonical stuff.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.4
> diff -u -r1.4 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	27 Mar 2008 11:29:20
> -0000	1.4 +++ Completion/Unix/Type/_canonical_paths	27
> Mar 2008 18:42:17 -0000 @@ -15,42 +15,45 @@
>  
>  _canonical_paths_pwd() {
>    # Get the canonical directory name by changing to it.
> -  # To be run in a subshell.
> -  (( ${+functions[chpwd]} )) && unfunction chpwd
> -  setopt CHASE_LINKS
> -  cd $1 2>/dev/null && pwd
> +  integer chaselinks
> +  [[ -o chaselinks ]] && (( chaselinks = 1 ))
> +  setopt localoptions nopushdignoredups chaselinks
> +  if builtin pushd -q -- $1 2>/dev/null; then
> +    REPLY=$PWD
> +    (( chaselinks )) || unsetopt chaselinks
> +    builtin popd -q
> +  else
> +    REPLY=$1
> +  fi
>  }
>  
>  _canonical_paths_get_canonical_path() {
> -  typeset newfile dir
> +  typeset newfile nondir
>    typeset -A seen
>  
>    REPLY=$1
> -  # Resolve any trailing symbolic links, guarding against loops.
> -  while [[ -z ${seen[$REPLY]} ]]; do
> -    seen[$REPLY]=1
> -    newfile=()
> -    zstat -A newfile +link $REPLY 2>/dev/null
> -    if [[ -n $newfile[1] ]]; then
> -      REPLY=$newfile[1]
> -    else
> -      break
> -    fi
> -  done
> -
>    # Canonicalise the directory path.  We may not be able to
>    # do this if we can't read all components.
>    if [[ -d $REPLY ]]; then
> -    dir="$(_canonical_paths_pwd $REPLY)"
> -    if [[ -n $dir ]]; then
> -      REPLY=$dir
> -    fi
> -  elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
> -    # Don't try this if there's a trailing slash or we're in
> -    # the root directory.
> -    dir="$(_canonical_paths_pwd ${REPLY%/*})"
> -    if [[ -n $dir ]]; then
> -      REPLY=$dir/${REPLY##*/}
> +    _canonical_paths_pwd $REPLY
> +  else
> +    # Resolve any trailing symbolic links, guarding against loops.
> +    while [[ -z ${seen[$REPLY]} ]]; do
> +      seen[$REPLY]=1
> +      newfile=()
> +      zstat -A newfile +link $REPLY 2>/dev/null
> +      if [[ -n $newfile[1] ]]; then
> +	REPLY=$newfile[1]
> +      else
> +	break
> +      fi
> +    done
> +    if [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
> +      # Don't try this if there's a trailing slash or we're in
> +      # the root directory.
> +      nondir=${REPLY##*/#}
> +      _canonical_paths_pwd ${REPLY%/#*}
> +      REPLY+="/$nondir"
>      fi
>    fi
>  }
> Index: Doc/Zsh/builtins.yo
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
> retrieving revision 1.103
> diff -u -r1.103 builtins.yo
> --- Doc/Zsh/builtins.yo	1 Feb 2008 19:59:45 -0000	1.103
> +++ Doc/Zsh/builtins.yo	27 Mar 2008 18:42:17 -0000
> @@ -152,16 +152,16 @@
>  module(cap)(zsh/cap)
>  findex(cd)
>  cindex(directories, changing)
> -xitem(tt(cd) [ tt(-sLP) ] [ var(arg) ])
> -xitem(tt(cd) [ tt(-sLP) ] var(old) var(new))
> -item(tt(cd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
> +xitem(tt(cd) [ tt(-qsLP) ] [ var(arg) ])
> +xitem(tt(cd) [ tt(-qsLP) ] var(old) var(new))
> +item(tt(cd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
>  Change the current directory.  In the first form, change the
>  current directory to var(arg), or to the value of tt($HOME) if
>  var(arg) is not specified.  If var(arg) is `tt(-)', change to the
>  value of tt($OLDPWD), the previous directory.
>  
>  Otherwise, if var(arg) begins with a slash, attempt to change to the
> -director given by var(arg).
> +directory given by var(arg).
>  
>  If var(arg) does not begin with a slash, the behaviour depends on
> whether the current directory `tt(.)' occurs in the list of
> directories contained @@ -189,11 +189,17 @@
>  If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())'
>  and `tt(-)' in this context are swapped.
>  
> +If the tt(-q) (quiet) option is specified, the hook function
> tt(chpwd) +and the functions in the array tt(chpwd_functions) are not
> called. +This is useful for calls to tt(cd) that do not change the
> environment +seen by an interactive user.
> +
>  If the tt(-s) option is specified, tt(cd) refuses to change the
> current directory if the given pathname contains symlinks.  If the
> tt(-P) option is given or the tt(CHASE_LINKS) option is set, symbolic
> links are resolved to their true values.  If the tt(-L) option is
> given symbolic links are -followed regardless of the state of the
> tt(CHASE_LINKS) option. +retained in the directory (and not resolved)
> regardless of the state of +the tt(CHASE_LINKS) option.
>  )
>  alias(chdir)(cd)
>  module(clone)(zsh/clone)
> @@ -795,7 +801,7 @@
>  )
>  prefix(noglob)
>  findex(popd)
> -item(tt(popd) [ {tt(PLUS())|tt(-)}var(n) ])(
> +item(tt(popd) [ [-q] {tt(PLUS())|tt(-)}var(n) ])(
>  Remove an entry from the directory stack, and perform a tt(cd) to
>  the new top directory.  With no argument, the current top entry is
>  removed.  An argument of the form `tt(PLUS())var(n)' identifies a
> stack @@ -804,6 +810,11 @@
>  pindex(PUSHD_MINUS, use of)
>  If the tt(PUSHD_MINUS) option is set, the meanings of `tt(PLUS())'
> and `tt(-)' in this context are swapped.
> +
> +If the tt(-q) (quiet) option is specified, the hook function
> tt(chpwd) +and the functions in the array tt($chpwd_functions) are
> not called, +and the new directory stack is not printed.  This is
> useful for calls to +tt(popd) that do not change the environment seen
> by an interactive user. )
>  findex(print)
>  xitem(tt(print) [ tt(-abcDilmnNoOpPrsz) ] [ tt(-u) var(n) ] [ tt(-f)
> var(format) ] [ tt(-C) var(cols) ]) @@ -935,9 +946,9 @@
>  pindex(PUSHD_MINUS, use of)
>  pindex(CDABLE_VARS, use of)
>  pindex(PUSHD_SILENT, use of)
> -xitem(tt(pushd) [ tt(-sLP) ] [ var(arg) ])
> -xitem(tt(pushd) [ tt(-sLP) ] var(old) var(new))
> -item(tt(pushd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
> +xitem(tt(pushd) [ tt(-qsLP) ] [ var(arg) ])
> +xitem(tt(pushd) [ tt(-qsLP) ] var(old) var(new))
> +item(tt(pushd) [ tt(-qsLP) ] {tt(PLUS())|tt(-)}var(n))(
>  Change the current directory, and push the old current directory
>  onto the directory stack.  In the first form, change the
>  current directory to var(arg).
> @@ -956,8 +967,14 @@
>  from the right.  If the tt(PUSHD_MINUS) option is set, the meanings
>  of `tt(PLUS())' and `tt(-)' in this context are swapped.
>  
> -If the option tt(PUSHD_SILENT) is not set, the directory
> -stack will be printed after a tt(pushd) is performed.
> +If the tt(-q) (quiet) option is specified, the hook function
> tt(chpwd) +and the functions in the array tt($chpwd_functions) are
> not called, +and the new directory stack is not printed.  This is
> useful for calls to +tt(pushd) that do not change the environment
> seen by an interactive user. +
> +If the option tt(-q) is not specified and the shell option
> tt(PUSHD_SILENT) +is not set, the directory stack will be printed
> after a tt(pushd) is +performed.
>  
>  The options tt(-s), tt(-L) and tt(-P) have the same meanings as for
> the tt(cd) builtin.
> Index: Src/builtin.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
> retrieving revision 1.188
> diff -u -r1.188 builtin.c
> --- Src/builtin.c	2 Mar 2008 21:21:53 -0000	1.188
> +++ Src/builtin.c	27 Mar 2008 18:42:18 -0000
> @@ -50,8 +50,8 @@
>      BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
>      BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK,
> NULL, NULL), BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
> -    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
> -    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
> +    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
> +    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
> BUILTIN("continue", BINF_PSPECIAL, bin_break, 0, 1, BIN_CONTINUE,
> NULL, NULL), BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS |
> BINF_PSPECIAL, bin_typeset, 0, -1, 0,
> "AE:%F:%HL:%R:%TUZ:%afghi:%klmprtuxz", NULL), BUILTIN("dirs", 0,
> bin_dirs, 0, -1, 0, "clpv", NULL), @@ -98,10 +98,10 @@
> BUILTIN("patdebug", 0, bin_patdebug, 1, -1, 0, "p", NULL), #endif 
> -    BUILTIN("popd", 0, bin_cd, 0, 1, BIN_POPD, NULL, NULL),
> +    BUILTIN("popd", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 1, BIN_POPD, "q", NULL),
> BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT,
> "abcC:Df:ilmnNoOpPrRsu:z-", NULL), BUILTIN("printf", 0, bin_print, 1,
> -1, BIN_PRINTF, NULL, NULL),
> -    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "sPL", NULL),
> +    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH |
> BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "qsPL", NULL),
> BUILTIN("pushln", 0, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
> BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL), BUILTIN("r", 0,
> bin_fc, 0, -1, BIN_R, "nrl", NULL), @@ -788,7 +788,7 @@
>  	unqueue_signals();
>  	return 1;
>      }
> -    cd_new_pwd(func, dir);
> +    cd_new_pwd(func, dir, OPT_ISSET(ops, 'q'));
>  
>      if (stat(unmeta(pwd), &st1) < 0) {
>  	setjobpwd();
> @@ -1087,7 +1087,7 @@
>  
>  /**/
>  static void
> -cd_new_pwd(int func, LinkNode dir)
> +cd_new_pwd(int func, LinkNode dir, int quiet)
>  {
>      char *new_pwd, *s;
>      int dirstacksize;
> @@ -1127,7 +1127,7 @@
>  
>      if (isset(INTERACTIVE)) {
>  	if (func != BIN_CD) {
> -            if (unset(PUSHDSILENT))
> +            if (unset(PUSHDSILENT) && !quiet)
>  	        printdirstack();
>          } else if (doprintdir) {
>  	    fprintdir(pwd, stdout);
> @@ -1138,7 +1138,8 @@
>      /* execute the chpwd function */
>      fflush(stdout);
>      fflush(stderr);
> -    callhookfunc("chpwd", NULL, 1);
> +    if (!quiet)
> +	callhookfunc("chpwd", NULL, 1);
>  
>      dirstacksize = getiparam("DIRSTACKSIZE");
>      /* handle directory stack sizes out of range */
> 

Hi,

Builds and works fine on OpenBSD.
umount [TAB] works correctly.

Regards,


-- 
Pierre-Emmanuel André <pea at raveland.org>
GPG key: 0x7AE329DC

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Mikael Magnusson]

On 27/03/2008, Peter Stephenson <pws@csr.com> wrote:
> On Thu, 27 Mar 2008 12:15:25 +0000
>
> Peter Stephenson <pws@csr.com> wrote:
>
> > Any comments about the following proposal, which adds the -q option
>  > to all relevant functions?  If it looks OK, I'll change the function
>  > above to unsetopt PUSHD_NO_DUPS and use pushd -q/popd -q to sanitize
>  > the directory with REPLY=$PWD and remove the fork.
>
>
> Here is what I'm likely to commit.  It includes various related
>  suggestions.  It doesn't include a new style for the canonical stuff.

I couldn't help but notice _cd doesn't complete directories after -q
(or -L or -P or -s), nor the options themselves. I took a look at it,
but it was special enough that I have no idea how to fix it in under 3
hours :).

-- 
Mikael Magnusson

Re: PATCH: cd -q (was Re: _canonical_path ...)

[Peter Stephenson]

On Fri, 28 Mar 2008 12:01:15 +0100
"Mikael Magnusson" <mikachu@gmail.com> wrote:
> I couldn't help but notice _cd doesn't complete directories after -q
> (or -L or -P or -s)

Yes, this is a long-standing bug and it's easy enough to teach it to
ignore options (remembering that -<-> is not an option).

> nor the options themselves.

This is less than easy enough to get working normally because we don't use
_arguments.  Probably the best thing to do is to use _arguments at the top
level and dispatch to the current code for first or second arguments.
Until then I've made it complete options only after a -, where it also
completes directory stack entries.  Since completing directory stack
entries is much more useful (at least with verbose listing) and the vast
majority of the time the options are just in the way I've added a style to
suppress option completions.  Normally this would be done with tags but in
this case I think we need them separate by default.  This all seems to make
rather heavy weather of something fairly simple.

Index: Completion/Zsh/Command/_cd
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Zsh/Command/_cd,v
retrieving revision 1.7
diff -u -r1.7 _cd
--- Completion/Zsh/Command/_cd	3 Sep 2003 14:07:26 -0000	1.7
+++ Completion/Zsh/Command/_cd	28 Mar 2008 14:33:02 -0000
@@ -11,17 +11,37 @@
 #    it's not a lot of use.  If you don't type the + or - it will
 #    complete directories as normal.
 
+_cd_options() {
+  _arguments -s \
+  '-q[Quiet, no output or use of hooks]' \
+  '-s[Refuse to use paths with symlinks]' \
+  '(-P)-L[Retain symbolic links ignoring CHASE_LINKS]' \
+  '(-L)-P[Resolve symbolic links as CHASE_LINKS]'
+}
+
 setopt localoptions nonomatch
 
-local expl ret=1
+local expl ret=1 curarg
+integer argstart=2 noopts
+
+if (( CURRENT > 1 )); then
+  # if not in command position, may have options.
+  # Careful: -<-> is not an option.
+  while [[ $words[$argstart] = -* && argstart -lt CURRENT ]]; do
+    curarg=$words[$argstart]
+    [[ $curarg = -<-> ]] && break
+    (( argstart++ ))
+    [[ $curarg = -- ]] && noopts=1 && break
+  done
+fi
 
-if [[ CURRENT -eq 3 ]]; then
+if [[ CURRENT -eq $((argstart+1)) ]]; then
   # cd old new: look for old in $PWD and see what can replace it
   local rep
   # Get possible completions using word in position 2
-  rep=(${~PWD/$words[2]/*}~$PWD(-/))
+  rep=(${~PWD/$words[$argstart]/*}~$PWD(-/))
   # Now remove all the common parts of $PWD and the completions from this
-  rep=(${${rep#${PWD%%$words[2]*}}%${PWD#*$words[2]}})
+  rep=(${${rep#${PWD%%$words[$argstart]*}}%${PWD#*$words[$argstart]}})
   (( $#rep )) && _wanted -C replacement strings expl replacement compadd -a rep
 else
   # Complete directory stack entries with ~ or when not in command position
@@ -70,6 +90,11 @@
     [[ CURRENT -ne 1 || ( -z "$path[(r).]" && $PREFIX != */* ) ]] &&
         alt=( "${cdpath+local-}directories:${cdpath+local }directory:_path_files -/" "$alt[@]" )
 
+    if [[ CURRENT -eq argstart && noopts -eq 0 && $PREFIX = -* ]] &&
+      zstyle -t ":completion:${curcontext}:options" complete-options; then
+      alt=("$service-options:$service option:_cd_options" "$alt[@]")
+    fi
+
     _alternative "$alt[@]" && ret=0
 
     return ret
Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.205
diff -u -r1.205 compsys.yo
--- Doc/Zsh/compsys.yo	28 Feb 2008 18:29:05 -0000	1.205
+++ Doc/Zsh/compsys.yo	28 Mar 2008 14:33:05 -0000
@@ -1221,6 +1221,16 @@
 line is not the name of an alias, matching alias names will be
 completed.
 )
+kindex(complete-options, completion style)
+time(tt(complete-options))(
+This is used by the completer for tt(cd), tt(chdir) and tt(pushd).
+For these commands a tt(-) is used to introduce a directory stack entry
+and completion of these is far more common than completing options.
+Hence unless the value of this style is true options will not be
+completed, even after an initial tt(-).  If it is true, options will
+be completed after an initial tt(-) unless there is a preceeding
+tt(--) on the command line.
+)
 kindex(completer, completion style)
 item(tt(completer))(
 The strings given as the value of this style provide the names of the


-- 
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: _canonical_path not working on *BSD

[Pea]

Le Thu, 27 Mar 2008 11:25:06 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> On Thu, 27 Mar 2008 12:08:07 +0100
> Pea <zsh@raveland.org> wrote:
> > [pea@pea-dsktp:~]% umount
> > _canonical_paths_get_canonical_path:unfunction:1: no such hash
> > table element: chpwd
> > _canonical_paths_get_canonical_path:unfunction:1: no such hash
> > table element: chpwd
> 
> I thought unfunction was silent if a function didn't exist, as is now
> true of (and required for) unset, but apparently not.
> 
> Since I can't seem to get away from this, I've rewritten the function
> more canonically so that the helper functions only get loaded once
> and aren't buried in the body of the main function that gets stored.
> Most of the patch is consequent rearrangement.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.3
> diff -u -r1.3 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	27 Mar 2008 10:35:21
> -0000	1.3 +++ Completion/Unix/Type/_canonical_paths	27
> Mar 2008 11:20:34 -0000 @@ -13,27 +13,13 @@
>  # case they are already so. `tag' and `desc' arguments are well,
> obvious :) In # addition, the options -M, -J, -V, -1, -2, -n, -F, -X
> are passed to compadd. 
> -local __index
> -typeset -a __gopts __opts
> -
> -zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
> -
> -: ${1:=canonical-paths} ${2:=path}
> -
> -__index=$__opts[(I)-A]
> -(( $__index )) && set -- $@ ${(P)__opts[__index+1]}
> -
> -local expl ret=1 tag=$1 desc=$2
> -
> -shift 2
> -
> -if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
> -  _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
> -  return ret
> -fi
> -
> -typeset REPLY
> -typeset -a matches files
> +_canonical_paths_pwd() {
> +  # Get the canonical directory name by changing to it.
> +  # To be run in a subshell.
> +  (( ${+functions[chpwd]} )) && unfunction chpwd
> +  setopt CHASE_LINKS
> +  cd $1 2>/dev/null && pwd
> +}
>  
>  _canonical_paths_get_canonical_path() {
>    typeset newfile dir
> @@ -55,34 +41,20 @@
>    # Canonicalise the directory path.  We may not be able to
>    # do this if we can't read all components.
>    if [[ -d $REPLY ]]; then
> -    dir="$(unfunction chpwd
> -           setopt CHASE_LINKS
> -           cd $REPLY 2>/dev/null && pwd)"
> +    dir="$(_canonical_paths_pwd $REPLY)"
>      if [[ -n $dir ]]; then
>        REPLY=$dir
>      fi
>    elif [[ $REPLY = */*[^/] && $REPLY != /[^/]# ]]; then
>      # Don't try this if there's a trailing slash or we're in
>      # the root directory.
> -    dir="$(unfunction chpwd
> -           setopt CHASE_LINKS
> -           cd ${REPLY%/*} 2>/dev/null && pwd)"
> +    dir="$(_canonical_paths_pwd ${REPLY%/*})"
>      if [[ -n $dir ]]; then
>        REPLY=$dir/${REPLY##*/}
>      fi
>    fi
>  }
>  
> -
> -if (( $__opts[(I)-N] )); then
> -  files=($@)
> -else
> -  for __index in $@; do
> -    _canonical_paths_get_canonical_path $__index
> -    files+=($REPLY)
> -  done
> -fi
> -
>  _canonical_paths_add_paths () {
>    local origpref=$1 expref rltrim curpref canpref subdir
>    [[ $2 != add ]] && matches=()
> @@ -104,35 +76,70 @@
>    done
>  }
>  
> -local base=$PREFIX
> -typeset -i blimit
> +_canonical_paths() {
> +  local __index
> +  typeset -a __gopts __opts
> +
> +  zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
> +
> +  : ${1:=canonical-paths} ${2:=path}
> +
> +  __index=$__opts[(I)-A]
> +  (( $__index )) && set -- $@ ${(P)__opts[__index+1]}
>  
> -_canonical_paths_add_paths $base
> +  local expl ret=1 tag=$1 desc=$2
>  
> -if [[ -z $base ]]; then
> -  _canonical_paths_add_paths / add
> -elif [[ $base == ..(/.(|.))#(|/) ]]; then
> -
> -  # This style controls how many parent directory links (..) to
> chase searching
> -  # for possible completions. The default is 8. Note that this
> chasing is
> -  # triggered only when the user enters atleast a .. and the path
> completed
> -  # contains only . or .. components. A value of 0 turns off .. link
> chasing
> -  # altogether.
> -
> -  zstyle -s ":completion:${curcontext}:$tag" \
> -    canonical-paths-back-limit blimit || blimit=8
> -
> -  if [[ $base != */ ]]; then
> -    [[ $base != *.. ]] && base+=.
> -    base+=/
> +  shift 2
> +
> +  if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
> +    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
> +    return ret
>    fi
> -  until [[ $base.. -ef $base || blimit -le 0 ]]; do
> -    base+=../
> -    _canonical_paths_add_paths $base add
> -    blimit+=-1
> -  done
> -fi
>  
> -_wanted "$tag" expl "$desc" compadd $__gopts -Q -U -a matches &&
> ret=0
> +  typeset REPLY
> +  typeset -a matches files
> +
> +  if (( $__opts[(I)-N] )); then
> +    files=($@)
> +  else
> +    for __index in $@; do
> +      _canonical_paths_get_canonical_path $__index
> +      files+=($REPLY)
> +    done
> +  fi
> +
> +  local base=$PREFIX
> +  typeset -i blimit
> +
> +  _canonical_paths_add_paths $base
> +
> +  if [[ -z $base ]]; then
> +    _canonical_paths_add_paths / add
> +  elif [[ $base == ..(/.(|.))#(|/) ]]; then
> +
> +    # This style controls how many parent directory links (..) to
> chase searching
> +    # for possible completions. The default is 8. Note that this
> chasing is
> +    # triggered only when the user enters atleast a .. and the path
> completed
> +    # contains only . or .. components. A value of 0 turns off ..
> link chasing
> +    # altogether.
> +
> +    zstyle -s ":completion:${curcontext}:$tag" \
> +      canonical-paths-back-limit blimit || blimit=8
> +
> +    if [[ $base != */ ]]; then
> +      [[ $base != *.. ]] && base+=.
> +      base+=/
> +    fi
> +    until [[ $base.. -ef $base || blimit -le 0 ]]; do
> +      base+=../
> +      _canonical_paths_add_paths $base add
> +      blimit+=-1
> +    done
> +  fi
> +
> +  _wanted "$tag" expl "$desc" compadd $__gopts -Q -U -a matches &&
> ret=0 +
> +  return ret
> +}
>  
> -return ret
> +_canonical_paths "$@"
> 

This works fine on OpenBSD.

Regards,

-- 
Pierre-Emmanuel André <pea at raveland.org>
GPG key: 0x7AE329DC

Re: _canonical_path not working on *BSD

[Bart Schaefer]

On Mar 27, 10:23am, Peter Stephenson wrote:
}
} I should make one more point about this before leaving the subject: this
} doesn't do everything that "readlink -f" does, in particular it doesn't
} remove .. path segments, strip multiple /'s or remove symbolic link
} references in intervening directories.
} 
} Luckily, we have the technology, I think.
} 
} We don't need the $(...) for the zstat.  It would be quite nice to
} be able to get the directory canonicalization without a fork, too.

Why do we even need the zstat if we're doing $(cd ...; pwd) ?  I used
zstat in the first place to avoid having to cd to a directory that might
not be readable/executable (though I suppose "umount" would normally be
used by root) but if you're going to cd anyway then you can skip the
whole "while" loop on zstat and just "pwd -P" once.

Or better, try the "pwd -P" first and fall back on the while loop.

(Or did your most recent patch do that?  I've lost track.)

I'm beginning to think this whole exercise is a bad idea anyway.  I have
visions of sysadmins attempting to umount a misbehaving NFS server or a
failing disk and ending up with their session frozen as zsh attempts to
access the bad filesystem behind the scenes.

Re: _canonical_path not working on *BSD

[Peter Stephenson]

Bart Schaefer wrote:
> Why do we even need the zstat if we're doing $(cd ...; pwd) ?

If the last segment is a directory we don't.  The function doesn't
actually say it needs to be, however.  I can move the -d test to
optimise this.

> I'm beginning to think this whole exercise is a bad idea anyway.  I have
> visions of sysadmins attempting to umount a misbehaving NFS server or a
> failing disk and ending up with their session frozen as zsh attempts to
> access the bad filesystem behind the scenes.

I think that's a generic problem with completing mount points, but it
would be perfectly reasonable to put the behaviour in a style.

-- 
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: _canonical_path not working on *BSD

[Bart Schaefer]

On Mar 27,  6:06pm, Peter Stephenson wrote:
}
} Bart Schaefer wrote:
} > Why do we even need the zstat if we're doing $(cd ...; pwd) ?
} 
} If the last segment is a directory we don't.

When is a mount point *not* a directory?

Re: _canonical_path not working on *BSD

[Mikael Magnusson]

On 28/03/2008, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mar 27,  6:06pm, Peter Stephenson wrote:
>  }
>  } Bart Schaefer wrote:
>  } > Why do we even need the zstat if we're doing $(cd ...; pwd) ?
>  }
>  } If the last segment is a directory we don't.
>
>
> When is a mount point *not* a directory?

On linux you've been able to mount --bind files on top of eachother
for quite some time, just as with directories.

-- 
Mikael Magnusson

Re: _canonical_path not working on *BSD

[Peter Stephenson]

Bart Schaefer wrote:
> On Mar 27,  6:06pm, Peter Stephenson wrote:
> }
> } Bart Schaefer wrote:
> } > Why do we even need the zstat if we're doing $(cd ...; pwd) ?
> } 
> } If the last segment is a directory we don't.
> 
> When is a mount point *not* a directory?

_canonical_paths isn't tied to mount points, although it's just used for
those at the moment.

I've committed the change and also an initial release note for 4.3.6:

Index: Etc/relnotes_4.3.6.txt
===================================================================
RCS file: Etc/relnotes_4.3.6.txt
diff -N Etc/relnotes_4.3.6.txt
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ Etc/relnotes_4.3.6.txt      28 Mar 2008 09:59:08 -0000      1.1
@@ -0,0 +1,19 @@
+Version 4.3.6 contains mostly bugfixes, but there are some small
+improvements.  No incompatibilities with previous versions are known.
+
+Visible changes in the shell and its modules since 4.3.5 include the
+following:
+
+The parameter subscripting flag "e", which existed but had limited
+usefulness, has been extended to allow reverse matching of strings instead
+of patterns.  For example, "${array[(ie)*]}" substitutes the index of the
+array element that contains the exact string "*".  In previous versions of
+the shell a fairly hairy process was necessary to ensure pattern characters
+were quoted.
+
+The cd, chdir, pushd and popd builtins now take the option -q (quiet) which
+avoids side effects when changing directories, suppressing the effect of
+the chpwd function, the chpwd_functions array and printing of the directory
+stack.  The last was already possible with the option PUSHD_SILENT, but in
+previous versions of the shell there was no easy way of suppressing the
+other side effects.

-- 
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: _canonical_path not working on *BSD

[Pea]

Le Wed, 26 Mar 2008 16:04:01 +0000,
Peter Stephenson <pws@csr.com> a écrit :

> Peter Stephenson wrote:
> > Bart Schaefer wrote:
> > > On Mar 26, 11:44am, Baptiste Daroussin wrote:
> > > }
> > > } _canonical_path is not working on freebsd, it rely on readlink
> > > 
> > > Try this.  I don't actually have any mount points that are symlink
> > > targets so it's hard to test, but I think this is right:
> > > 
> > > -if (( ! $+commands[readlink] )); then
> > > +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
> > 
> > That's probably a better solution; it doesn't necessarily work on
> > all systems, particularly if the shell is statically linked, but it
> > should be at least as widely applicable as, and easier to test for
> > than, anything involving readlink.
> 
> Hmm... sorry about all the traffic... actually, it still doesn't
> guarantee to give a canonical path as "readlink -f" does, since it
> doesn't check if the value returned is itself a symbolic link, and
> also it returns empty instead of the original file if it wasn't a
> link.  We would need to do something like the following... if I've
> correctly divined that the intention in both cases is that if a file
> exists at all we should always use the name, but converted to the
> canonical form.
> 
> Index: Completion/Unix/Type/_canonical_paths
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Completion/Unix/Type/_canonical_paths,v
> retrieving revision 1.1
> diff -u -r1.1 _canonical_paths
> --- Completion/Unix/Type/_canonical_paths	28 May 2006 18:36:06
> -0000	1.1 +++ Completion/Unix/Type/_canonical_paths	26
> Mar 2008 15:58:52 -0000 @@ -27,18 +27,28 @@
>  
>  shift 2
>  
> -if (( ! $+commands[readlink] )); then
> +if ! zmodload -F zsh/stat b:zstat 2>/dev/null; then
>    _wanted "$tag" expl "$desc" compadd $__gopts $@ && ret=0
>    return ret
>  fi
>  
> +typeset addfile newfile
>  typeset -a matches files
>  
>  if (( $__opts[(I)-N] )); then
>    files=($@)
>  else
>    for __index in $@; do
> -    files+=$(readlink -qf $__index)
> +    addfile=$__index
> +    while true; do
> +      newfile=$(zstat +link $addfile)
> +      if [[ -n $newfile ]]; then
> +	addfile=$newfile
> +      else
> +	break
> +      fi
> +    done
> +    files+=($addfile)
>    done
>  fi
>  
> @@ -48,8 +58,18 @@
>    expref=${~origpref}
>    [[ $origpref == (|*/). ]] && rltrim=.
>    curpref=${${expref%$rltrim}:-./}
> -  canpref=$(readlink -qf $curpref)
> -  if [[ $? -eq 0 ]]; then
> +  if zstat $curpref >&/dev/null; then
> +    canpref=$curpref
> +    while true; do
> +      newfile=$(zstat +link $canpref)
> +      if [[ -n $newfile ]]; then
> +	canpref=$newfile
> +      else
> +	break
> +      fi
> +    done
> +  fi
> +  if [[ -n "$canpref" ]]; then
>      [[ $curpref == */ && $canpref == *[^/] ]] && canpref+=/
>      canpref+=$rltrim
>      [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/
> 
> 

This patch doesn't work on OpenBSD (-current @i386).

umount [TAB] gives me:

[pea@coredump:~]% umount _canonical_paths_add_paths:zstat:1:
usr/src/sys: no such file or directory
_canonical_paths_add_paths:zstat:1: usr/src/sys: no such file or
directory 
	umount
             /            /dev/sd0a    /dev/svnd0a  /home/pea