Add nvs support

[Loghorn]

Add support for 'nvs' to manage node versions, other than 'nvm' ('nvm-windows').

The version manager to use can be selected by runtimeVersionManager.

[msftclas]

All CLA requirements met.

[Loghorn]

@weinand Is there any chance this could be approved? If I did something wrong, please let me know.
Thank you very much.

[weinand]

Thanks for the PR.

I'm not sure how important "nvs" is in comparison with "nvm", but since the size of this PR is small, I'm happy to merge this.

One question:
Is it really necessary to introduce another launch config attribute runtimeVersionManager?
Couldn't you just probe for the existence of NVS_HOME?
Do users really have multiple version managers installed and want to switch between them?

[Loghorn]

If GitHub stars is a good metric "nvs" is a few orders of magnitude less important than "nvm", but I prefer it because it's more portable (nvm doesn't work on Windows) and it does support getting node from various repositories (you can use it to install nightly builds or chakracore).

I don't know if people uses both "nvs" and "nvm"... probably not, I guess.
I thought about simply checking for NVS_HOME, but "nvs" runtimeVersion can be incompatible with "nvm" (for example you can set it as node/9.5/x32), so I thought that explicitly setting the manager would be less confusing. Anyway, if you prefer I can change the code to check for the environment variables, runtimeVersion format and choose what is better.

[weinand]

The problem with the "runtimeVersionManager" is that it requires that all people working on a project have to use the same version manager. Because if you check in a launch configuration with a "runtimeVersionManager" attribute "nvs" and I'm using "nvm", then the version in the checked-in launch config will be ignored because I do not have "nvs" installed (since I use "nvm").

We could make the code smarter to "translate" your "nvs" settings to my "nvm" setup but I'm not sure that this is worth the effort.

On the other hand if we would not use a "runtimeVersionManager" attribute and we could agree on a version scheme that works across different version managers, then a checked-in attribute "runtimeVersion": "9.5.1" would work for you (with nvs) and for me (with nvm).

That is basically the approach I'm using for "nvm" and "nvm-windows": I assume that what is specified on macOS and linux works on Windows too because the version number format is the same.

Yes, for "nvs" you support more formats but there is still a subset that works for "nvm" too. So if those using "nvs" limit themselves to only use the simple syntax, then this will work with "nvm" too.

[Loghorn]

Agreed, I'm going to change the code 😄

[Loghorn]

Done. I'm not 100% happy with the extra code to provide the various error messages, but I'm not sure a generic "Attribute 'runtimeVersion' requires Node.js version manager 'nvm[-windows]' or 'nvs'" is better because it doesn't mention the environment variable. What do you think?

[weinand]

The refactoring looks mostly ok.
One problem is the resulting "Path" on Windows: it uses the Unix delimiter ':' instead of the correct Windows delimiter ';'.
I've pushed a fix for this to the branch and merged into master.

Thanks again for the PR.

[Loghorn]

Oops, I'm so sorry about that. Thank you for fixing it and merging the PR

[weinand]

np. The feature should be available in the next Insiders.