feature request: automatic nvm use when entering directories with .nvmrc

[sanzoghenzo]

Hi, thanks a lot for this great project! I started using a few days ago, and I really like the fact that I can get most of the oh-my-zsh feeling without switching the default shell (I ditched Mac OS a long time ago).

I'm using the nvm plugin, and I would like to have the automatic switch of nodejs version when I cd to a directory that has a .nvmrc file in it.

I solved this thanks to this great answer from stackoverflow, adding the following at the end of my .bashrc

enter_directory() {
  if [[ $PWD == "$PREV_PWD" ]]; then
    return
  fi

  if [[ "$PWD" =~ $PREV_PWD && ! -f ".nvmrc" ]]; then
    return
  fi

  PREV_PWD=$PWD
  if [[ -f ".nvmrc" ]]; then
    nvm use
    NVM_DIRTY=true
  elif [[ $NVM_DIRTY = true ]]; then
    nvm use default
    NVM_DIRTY=false
  fi
}

export PROMPT_COMMAND="$PROMPT_COMMAND; enter_directory"

could this be embedded in the nvm plugin?
would you accept a PR with that?

[akinomyoga]

Thank you for your suggestion! This hears like useful.

could this be embedded in the nvm plugin?
would you accept a PR with that?

Do you intend to make this behavior turned on by default?

If it would be an opt-in feature (which is disabled by default but can be enabled by a user option), I think I will accept it after reviews and adjustments.

But if it would be an opt-out feature (which would be enabled by default when the existing nvm plugin is loaded), it is hard for me to judge. I'm unfamiliar with nvm so I wouldn't be able to properly judge whether this should be the default. Possible problems are, for example, 1) existing users of the nvm plugin might want to control the current nodejs version manually. When we suddenly change its behavior, the users would be confused. 2) I'm not sure, but could some malicious setting be put in .nvmrc? If we start to read random .nvmrc files that may be included in wild git repositories or archives downloaded from the internet, this could be a security issue. But if the other framework such as oh-my-zsh makes it default, then maybe it is safe to make it default in oh-my-bash too.

[sanzoghenzo]

Thanks for the quick response!

I found myself on the opposite side, I thought nvm itself or the nvm plugin already took care of the automatic switching so I don't have to always remember to switch from the default nodejs version to the one needed by the current project.
But I totally see your point, and this can definitely be an opt-in feature (and oh-my-zsh has a separate plugin for this).

Is it enough to use an environment variable (for example OSH_AUTO_NVM_USE) in the plugin and add it to the .bashrc? or is there another way to add configuration to a plugin? I looked a bunch of plugins but I can't find any user options there)

As for the .nvmrc security, I took a look at nvm source code and the file is not sourced, but it is read via head -n1 (so only the first line) and then parsed against valid values, so I don't see any vulnerability issues.

[akinomyoga]

Thank you for your response!

[...] (and oh-my-zsh has a separate plugin for this).

Nice, this is important information!

Is it enough to use an environment variable (for example OSH_AUTO_NVM_USE) in the plugin and add it to the .bashrc? or is there another way to add configuration to a plugin? I looked a bunch of plugins but I can't find any user options there)

Yes, if we would make it a configuration option of the nvm plugin, we are going to use a global shell variable. As for the variable name, we had been using the prefix OSH_, but now we are slowly moving to the prefix OMB_. In particular, the variables for plugins are supposed to have the name of OMB_PLUGIN_{PluginName}_{ConfigName} (see the xterm and fasd plugins for examples). We do not include the variables for specific plugins in the bashrc template by default, but instead, describe the usage in the README of each plugin to let the user add it in their bashrc by themselves.

But taking into consideration the fact that oh-my-zsh has a separate plugin for that, I now think we should follow it so that people can easily find the corresponding feature in oh-my-bash. If we make it a separate plugin, the user option to turn it on is actually the plugins array in .bashrc where the user can add the corresponding entry (e.g., nvm-auto-use).

As for the .nvmrc security, I took a look at nvm source code and the file is not sourced, but it is read via head -n1 (so only the first line) and then parsed against valid values, so I don't see any vulnerability issues.

I repeat that I'm unfamiliar with nvm, so I'm not sure what kind of values can be configurable through .nvmrc. After a quick search, is the content of .nvmrc just a single version string? Then, there must be no problem, I guess.

(Without knowing the details of .nvmrc, I had thought about the possibility that it could allow configuring a wider range of behavior. In general, even a configuration entry for the log filename of some software, for example, might possibly be exploited to overwrite and break the user's important file by specifying the path of the important file as the log filename. It's not just the vulnerabilities of the parsing phase.)

[sanzoghenzo]

But taking into consideration the fact that oh-my-zsh has a separate plugin for that, I now think we should follow it so that people can easily find the corresponding feature in oh-my-bash. If we make it a separate plugin, the user option to turn it on is actually the plugins array in .bashrc where the user can add the corresponding entry (e.g., nvm-auto-use).

I should have searched the official nvm plugin first! It also has the (opt-in) feature built-in.
This makes more sense to me, but if you think it's better to keep them separated, I'll create another plugin.

I repeat that I'm unfamiliar with nvm, so I'm not sure what kind of values can be configurable through .nvmrc. After a quick search, is the content of .nvmrc just a single version string? Then, there must be no problem, I guess.

sorry, I should have specified better... Yes, it only contains a string with the version and it is used only for that, no other configuration.

[...] the variables for plugins are supposed to have the name of OMB_PLUGIN_{PluginName}_{ConfigName} (see the xterm and fasd plugins for examples). We do not include the variables for specific plugins in the bashrc template by default, but instead, describe the usage in the README of each plugin to let the user add it in their bashrc by themselves.

This is valuable information that can be added to CONTRIBUTING.md!

[akinomyoga]

I should have searched the official nvm plugin first! It also has the (opt-in) feature built-in.
This makes more sense to me, [...]

Oh, OK. That's nice. Then, let us go in this direction.

sorry, I should have specified better... Yes, it only contains a string with the version and it is used only for that, no other configuration.

Thank you for the confirmation!

This is valuable information that can be added to CONTRIBUTING.md!

Thank you for the suggestion! Actually, the convention of OMB_THEME_{plugin}_{config} is a part of a proposal in Discussion #280. There seem to be no further opinions on the proposal, so I guess we can now freeze the convention and put it somewhere in the documents. The current content of CONTRIBUTING.md is the one inherited from the upstream oh-my-zsh. I actually think that we need to do an overhaul of CONTRIBUTING.md at some point, but incremental fixes are also welcome. By the way, there is also a PR on CONTRIBUTING.md at #90, but I don't see reasons for large changes in #90 so currently am leaving it as is.