[Bugfix] rbenv.p9k fails if specified local ruby version is not present

[ChrisBaker97]

I believe this should fix the same issue as #1197 in the master branch, but I can't actually test it, since several segments in the next branch are broken on my install, including rbenv.

[ChrisBaker97]

I've got the next branch working now on my system, and I can verify that this is working properly to fix the bug.

[dritter]

Hey @ChrisBaker97 ,

Thanks for the contribution. Much appreciated. I made some changes to your PR. Could you verify that these fixes work for you?

And would you be so kind to post the output of rbenv version-name, rbenv local and rbenv global here, so that I can add some tests for that segment? That would be super helpful.

Thanks a lot.

[ChrisBaker97]

Hi @dritter,

Appreciate you taking a look at this. Your changes appear to be working fine.

Here's the output you requested:

# let's say that the global ruby version is 2.6.2, and the user also has 
# 2.6.1 installed, but not 2.5.3

# in a directory where no particular local Ruby version is specified:
$ rbenv version-name
2.6.2
$ rbenv local
rbenv: no local version configured for this directory # <-- this is output to stderr / exit code is 1
$ rbenv global
2.6.2

# in a directory where the specified Ruby version is also the global version:
$ rbenv version-name
2.6.2
$ rbenv local
2.6.2
$ rbenv global
2.6.2

# in a directory where the specified Ruby version is installed on the system, 
# but is different from the global version:
$ rbenv version-name
2.6.1
$ rbenv local
2.6.1
$ rbenv global
2.6.2

# in a directory where the specified Ruby version is *not* installed on the system:
$ rbenv version-name
rbenv: version `2.5.3' is not installed (set by /Users/chris/.oh-my-zsh/custom/plugins/zsh-autosuggestions/.ruby-version). # <-- this is output to stderr / exit code is 1
$ rbenv local
2.5.3
$ rbenv global
2.6.2

Looking at this now, after a break, I realize that it could be clearer, in that I included the icon in the previously-existing variable rbenv_version_name. Logically, it works, because if it's gotten to that point, we already know that the requested version isn't going to match rbenv global anyway, but it's a little ugly, and also the subsequent call to rbenv global is unnecessary overhead. Probably it should be refactored for clarity and efficiency, but it works as written here, and does fix the bug.

[dritter]

Awesome 😎 ! Thanks for the quick answer and the helpful use-cases.

Now that I know that there are multiple layers of ruby versions that can be active, I think about turning this segment into a stateful one. Maybe we could find the relevant state this way (warning: untested pseudo code):

local -A states=( "$(echo -n 'VERSION ' && rbenv version-name 2>/dev/null || echo -n " ''" && echo -n 'LOCAL ' && rbenv local 2>/dev/null || echo -n " ''" && echo -n 'GLOBAL ' && rbenv global 2>/dev/null || echo -n " ''" )" )

Caveat: I am not sure, if && spawns a new subshell.
And to find the relevant item:

local currentState
[[ -n "${states[GLOBAL]}" ]] && currentState="GLOBAL"
[[ -n "${states[LOCAL]}" ]] && currentState="LOCAL"
[[ -n "${states[VERSION]}" ]] && currentState="VERSION"

local currentVersion="${states[$currentState]}"

p9k::prepare_segment "$0" "${currentState}" $1 "$2" $3 "${currentVersion}"

This is a very wild idea, so it might not work, but if it does, it might help to clear the code a bit up.
Making this segment stateful, would make it possible to style it differently, depending on the used level of rbenv. So it might be possible to have a blue segment, if global ruby is used, and a red one if a local one is in use. What do you think? Is it worth it?

And I still have some questions:

• Do you have a better name for the version-name state? I honestly didn't understand the difference between that and the local one.
• In the segment we use a shortcut: If ${RBENV_VERSION} is set, we use that one for rendering the segment. In a stateful variant, that wouldn't be possible any more. The segment has to determine the state by itself.. And I don't know if this variable always yield the correct value, if we stay without states. Do you have more insights here?
• Do you want to try to implement some tests for that segment? That would be awesome. I'll guide you through, if you need help. :)

Edit:
Thinking about that again, maybe it is not the right time to introduce states to this segment. Maybe we should do that for a future version. One drawback of states is that the configuration gets more extensive (needs configuration per state).

[ChrisBaker97]

Sorry about the delay. I'm happy to work on this, but it could be a little bit until I have a block of time. I'll have to take a look at some of the stateful segments to get a handle on that. Tests aren't a problem—I've already gotten my head around that working on the Node segment. I can probably tackle tests first, with the current code, and then use that to help verify that a stateful mod is working properly.

Addressing your questions, I think this might do a better job than I could explaining how the various layers of Ruby version specification interact. More than a shortcut, ${RBENV_VERSION} is actually the king—and overrides the rest of of them—so its value would definitely have to come into play. (So if there's no way around that for stateful segments, then I guess that's not going to be an option.)

I don't really have a better name for rbenv_version_name. As written, it's okay, until the point that I co-opted it to display an error icon, and it starts to serve more as a rbenv_segment_text function instead. I think I can do a bit of a refactor to clarify the execution and refine this.

I don't believe that && creates a new subshell. It just controls execution of the latter command, based upon the exit code of the former. (See this StackExchange answer.)

With all that in mind, does it make sense to commit this PR once tests are in place, and then open up a separate issue for the stateful conversion, if you think there can be a workaround to the environment variable problem?