Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zsh script improvements #701

Merged
merged 2 commits into from
Oct 14, 2016
Merged

Conversation

nthapaliya
Copy link
Contributor

I'm using macOS, but I've installed gnu coreutils from homebrew. By default, in order to avoid conflicts with system utils, they are prefixed with a g, eg. ls is installed as gls and so on.

There is a zsh module (from prezto) that wraps these utils in functions so that I can use them without the prefix in interactive shells. For example:

$ which -a cat
cat () {
        '/usr/local/bin/gcat' "$@"
}
/bin/cat

And so on.

This PR fixes some issues I encountered with this setup. In particular [ and cat. I've also tested these changes on 'vanilla' zsh (using system utils, no gnu-coreutils) and nothing breaks.

Bug from first commit:

Autocomplete with **<tab> would print an error like:

vi **/usr/local/bin/g[: missing argument after ‘-d’

From the bsd man page for test. I assume this is the same for gnu-test.

BUGS
     Both sides are always evaluated in -a and -o.  For instance, the writable status of file will be tested by the fol-
     lowing command even though the former expression indicated false, which results in a gratuitous access to the file
     system:
           [ -z abc -a -w file ]
     To avoid this, write
           [ -z abc ] && [ -w file ]

To reproduce the bug from the second commit:

I tested by starting a clean zsh session with ZDOTDIR=/dev/null zsh

$ source .fzf.zsh
$ which cat # making sure cat is /bin/cat (default)
$ ssh **<tab> # works
# now define your own cat wrapper
# you can just wrap the default cat, you still get the bug.
$ cat() { '/bin/cat' "$@" }

# typing in ssh ** and hitting tab will now give us
MacBook-Pro% ssh **cat: /dev/fd/12: Bad file descriptor
cat: /dev/fd/13: Bad file descriptor
cat: /dev/fd/14: Bad file descriptor
MacBook-Pro% ssh **

@junegunn
Copy link
Owner

I see, thanks. What do you think about the following? Smaller diff.

diff --git a/shell/completion.zsh b/shell/completion.zsh
index cef8afa..0be5ad1 100644
--- a/shell/completion.zsh
+++ b/shell/completion.zsh
@@ -44,7 +44,7 @@ __fzf_generic_path_completion() {
   setopt localoptions nonomatch
   dir="$base"
   while [ 1 ]; do
-    if [ -z "$dir" -o -d ${~dir} ]; then
+    if [[ -z "$dir" || -d ${~dir} ]]; then
       leftover=${base/#"$dir"}
       leftover=${leftover/#\/}
       [ -z "$dir" ] && dir='.'
@@ -111,7 +111,7 @@ _fzf_complete_telnet() {

 _fzf_complete_ssh() {
   _fzf_complete '+m' "$@" < <(
-    cat <(cat ~/.ssh/config /etc/ssh/ssh_config 2> /dev/null | command grep -i '^host' | command grep -v '*') \
+    command cat <(cat ~/.ssh/config /etc/ssh/ssh_config 2> /dev/null | command grep -i '^host' | command grep -v '*') \
         <(command grep -oE '^[^ ]+' ~/.ssh/known_hosts | tr ',' '\n' | awk '{ print $1 " " $1 }') \
         <(command grep -v '^\s*\(#\|$\)' /etc/hosts | command grep -Fv '0.0.0.0') |
         awk '{if (length($2) > 0) {print $2}}' | sort -u

@nthapaliya
Copy link
Contributor Author

Yeah, yours is better. I'll make the change.

It doesn't short circuit like we expect, causing trouble when $dir is
empty

Use shell builtin instead
@nthapaliya
Copy link
Contributor Author

OK, I've updated the commits.

@junegunn junegunn merged commit 47b11cb into junegunn:master Oct 14, 2016
@junegunn
Copy link
Owner

Merged, thanks!

@nthapaliya nthapaliya deleted the zsh_script_improvements branch October 14, 2016 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants