* A cute bug involving aliases and _remote_files @ 2019-02-26 2:49 Philippe Troin 2019-02-26 9:23 ` dana 0 siblings, 1 reply; 5+ messages in thread From: Philippe Troin @ 2019-02-26 2:49 UTC (permalink / raw) To: Zsh hackers list Hi, I have the following aliases in my .zshenv: alias ls='command ls -Fbv' The interesting part there is the "-b" which instructs ls to quote unprintable characters and spaces. But in _remote_files we do: _call_program files $cmd $cmd_args $host ls -d1FL -- which will use the alias, and when I complete a file with a space, zsh ends up prefixing the space with 7 backslashes, as there's now an extra round of escaping performed. Would it make sense to use "command ls" instead of "ls" for remote directory listings in _remote_files? That would make sure that no aliases nor functions are involved. I don't need any downsides as all shells that I'm aware of have the "command" pre-command modifier. Example of the issue (you need to alias ls as described above for this to show): % mkdir test_dir % touch test_dir/'a file' % scp localhost:$PWD/test_dir/a ^ Press TAB ===> completes to: % scp localhost:$PWD/test_dir/a\\\\\\\ file I have this in my .zshrc in the meantime: _remote_files() { unfunction _remote_files autoload +X _remote_files if [[ $functions[_remote_files] != *' command ls -' ]] then functions[_remote_files]=${functions[_remote_files]// ls -/ command ls -} fi _remote_files $* } Yet it would be neat if this were fixed upstream. Thanks, Phil. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: A cute bug involving aliases and _remote_files 2019-02-26 2:49 A cute bug involving aliases and _remote_files Philippe Troin @ 2019-02-26 9:23 ` dana 2019-02-26 9:40 ` Peter Stephenson 2019-02-27 8:27 ` Philippe Troin 0 siblings, 2 replies; 5+ messages in thread From: dana @ 2019-02-26 9:23 UTC (permalink / raw) To: Philippe Troin; +Cc: Zsh hackers list On 25 Feb 2019, at 20:49, Philippe Troin <phil@fifi.org> wrote: >Would it make sense to use "command ls" instead of "ls" for remote >directory listings in _remote_files? That would make sure that no >aliases nor functions are involved. That *seems* reasonable...? Now that you mention it, GNU ls changed its default quoting behaviour somewhat recently. I think it only quotes when (a) QUOTING_STYLE is set, (b) one of the quoting options is given on the command line, or (c) the output is a TTY — so it's unlikely to be an issue for _remote_files, given how it's normally used. But maybe it'd be a good idea to force it off anyway, just in case? dana diff --git a/Completion/Unix/Type/_remote_files b/Completion/Unix/Type/_remote_files index 267715a51..89cf102cf 100644 --- a/Completion/Unix/Type/_remote_files +++ b/Completion/Unix/Type/_remote_files @@ -59,7 +59,10 @@ if zstyle -T ":completion:${curcontext}:files" remote-access; then fi # remote filenames - remfiles=(${(M)${(f)"$(_call_program files $cmd $cmd_args $host ls -d1FL -- "$rempat" 2>/dev/null)"}%%[^/]#(|/)}) + remfiles=(${(M)${(f)"$( + _call_program files $cmd $cmd_args $host \ + QUOTING_STYLE=literal command ls -d1FL -- "$rempat" 2>/dev/null + )"}%%[^/]#(|/)}) compset -P '*/' compset -S '/*' || (( ${args[(I)-/]} )) || suf='remote file' ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: A cute bug involving aliases and _remote_files 2019-02-26 9:23 ` dana @ 2019-02-26 9:40 ` Peter Stephenson 2019-02-27 8:27 ` Philippe Troin 1 sibling, 0 replies; 5+ messages in thread From: Peter Stephenson @ 2019-02-26 9:40 UTC (permalink / raw) To: zsh-workers On Tue, 2019-02-26 at 03:23 -0600, dana wrote: > On 25 Feb 2019, at 20:49, Philippe Troin <phil@fifi.org> wrote: > > > > Would it make sense to use "command ls" instead of "ls" for remote > > directory listings in _remote_files? That would make sure that no > > aliases nor functions are involved. > That *seems* reasonable...? Looks to me like that's probably a good rule of thumb any time you're relying on a bunch of specific arguments to a command... But ls is particularly often aliased. pws ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: A cute bug involving aliases and _remote_files 2019-02-26 9:23 ` dana 2019-02-26 9:40 ` Peter Stephenson @ 2019-02-27 8:27 ` Philippe Troin 2019-02-27 8:42 ` dana 1 sibling, 1 reply; 5+ messages in thread From: Philippe Troin @ 2019-02-27 8:27 UTC (permalink / raw) To: Zsh hackers list On Tue, 2019-02-26 at 03:23 -0600, dana wrote: > On 25 Feb 2019, at 20:49, Philippe Troin <phil@fifi.org> wrote: > > Would it make sense to use "command ls" instead of "ls" for remote > > directory listings in _remote_files? That would make sure that no > > aliases nor functions are involved. > > That *seems* reasonable...? > > Now that you mention it, GNU ls changed its default quoting behaviour somewhat > recently. I think it only quotes when (a) QUOTING_STYLE is set, (b) > one of the > quoting options is given on the command line, or (c) the output is a > TTY — so > it's unlikely to be an issue for _remote_files, given how it's > normally used. > But maybe it'd be a good idea to force it off anyway, just in case? I'm completely unaware of how QUOTING_STYLE could possibly be getting us into trouble, but the way you call the binary (variable=value command ls) is bourne-shell only (bash, sh, ash, ksh), and will not work if the remote system is using c-shells (csh, tcsh). I'd recommend using env, as in: + remfiles=(${(M)${(f)"$( + _call_program files $cmd $cmd_args $host \ + env QUOTING_STYLE=literal ls -d1FL -- "$rempat" > 2>/dev/null + )"}%%[^/]#(|/)}) That will work everywhere. Now, I haven't seen anyone using c-shells in a very long while, but I'm sure there are some holdouts :-) Since you use "env" instead of calling ls directory, you don't need command anymore, because no alias expansion will occur for ls. I assume we don't care about global aliases, which none of the proposed solutions would handle anyways. Phil. > diff --git a/Completion/Unix/Type/_remote_files > b/Completion/Unix/Type/_remote_files > index 267715a51..89cf102cf 100644 > --- a/Completion/Unix/Type/_remote_files > +++ b/Completion/Unix/Type/_remote_files > @@ -59,7 +59,10 @@ if zstyle -T ":completion:${curcontext}:files" > remote-access; then > fi > > # remote filenames > - remfiles=(${(M)${(f)"$(_call_program files $cmd $cmd_args $host ls > -d1FL -- "$rempat" 2>/dev/null)"}%%[^/]#(|/)}) > + remfiles=(${(M)${(f)"$( > + _call_program files $cmd $cmd_args $host \ > + QUOTING_STYLE=literal command ls -d1FL -- "$rempat" > 2>/dev/null > + )"}%%[^/]#(|/)}) > > compset -P '*/' > compset -S '/*' || (( ${args[(I)-/]} )) || suf='remote file' > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: A cute bug involving aliases and _remote_files 2019-02-27 8:27 ` Philippe Troin @ 2019-02-27 8:42 ` dana 0 siblings, 0 replies; 5+ messages in thread From: dana @ 2019-02-27 8:42 UTC (permalink / raw) To: Philippe Troin; +Cc: Zsh hackers list On 27 Feb 2019, at 02:27, Philippe Troin <phil@fifi.org> wrote: >I'm completely unaware of how QUOTING_STYLE could possibly be getting >us into trouble, but the way you call the binary (variable=value >command ls) is bourne-shell only (bash, sh, ash, ksh), and will not >work if the remote system is using c-shells (csh, tcsh). Oh, i forgot. It won't work in fish either. Not good enough :/ The quoting thing probably isn't a big deal, like i said — seems easiest to just go with the original suggestion and leave it out. Unless someone feels strongly otherwise, i think i'll do that dana ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-27 8:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-26 2:49 A cute bug involving aliases and _remote_files Philippe Troin 2019-02-26 9:23 ` dana 2019-02-26 9:40 ` Peter Stephenson 2019-02-27 8:27 ` Philippe Troin 2019-02-27 8:42 ` dana
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).