Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node must be installed or the NVM segment raises an error #212

Open
ddnexus opened this issue Mar 29, 2017 · 11 comments
Open

Node must be installed or the NVM segment raises an error #212

ddnexus opened this issue Mar 29, 2017 · 11 comments
Labels

Comments

@ddnexus
Copy link

ddnexus commented Mar 29, 2017

NVM is included in the default segments in BULLETTRAIN_PROMPT_ORDER, but there is no condition to avoid an error in case node is not installed. The condition is checking NVM, but it calls node either ways, which gives an error.
A condition at the start of the prompt function should return if node is not installed.

@dawikur
Copy link
Collaborator

dawikur commented Mar 29, 2017

I created PR witch removing nvm from default configuration: I prefer user explicitly adding segment to have it instead of silencing errors.

Let me know what you think.

@ddnexus
Copy link
Author

ddnexus commented Apr 4, 2017

While that avoids the problem, it would be inconsistent with the concept explained in comments in the code: "Each component will draw itself, and hide itself if no information needs to be shown".

@dawikur
Copy link
Collaborator

dawikur commented Apr 4, 2017

If NVM is not installed than how would you show the segment? And how is the user experience different from yours suggestion?

@ddnexus
Copy link
Author

ddnexus commented Apr 4, 2017

You shouldn't show the segment if NVM is not installed. The user should not tweak anything in order to make it work without errors, out of the box, with or without NVM/node. Then if you put an obvious suggestion that says something like "for efficiency, please remove the segments that you will never use from the BULLETTRAIN_PROMPT_ORDER" that would be OK too.

Something as simple as:

  if (( ! $+commands[node] )); then
    return      
  fi

at the beginning of the function would avoid the error and would not change the rest of the logic.

@jameshounshell
Copy link

I am not familiar with nvm but I like how Bullet Train responds to python. The python segment doesn't get triggered unless you have activated a virtual environment with virtualenv. Otherwise it makes no sense for it to display your system's node version all the time. (waste of space imho).

  1. is it as popular in nodejs to use a virtual environment for each node project?
  2. if so, what is the best bash environment variable to trigger the presence of a node virtual environment?

@jameshounshell
Copy link

Actually, I realized that the python virtualenv stuff isn't part of Bullet Train but just a part of vanilla oh-my-zsh, and it doesn't display the python version, just the fact that you have an active virtual environment.
image

@jameshounshell
Copy link

Here is the behavior with "robbyrussell" theme.
image

@dawikur
Copy link
Collaborator

dawikur commented May 16, 2017

Marked as question as this evolves a little bit into discussion 'what is the correct behavior'

@hvindin
Copy link

hvindin commented Jun 14, 2017

Just writing up a pull request for this, IMO it makes more sense to do a similar thing as is done for the AWS prompt segment but to work out the node_prompt value beforehand.

So, for example, if I have nvm or node then the regular logic will run and set the value of node_prompt and therefore it will be displayed in the prompt.

If nvm and node are not installed then nothing gets displayed because the node_prompt is never set (unless it's set explicitly by the user for some reason.)

@bicarbonate
Copy link

I had to install NVM and Node before the error: prompt_nvm:6: command not found: node Would go away. And I don;t use either of those packages nor were they listed in the plugins section of my .zshrc file.

@MRZ07
Copy link

MRZ07 commented Jul 24, 2022

I forked this theme and applied some fixes, it will show the node version without depending on NVM and fix some other issues: https://github.com/MRZ07/shinkansen.zsh-theme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants