From e91091acdbb4224f746fc9f6852764ed7fa4f58e Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Sat, 22 Oct 2016 10:53:51 +0200 Subject: [PATCH 1/8] [chruby] scope variable to local to prevent a leak --- plugins/chruby/chruby.plugin.zsh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index 758b4a56c..1addc00e6 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -55,8 +55,8 @@ _source_from_omz_settings() { } _chruby_dirs() { - chrubydirs=($HOME/.rubies/ $PREFIX/opt/rubies) - for dir in chrubydirs; do + local _chrubydirs=($HOME/.rubies/ $PREFIX/opt/rubies) + for dir in _chrubydirs; do if [[ -d $dir ]]; then RUBIES+=$dir fi From 53526db9792d50278bfc2cfeaf0c07cb6faeba2f Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Sat, 22 Oct 2016 10:54:19 +0200 Subject: [PATCH 2/8] [chruby] use one line assign to local to match style with rest of the file --- plugins/chruby/chruby.plugin.zsh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index 1addc00e6..f4dd2c47d 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -81,8 +81,7 @@ function ensure_chruby() { } function current_ruby() { - local _ruby - _ruby="$(chruby |grep \* |tr -d '* ')" + local _ruby="$(chruby |grep \* |tr -d '* ')" if [[ $(chruby |grep -c \*) -eq 1 ]]; then echo ${_ruby} else From 927d5ecec6969fe37d3a4d0b9a1482b04cf4f154 Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Fri, 21 Oct 2016 21:32:42 +0200 Subject: [PATCH 3/8] [chruby] avoid calling brew --prefix 3 times once is enough, improves performance significantly scope variable to local via anonymous function --- plugins/chruby/chruby.plugin.zsh | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index f4dd2c47d..c4453b86a 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -20,10 +20,6 @@ _homebrew-installed() { whence brew &> /dev/null } -_chruby-from-homebrew-installed() { - [ -r $(brew --prefix chruby) ] &> /dev/null -} - _ruby-build_installed() { whence ruby-build &> /dev/null } @@ -63,18 +59,22 @@ _chruby_dirs() { done } -if _homebrew-installed && _chruby-from-homebrew-installed ; then - source $(brew --prefix chruby)/share/chruby/chruby.sh - source $(brew --prefix chruby)/share/chruby/auto.sh - _chruby_dirs -elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then - source /usr/local/share/chruby/chruby.sh - source /usr/local/share/chruby/auto.sh - _chruby_dirs -else - _source_from_omz_settings - _chruby_dirs -fi +function () { + local _chruby_homebrew_prefix=$(brew --prefix chruby) &> /dev/null + + if _homebrew-installed && [ -r ${_chruby_homebrew_prefix} ] ; then + source ${_chruby_homebrew_prefix}/share/chruby/chruby.sh + source ${_chruby_homebrew_prefix}/share/chruby/auto.sh + _chruby_dirs + elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then + source /usr/local/share/chruby/chruby.sh + source /usr/local/share/chruby/auto.sh + _chruby_dirs + else + _source_from_omz_settings + _chruby_dirs + fi +} function ensure_chruby() { $(whence chruby) From c78a6e2b4b6e81b80130d6572d907b905ede154e Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Tue, 25 Oct 2016 12:39:25 +0200 Subject: [PATCH 4/8] [chruby] don't add rubies to chruby chruby does that automatically. from documentation: > When chruby is first loaded by the shell, it will auto-detect Rubies installed in /opt/rubies/ and ~/.rubies/ --- plugins/chruby/chruby.plugin.zsh | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index c4453b86a..44a2a660d 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -50,29 +50,17 @@ _source_from_omz_settings() { fi } -_chruby_dirs() { - local _chrubydirs=($HOME/.rubies/ $PREFIX/opt/rubies) - for dir in _chrubydirs; do - if [[ -d $dir ]]; then - RUBIES+=$dir - fi - done -} - function () { local _chruby_homebrew_prefix=$(brew --prefix chruby) &> /dev/null if _homebrew-installed && [ -r ${_chruby_homebrew_prefix} ] ; then source ${_chruby_homebrew_prefix}/share/chruby/chruby.sh source ${_chruby_homebrew_prefix}/share/chruby/auto.sh - _chruby_dirs elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then source /usr/local/share/chruby/chruby.sh source /usr/local/share/chruby/auto.sh - _chruby_dirs else _source_from_omz_settings - _chruby_dirs fi } From edf38111d945a77397fe8f6b4b13b20429a56a89 Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Tue, 25 Oct 2016 12:45:44 +0200 Subject: [PATCH 5/8] [chruby] properly quote variable expansion to prevent word splitting --- plugins/chruby/chruby.plugin.zsh | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index 44a2a660d..98e59b567 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -30,7 +30,9 @@ _ruby-install-installed() { # Simple definition completer for ruby-build if _ruby-build_installed; then - _ruby-build() { compadd $(ruby-build --definitions) } + _ruby-build() { + compadd $(ruby-build --definitions) + } compdef _ruby-build ruby-build fi @@ -41,21 +43,21 @@ _source_from_omz_settings() { zstyle -s :omz:plugins:chruby path _chruby_path zstyle -s :omz:plugins:chruby auto _chruby_auto - if [[ -r ${_chruby_path} ]]; then - source ${_chruby_path} + if [[ -r "${_chruby_path}" ]]; then + source "${_chruby_path}" fi - if [[ -r ${_chruby_auto} ]]; then - source ${_chruby_auto} + if [[ -r "${_chruby_auto}" ]]; then + source "${_chruby_auto}" fi } function () { - local _chruby_homebrew_prefix=$(brew --prefix chruby) &> /dev/null + local _chruby_homebrew_prefix="$(brew --prefix chruby) &> /dev/null" - if _homebrew-installed && [ -r ${_chruby_homebrew_prefix} ] ; then - source ${_chruby_homebrew_prefix}/share/chruby/chruby.sh - source ${_chruby_homebrew_prefix}/share/chruby/auto.sh + if _homebrew-installed && [ -r "${_chruby_homebrew_prefix}" ] ; then + source "${_chruby_homebrew_prefix}/share/chruby/chruby.sh" + source "${_chruby_homebrew_prefix}/share/chruby/auto.sh" elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then source /usr/local/share/chruby/chruby.sh source /usr/local/share/chruby/auto.sh @@ -69,9 +71,9 @@ function ensure_chruby() { } function current_ruby() { - local _ruby="$(chruby |grep \* |tr -d '* ')" - if [[ $(chruby |grep -c \*) -eq 1 ]]; then - echo ${_ruby} + local _ruby="$(chruby | grep \* | tr -d '* ')" + if [[ -n "${_ruby}" ]]; then + echo "${_ruby}" else echo "system" fi From 98721878fb3f70a4ced7198cb18360976548e0f2 Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Mon, 2 Jan 2017 14:04:51 +0100 Subject: [PATCH 6/8] [chruby] adhere to the style guide noted in the review: https://github.com/robbyrussell/oh-my-zsh/pull/5562/files#r94311319 --- plugins/chruby/chruby.plugin.zsh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index 98e59b567..0c71b87fa 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -43,19 +43,19 @@ _source_from_omz_settings() { zstyle -s :omz:plugins:chruby path _chruby_path zstyle -s :omz:plugins:chruby auto _chruby_auto - if [[ -r "${_chruby_path}" ]]; then - source "${_chruby_path}" + if [[ -r "$_chruby_path" ]]; then + source "$_chruby_path" fi - if [[ -r "${_chruby_auto}" ]]; then - source "${_chruby_auto}" + if [[ -r "$_chruby_auto" ]]; then + source "$_chruby_auto" fi } function () { local _chruby_homebrew_prefix="$(brew --prefix chruby) &> /dev/null" - if _homebrew-installed && [ -r "${_chruby_homebrew_prefix}" ] ; then + if _homebrew-installed && [[ -r "$_chruby_homebrew_prefix" ]] ; then source "${_chruby_homebrew_prefix}/share/chruby/chruby.sh" source "${_chruby_homebrew_prefix}/share/chruby/auto.sh" elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then @@ -72,8 +72,8 @@ function ensure_chruby() { function current_ruby() { local _ruby="$(chruby | grep \* | tr -d '* ')" - if [[ -n "${_ruby}" ]]; then - echo "${_ruby}" + if [[ -n "$_ruby" ]]; then + echo "$_ruby" else echo "system" fi From 8f91babe95de5582eccb620014999e6da2075f90 Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Mon, 2 Jan 2017 14:11:50 +0100 Subject: [PATCH 7/8] [chruby] properly redirect errors on missing chruby --- plugins/chruby/chruby.plugin.zsh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index 0c71b87fa..a10d7785d 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -53,7 +53,7 @@ _source_from_omz_settings() { } function () { - local _chruby_homebrew_prefix="$(brew --prefix chruby) &> /dev/null" + local _chruby_homebrew_prefix="$(brew --prefix chruby 2> /dev/null)" if _homebrew-installed && [[ -r "$_chruby_homebrew_prefix" ]] ; then source "${_chruby_homebrew_prefix}/share/chruby/chruby.sh" From 4684c8c0a1832126594a8eeccafb678ec025720d Mon Sep 17 00:00:00 2001 From: Randall Leeds Date: Fri, 11 Aug 2017 13:48:37 -0700 Subject: [PATCH 8/8] Avoid brew unless necessary to find chruby Rather than checking homebrew first, honor explicit user preferences by checking the zstyle first, then looking for chruby in its default location, and finally falling back on homebrew only if chruby was not found anywhere else. --- plugins/chruby/chruby.plugin.zsh | 53 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/plugins/chruby/chruby.plugin.zsh b/plugins/chruby/chruby.plugin.zsh index a10d7785d..8f4a20cab 100644 --- a/plugins/chruby/chruby.plugin.zsh +++ b/plugins/chruby/chruby.plugin.zsh @@ -8,7 +8,7 @@ # # zstyle :omz:plugins:chruby path /local/path/to/chruby.sh # zstyle :omz:plugins:chruby auto /local/path/to/auto.sh -# +# # TODO # - autodetermine correct source path on non OS X systems # - completion if ruby-install exists @@ -36,33 +36,34 @@ if _ruby-build_installed; then compdef _ruby-build ruby-build fi -_source_from_omz_settings() { - local _chruby_path - local _chruby_auto - - zstyle -s :omz:plugins:chruby path _chruby_path - zstyle -s :omz:plugins:chruby auto _chruby_auto - - if [[ -r "$_chruby_path" ]]; then - source "$_chruby_path" - fi - - if [[ -r "$_chruby_auto" ]]; then - source "$_chruby_auto" - fi -} - function () { - local _chruby_homebrew_prefix="$(brew --prefix chruby 2> /dev/null)" + local _path + local _auto + local _prefix="/usr/local/share/chruby" - if _homebrew-installed && [[ -r "$_chruby_homebrew_prefix" ]] ; then - source "${_chruby_homebrew_prefix}/share/chruby/chruby.sh" - source "${_chruby_homebrew_prefix}/share/chruby/auto.sh" - elif [[ -r "/usr/local/share/chruby/chruby.sh" ]] ; then - source /usr/local/share/chruby/chruby.sh - source /usr/local/share/chruby/auto.sh - else - _source_from_omz_settings + # Honor explicit user preference + zstyle -s :omz:plugins:chruby path _path + zstyle -s :omz:plugins:chruby auto _auto + + # Default to /usr/local/share/chruby if either is not defined + [[ -r "$_path" ]] || _path="${_prefix}/chruby.sh" + [[ -r "$_auto" ]] || _auto="${_prefix}/auto.sh" + + # Fall back on homebrew + if [[ ! ( -r "$_path" && -r "$_auto" ) ]]; then + if _homebrew-installed; then + _prefix="$(brew --prefix chruby 2> /dev/null)/share/chruby" + [[ -r "$_path" ]] || _path="${_prefix}/chruby.sh" + [[ -r "$_auto" ]] || _auto="${_prefix}/auto.sh" + fi + fi + + if [[ -r "$_path" ]]; then + source "$_path" + fi + + if [[ -r "$_auto" ]]; then + source "$_auto" fi }