hmans / rbfu

Minimal Ruby version management is minimal.
MIT License
118 stars 10 forks source link

Aliased cd command does not forward original cd exit status #15

Closed mtgrosser closed 12 years ago

mtgrosser commented 12 years ago

When using rbfu's auto-switch mode, the "cd" command is aliased in order to automatically switch the rbfu environment when changing directories. However, this aliased version does not return the correct exit status of the original cd command, so it is no longer possible to determine whether changing the directory succeeded, and scripts like

cd somedir && do_something

inadvertently fail.

To reproduce the behaviour, open a new shell session:

$ cd
$ echo $?
0
$ eval "$(rbfu --init --auto)"
$ cd
$ echo $?
1
hmans commented 12 years ago

What version are you using? It appears to works correctly in the current HEAD (I just tried).

hmans commented 12 years ago

I'm closing this issue for now. If you're seeing this problem on HEAD, post a comment here and I'll gladly reopen the issue.

mtgrosser commented 12 years ago

I'm using the latest version. The cd command returns 1 for me if there is no .rbfu-version file in the directory I'm cding to.

$ cd
$ echo $?
1
$ echo "system" > .rbfu-version
$ cd
Activated system Ruby. (from /home/rails/.rbfu-version)
$ echo $?
0
hmans commented 12 years ago

Define "the latest version"? I'm running on HEAD and can't reproduce any of your examples. Also, what shell are you using? bash or zsh?

mtgrosser commented 12 years ago

I'm sorry, I mean HEAD of course. I can reproduce the behaviour on

GNU bash, version 4.1.2(1)-release (x86_64-redhat-linux-gnu) on CentOS 6.2, GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0) on OSX 10.6.8, GNU bash, Version 4.2.24(1)-release (x86_64-pc-linux-gnu) on Linux Mint 13 Maya

However, it seems to work correctly in zsh 4.3.17 (x86_64-unknown-linux-gnu) on Linux Mint. Maybe a bash related issue?

hmans commented 12 years ago

I'm using bash 4.2.24 on OS X. Hmmm -- okay, I'll investigate some more. Thanks for letting me know!

winks commented 12 years ago

Yes, I can reproduce it with bash 4.1.5(1)-release on Debian testing

$ bash ^^(florian@portia):(~) $ cd ^^(florian@portia):(~) $ echo $? 0 ^^(florian@portia):(~) $ eval "$(rbfu --init --auto)" --(florian@portia):(~) $ echo $? 1 ^^(florian@portia):(~) $ cd --(florian@portia):(~) $ echo $? 1

menski commented 12 years ago

I can also reproduce it with GNU bash, version 4.2.29(2)-release (x86_64-unknown-linux-gnu) on archlinux

menski@snickers ~ % bash
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ eval "$(rbfu --init --auto)"
[menski@snickers ~]$ echo $?
1
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
1

I think the problem is the && conjunction in the cd function

function cd() {
  builtin cd "\$@" && _rbfu_auto
}

It returns the exit status of the last command _rbfu_auto which will be 1 if no .rbfu-version or .ruby-version file exists.

Is the return value of _rbfu_auto important?

If it would always return 0 the problem is fixed. Because if builtin cd returns 1 _rbfu_auto is not called and the function cd returns 1. If builtin cd return 0 _rbfu_auto is called and returns 0 so the function cd also returns 0.

For example:

function _rbfu_auto() {
  ([ -f "\$PWD/.rbfu-version" ] || [ -f "\$PWD/.ruby-version" ]) && source rbfu true
  return 0
}

function cd() {
  builtin cd "\$@" && _rbfu_auto
}

Works for me:

menski@snickers ~ % bash
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ eval "$(rbfu --init --auto)"
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ cd /not/exists
bash: cd: /not/exists: No such file or directory
[menski@snickers ~]$ echo $?
1

Another solution would be:

function cd() {
  builtin cd "\$@" && ( _rbfu_auto || return 0 )
}

Because if _rbfu_auto is called builtin cd has returned 0 so you could return it if _rbfu_auto fails

menski@snickers ~ % bash
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ eval "$(rbfu --init --auto)"
[menski@snickers ~]$ echo $?
1
[menski@snickers ~]$ cd
[menski@snickers ~]$ echo $?
0
[menski@snickers ~]$ cd /not/exists
bash: cd: /not/exists: No such file or directory
[menski@snickers ~]$ echo $?
1
hmans commented 12 years ago

Thanks for all the comments. I'm looking into this now.