[ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Clint Adams]

I assume that this is a quoting issue again.

----- Forwarded message from Jay Berkenbilt <ejb@ql.org> -----

Date: Mon, 12 Apr 2004 15:59:30 -0400
From: Jay Berkenbilt <ejb@ql.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: Bug#243385: zsh: rsync completion expands * too soon

Package: zsh
Version: 4.1.1-14
Severity: normal
Tags: patch

(Unless zsh-workers has been reconfigured since my last bug report,
the X-Debbugs-CC to zsh-workers will fail and this bug will not have
been reported upstream.)

This is essentially identical to what I reported in Debian bug 239070
for scp: the same bug existed with rsync's completion function.  The
attached patch removes "_call_program files" from the ssh line used to
generate remote files and also adds the -L flag to ls, just like my
earlier patch to _ssh.  I have dispensed with a lengthy explanation
because it is essentially identical to the one I included in bug
239070, the fix to which appears to have been accepted upstream.

This patch is to /usr/share/zsh/4.1.1/functions/Completion/Unix/_rsync.

-------8<--------
--- _rsync~	2004-03-25 13:29:33.000000000 -0500
+++ _rsync	2004-04-09 18:14:08.000000000 -0400
@@ -37,7 +37,7 @@
 
   if zstyle -T ":completion:${curcontext}:" remote-access; then
     slash=/
-    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1F ${${${words[CURRENT
+    remfiles=(${(f)"$(ssh -a -x ${words[CURRENT]%:*} ls -d1FL ${${${words[CURRENT
 ]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null)"})
 
     remdispf=(${remfiles:#*/})
-------8<--------

This patch causes rsync completion to work properly for remote files
when you have ssh access to the remote system.  It still doesn't
behave quite as well as scp completion (rsync host:/path/ TAB doesn't
show alternatives unless you type at least one character after the /),
but at least it behaves as well as it did before the _call_program
function call was added.

A grep for '_call_program files ssh' suggests that this may be the
only place left that has this particular bug.  Too bad I didn't think
to do that last time. :-)

Please let me know if additional information is required or if I
should have repeated the information to substantiate this patch.

-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.4.25-1-686-smp
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8

Versions of packages zsh depends on:
ii  debconf                     1.4.16       Debian configuration management sy
ii  libc6                       2.3.2.ds1-11 GNU C Library: Shared libraries an
ii  libcap1                     1:1.10-13    support for getting/setting POSIX.
ii  libncurses5                 5.4-2        Shared libraries for terminal hand
ii  passwd                      1:4.0.3-21   Change and administer password and

-- debconf information:
  zsh/rcmove: 

--- _rsync~	2004-03-25 13:29:33.000000000 -0500
+++ _rsync	2004-04-09 18:14:08.000000000 -0400
@@ -37,7 +37,7 @@
 
   if zstyle -T ":completion:${curcontext}:" remote-access; then
     slash=/
-    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1F ${${${words[CURRENT
+    remfiles=(${(f)"$(ssh -a -x ${words[CURRENT]%:*} ls -d1FL ${${${words[CURRENT
 ]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null)"})
 
     remdispf=(${remfiles:#*/})


----- End forwarded message -----

Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Jay Berkenbilt]

>   I assume that this is a quoting issue again.

Yes.  Well, kind of.  It's an eval issue.  The call to _call_program
(which I have removed) causes the wildcard to be expanded too early
because it calls eval.  Here's a trace excerpt.

+_rsync_remote_files:37> remfiles=+_rsync_remote_files:1> _call_program files ssh -a -x apex ls -d1F '/*'
+_call_program:3> local tmp
+_call_program:5> zstyle -s :completion::complete:rsync:argument-rest:files command tmp
+_call_program:12> eval 'ssh -a -x apex ls -d1F /*'
+(eval):1> ssh -a -x apex ls -d1F /bin /boot /cdrom /cdrom0 /dark /dark2 /dev /etc /fc1 /floppy /home /initrd /initrd.img /initrd.img.old /lib /mnt /opt /proc /projects /.rnd /root /sbin /smb /sys /systems /tmp /tmp-Xhack /u1 /u2 /usr /var /vmlinuz
+_rsync_remote_files:37> remfiles=( /bin/ /boot/ /dev/ /etc/ /home@ /initrd/ /lib/ /mnt/ /opt/ /proc/ /projects@ /root/ /sbin/ /smb/ /systems/ /tmp/ /usr/ /var/ )
The ls command ("(eval:1)" above) does an ls on the remote system
after having expanded the wildcard on my local system.

--Jay

Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Clint Adams]

> Yes.  Well, kind of.  It's an eval issue.  The call to _call_program
> (which I have removed) causes the wildcard to be expanded too early
> because it calls eval.  Here's a trace excerpt.

I haven't tried this at all, but maybe this will fix your problem.

Index: Completion/Unix/Command/_rsync
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_rsync,v
retrieving revision 1.8
diff -u -r1.8 _rsync
--- Completion/Unix/Command/_rsync	14 Nov 2003 11:56:55 -0000	1.8
+++ Completion/Unix/Command/_rsync	13 Apr 2004 02:05:03 -0000
@@ -37,8 +37,8 @@
 
   if zstyle -T ":completion:${curcontext}:" remote-access; then
     slash=/
-    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1F ${${${words[CURRENT
-]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null)"})
+    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1F "${${${words[CURRENT
+]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null")"})
 
     remdispf=(${remfiles:#*/})
     remdispd=(${(M)remfiles:#*/})

Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Jay Berkenbilt]

>   > Yes.  Well, kind of.  It's an eval issue.  The call to _call_program
>   > (which I have removed) causes the wildcard to be expanded too early
>   > because it calls eval.  Here's a trace excerpt.
>
>   I haven't tried this at all, but maybe this will fix your problem.

>   [removed patch that encloses ls args in ""]

Yes, this works and has the advantage that it doesn't clobber any
other advantage of using _call_program.  I would still suggest adding
"L" to the ls arguments so that valid (non-dangling) symbolic links to
directories complete with a trailing slash.

Off hand, I'm not able to think of a compelling reason why this would
fail.  It seems to behave properly even with double quote characters
or spaces in the file name (as does my original patch).

To me, something feels cleaner and safer about avoiding the
unnecessary eval than enclosing everything in an extra set of quotes,
This will probably break if _call_program is changed to do what it
does differently.  On the other hand, since I'll admit to not
completely understanding what the purpose of _call_program is (it
looks to me like it provides a mechanism for using zstyle to tell zsh
that command names should be completed at a given point in expanding a
program's argument), I wouldn't try to argue that my original patch is
better.

Note that _ssh and _rlogin both have remote_files functions that do
not call _call_program.  _ssh used to before my previous patch but now
doesn't.

Here's a new patch that is identical to yours except that it adds the
"L" to the ls arguments.

--- _rsync.qdist	2004-03-25 13:29:33.000000000 -0500
+++ _rsync	2004-04-13 12:11:36.000000000 -0400
@@ -37,8 +37,8 @@
 
   if zstyle -T ":completion:${curcontext}:" remote-access; then
     slash=/
-    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1F ${${${words[CURRENT
-]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null)"})
+    remfiles=(${(f)"$(_call_program files ssh -a -x ${words[CURRENT]%:*} ls -d1FL "${${${words[CURRENT
+]#*:}:h}/${slash}(#e)/}/\* 2>/dev/null")"})
 
     remdispf=(${remfiles:#*/})
     remdispd=(${(M)remfiles:#*/})



-- 
Jay Berkenbilt <ejb@ql.org>
http://www.ql.org/q/

Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Bart Schaefer]

[I don't know whether ejb will see this, because I don't know if he's
subscribed to zsh-uses and I suspect bugs.debian.org is going to reject
my mail because I'm on a DSL line.  We'll see ...]

On Apr 13, 12:17pm, Jay Berkenbilt wrote:
} Subject: Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too 
}
} To me, something feels cleaner and safer about avoiding the
} unnecessary eval than enclosing everything in an extra set of quotes

If the eval were unnecessary, believe me, it wouldn't be there.

} This will probably break if _call_program is changed to do what it
} does differently.

It's extremely unlikely that _call_program could be made to do what it
does without the eval.

} On the other hand, since I'll admit to not completely understanding
} what the purpose of _call_program is (it looks to me like it provides
} a mechanism for using zstyle to tell zsh that command names should be
} completed at a given point in expanding a program's argument)

No, that's not what it's for at all.

In this particular case, It's so that you can use a zstyle to tell the
completion function that it should use something other than ssh as the
remote shell program for expanding the file names.  It implements the
"command" zstyle, which is documented:

command
     In many places, completion functions need to call external
     commands to generate the list of completions.  This style can be
     used to override the command that is called in some such cases.
     The elements of the value are joined with spaces to form a command
     line to execute.  The value can also start with a hyphen, in which
     case the usual command will be added to the end; this is most
     useful for putting `builtin' or `command' in front to make sure
     the appropriate version of a command is called, for example to
     avoid calling a shell function with the same name as an external
     command.

So, if for example I have a shell function named "ssh", I might do

    zstyle ':completion:*:complete:scp:*:files' command command -

which would cause _call_program to eval "command ssh ..." when _ssh asks
for remote file completions, and thereby bypass my shell function.

} Note that _ssh and _rlogin both have remote_files functions that do
} not call _call_program.

In the case of _rlogin, it *should* be using _call_program, but that
function predates the invention of the "command" style and has not yet
been updated.  Thanks for pointing that out.

} _ssh used to before my previous patch but now doesn't.

Your previous patch was not applied to the "official" zsh sources (I
don't know about the Debian repackaging).  Instead a patch that added
quoting was done, very similar to what Clint posted for _rsync.  (The
-L option *did* get added to the "ls" in _ssh.)  So _ssh *does* have
_call_program.

Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]

[Clint Adams]

[This message probably contains no content of interest to anyone
 who doesn't care about the Debian BTS or Debian's packaging of zsh]

> [I don't know whether ejb will see this, because I don't know if he's
> subscribed to zsh-uses and I suspect bugs.debian.org is going to reject
> my mail because I'm on a DSL line.  We'll see ...]

bugs.debian.org accepts mail from nearly everyone, on the principle that
people shouldn't have to jump through hoops to report problems.  Or
something similar.

Assuming ejb is still not on the mailing lists (he wasn't last round),
he, as a submitter, would not get your reply unless he were Cc'd, the
object of an X-Debbugs-Cc header, or the message were sent to
nnnnnn-submitter@bugs.debian.org, where nnnnnn is the bug number.

Since your message did go though to the bug log as well as the
zsh-workers list, he can now read it by going to
http://bugs.debian.org/243385 , requesting a copy via the email
interface, or checking the zsh-workers ML archives.

> Your previous patch was not applied to the "official" zsh sources (I
> don't know about the Debian repackaging).  Instead a patch that added
> quoting was done, very similar to what Clint posted for _rsync.  (The
> -L option *did* get added to the "ls" in _ssh.)  So _ssh *does* have
> _call_program.

The Debian repackaging contains the same version as in CVS HEAD.

If anyone is interested, the Debian tree has been in Arch
for a while now.  You can check out a copy by doing the following
(assuming you use tla)

% tla register-archive zsh@packages.debian.org--pkg-zsh \
	http://arch.debian.org/arch/pkg-zsh/current

% tla get zsh@packages.debian.org--pkg-zsh/zsh--debian



At present, the only differences from 4.2.0 are


2004-03-25  Clint Adams  <clint@zsh.org>

	* 19691: Completion/Unix/Type/_diff_options: Allow more than
	one --exclude or -x to diff.

2004-03-23  Peter Stephenson  <pws@csr.com>

	* 19661: Danek Duvall: Completion/Unix/Type/_diff_options:
	support diff -u on Solaris from 2.10 up.

2004-03-22  Clint Adams  <clint@zsh.org>

	* 19673: Peter Palfrader: Completion/Unix/Command/_cvs:
	tags can contain the + character.

2004-03-20  Bart Schaefer  <schaefer@zsh.org>

	* 19666 (tweaked): Completion/Unix/Command/_ssh: fixes for remote
	file completion for scp, suggested by Jay Berkenbilt <ejb@ql.org>.

	* users/7172: Functions/Prompts/prompt_bart_setup: include "unset"
	among the localoptions, as this relies on it.

more boring zsh-in-Arch chatter [was Re: [ejb@ql.org: Bug#243385: zsh: rsync completion expands * too soon]]

[Clint Adams]

> % tla register-archive zsh@packages.debian.org--pkg-zsh \
> 	http://arch.debian.org/arch/pkg-zsh/current
> 
> % tla get zsh@packages.debian.org--pkg-zsh/zsh--debian

Also, I've started mirroring CVS HEAD in Arch; this can be accessed by

% tla register-archive schizo@debian.org--2004-zsh-upstream-cvs-head \
 	http://arch.debian.org/arch/private/schizo/2004-zsh-upstream-cvs-head
 
% tla get schizo@debian.org--2004-zsh-upstream-cvs-head/zsh--cvshead

The cronjob runs every 2 hours, and uses Sourceforge anonymous CVS, so
unless something breaks, it shouldn't lag behind by more than 3 hours or
so.

This experiment is predicated on the theory that it'll be easier to
merge stuff from HEAD via an Arch repo.

Re: more boring zsh-in-Arch chatter

[Clint Adams]

> This experiment is predicated on the theory that it'll be easier to
> merge stuff from HEAD via an Arch repo.

Also, this may provide a useful complement to Sourceforge viewcvs:

http://arch.debian.org/cgi-bin/viewarch.cgi/schizo@debian.org--2004-zsh-upstream-cvs-head/zsh--cvshead--1.0

for patch-8 and later, you'll see the ChangeLog hunk followed by links
to each modified file (assuming that everything is working properly).