-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
adding rbenv support to all the rvm themes
- Loading branch information
Brent Faulkner
committed
Jan 24, 2012
1 parent
ac910b8
commit 6496acf
Showing
17 changed files
with
96 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of these themes used to work without using rvm (or rbenv or the rbenv plugin) at all, because the ruby path string was just an empty string.
Now they fail with #878:
Perhaps provide an empty
rbenv_prompt_info
using something like this?6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've submitted pull request #922 with a fix for this
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried your fix from pull request #922 and it still doesn't work (at least with the superjarin theme).
If I comment out the changes, it works. Would it be better to replace the RVM test with:
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried it with
if [ -e ~/.rvm/bin/rvm-prompt ]; then
and it works for me. I don't have rbenv installed, so I don't know if that part works.6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does "it still doesn't work" mean? when I try it on my machine it seems fine... what is the result and what is the expected result?
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shows the prompt:
Instead of the expected prompt:
Using the alternative test for rvm-prompt's existence fixed it.
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but, I'm a bit confused by description of the problem. When I comment out the code that I added, ie. so all that's there is the original
JARIN_CURRENT_RUBY_="..."
line, I get the following (provided that rvm is not available)...What plugins do you have enabled? and do you have rvm installed or not? (it's good you have a workaround, but I want to look at the larger issue and make sure it gets fixed properly everywhere)
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw... the pull request #922 was to fix a slightly different problem (if I understand yours correctly), the problem reported by @orip was
command not found: rbenv_prompt_info
, while yours seems rvm-relatedthat said, it would be good to fix this one too, so let me know regarding the above-asked questions...
cheers
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes I used the solution from
nebirhos.zsh-theme
. Instead ofif which rvm-prompt &> /dev/null; then
I usedif [ -e ~/.rvm/bin/rvm-prompt ]; then
and it works for me.I do have RVM installed, and here are the plugins I'm using:
I think that the problem might be that
which rvm-prompt &> /dev/null
is returning false even if you have RVM installed, which is causing it to skip down to the rbenv section. Can you try usingif [ -e ~/.rvm/bin/rvm-prompt ]; then
and see if that works?6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated pull request #922 to include an extra commit that should address this
cheers
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks :)
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbfaulkner your fix in #922 didn't address all of the themes that were originally edited so I've opened #964 to fix the others I found that needed to be updated with the fix, which did fix the theme I was using.
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, the problem you're fixing, while very similar is not quite the same... the ones that I fixed were the ones that had hard-coded paths - you are fixing the ones that assume that rvm-prompt is in the path
that said, I appreciate the fact that you are actually providing a fix (rather than just reporting the problem :-)
6496acf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad well I didn't read through everything going on here so that's what I get for assuming. Regardless I was having an issue with the theme I was using that was related to this change, and implementing the fix you had done for the other themes fixed my theme as well.