postmodern / chruby

Changes the current Ruby
MIT License
2.88k stars 189 forks source link

Invoke chruby_auto before the prompt prints in interactive mode #191

Open postmodern opened 11 years ago

postmodern commented 11 years ago

@zendeavor points out that it is confusing to users that chruby_auto is invoked before the command is executed and before the prompt is printed. If user's (ex: @jc00ke) include $RUBY_ENGINE and $RUBY_VERSION in their $PS1, then these values wont be updated until the command after the cd command is executed. @zendeavor recommends using different hooking strategies for interactive vs. non-interactive modes.

Interactive

This would invoke chruby_auto before PS1 is rendered in interactive mode, but invoke chruby_auto before the command is even executed in non-interactive mode.

zendeavor commented 11 years ago

ksh would be a Discipline function for interactive mode, and potentially for non-interactive use as well. sticking chruby_auto into $PS1 will cause equally surprising and maybe hard to debug behaviour in the case that chruby_auto fails; the stderr might get printed directly into PS1 (ugly and surprising) or it might be discarded entirely (hard to debug)

regularfry commented 11 years ago

Would it be total heresy to suggest that chruby_auto is a misfeature? Automatically running stuffs on cd is all well and good, but it's fairly well-handled by, for instance, http://github.com/kennethreitz/autoenv. Given that this responsibility can be punted to another tool, shouldn't we do that rather than complicate chruby?

postmodern commented 11 years ago

Auto-switching was a much requested feature. We put it in a separate file so as not to force it upon the other users. Implementing auto-switching "correctly" is tricky. For example autoenv merely hooks cd (tsk tsk), which will miss things such as git pull or git checkout other_branch.

zendeavor commented 11 years ago
function chruby_preexec_bash_set {                                              
  typeset -a hook                                                               
  typeset trap=$(trap -p DEBUG)                                                 
  if [[ $1 == -r ]]; then                                                       
    PROMPT_COMMAND=${PROMPT_COMMAND//chruby_auto?}                              
    IFS=\' read -ra hook <<<"$trap"                                             
    IFS=\; read -ra hook <<<"${hook[@]:1:${#hook[@]}-2}"                        
    trap "${hook[@]//chruby_auto?}" DEBUG                                       
  fi                                                                            
  [[ $PROMPT_COMMAND == *chruby_auto* || $(trap -p DEBUG) == *chruby_auto* ]] &&
    chruby_preexec_bash_set -r                                                  
  if [[ $- == *i* ]]; then                                                      
    IFS=\; read -ra hook <<<"$PROMPT_COMMAND"                                   
    PROMPT_COMMAND="${hook[@]/%/;} chruby_auto"                                 
  else                                                                          
    IFS=\' read -ra hook <<<"$trap"                                             
    IFS=\; read -ra hook <<<"${hook[@]:1:${#hook[@]}-2}"                        
    trap "${hook[@]/%/;} chruby_auto" DEBUG                                     
  fi                                                                            
}

updated with some basic hook-removal logic updated again with a bit of deduplication. there's some more redundancy left, but i don't see a clean way to handle it without polluting the function namespace with a stupidly complicated chruby_read_hook that no one would ever use for anything in the world.

zendeavor commented 11 years ago

i admit there's probably quite a bit of magic happening in a black hole with the above code but it's pretty much implementation detail. this method removes the need for some really arcane parameter expansions and should Just Work™ but it may need a little bit of extra error checking to make sure ${hook[@]} isn't empty before we use it directly.

edlebert commented 10 years ago

$_ is also broken in bash with chruby_auto. It always returns chruby_auto

~ $ echo $_ chruby_auto

2called-chaos commented 10 years ago

I don't if this is exactly related so tell me if I should create a new issue for this. I've set my OS X Terminal to cd into the current directory when opening a new tab. This happens:

Last login: Wed Jan 15 15:53:58 on ttys000
You have new mail.
cd "/Volumes/code/_cont/etn-intranet" && clear
chruby: unknown Ruby: 1.9.3-p327-perf
/Volumes/code/_cont/etn-intranet[master]$ cd "/Volumes/code/_cont/etn-intranet" && clear

chruby-auto seems to hook (ruby 2 is default, 1.9 is in .ruby-version) but it doesn't recognize the version at this point. When I put export RUBIES=(~/.rbenv/versions/*) before source '/usr/local/share/chruby/auto.sh' it won't have this error but it also doesn't switch effectively failing silently...

caleb commented 9 years ago

In my ZSH configuration, I use this to move chruby_auto to precmd:

if [[ $PS1 ]]; then
  precmd_functions+=("chruby_auto")
  preexec_functions=${preexec_functions:#"chruby_auto"}
fi

This seems to work for me, and fixes the problem of my prompt not updating when I first cd to a directory with a .ruby-version file. This is because preexec_functions are executed before a command is executed. Whereas precmd_functions get executed before the prompt is printed.

It would be nice to see this change at least for ZSH in chruby. I don't know anything about bash, and I suppose I could be missing something with this ZSH configuration, but it works for me.

swrobel commented 8 years ago

Thank you, @caleb for that workaround! Adding my +1 for seeing this added to chruby.

FranklinYu commented 4 years ago

I wonder whether we need to detect interactive shell in auto.sh. As far as I understand, auto.sh is only intended for interactive shell; non-interactive shell should use chruby-exec which only uses chruby.sh.

In addition, I feel like it’s more elegant to use the Zsh builtin add-zsh-hook, like

add-zsh-hook -d preexec chruby_auto
add-zsh-hook precmd chruby_auto

where -d means “delete”.