Skip to content
This repository has been archived by the owner on Dec 24, 2018. It is now read-only.

Persist choice from 'nvmw use [...]' #43

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

yoannmoinet
Copy link

With these changes nvmw use [iojs|node] will persist in system path accross cmd's sessions.
As quickly discussed in #42.

Here are the steps :

  • clean path from special characters
  • remove previous .nvmw paths
  • add back special characters
  • determine which iojs or node to get
  • set PATH and NODE_PATH in System Path (won't work if System Path is more than 1024 characters, silly windows)
  • set PATH and NODE_PATH in current cmd's session

Let me now if this is useful to anyone else.

@simoneb
Copy link

simoneb commented Feb 20, 2015

Is there any reason why you are modifying the system path (with the risk of breaking it by exceeding 1024 chars)? Persisting across sessions would work without setting the system path, right?

@yoannmoinet
Copy link
Author

Setting it in the System Path is what makes it persistent across sessions.
Otherwise it won't be persistent, we could also put it into the User Path (by removing /M flag), and the result would be the same, because User Path gets merged into System Path in an environment point of view.

Then, if we exceed 1024 chars, System Path won't be updated, it will stay how it was before we tried to modify it. So we're safe here. It will just follow the same behavior as before.

@simoneb
Copy link

simoneb commented Feb 20, 2015

Depends on what you mean with session. If you mean across different users' sessions then yes, otherwise the user path is enough. I wouldn't fiddle with the system path cause it can easily break (which just happened to me by using this PR). Great job here though.

@yoannmoinet
Copy link
Author

Oh sorry for your Path, what happened?

I'll try to see if I can set it into the User Path instead.

@simoneb
Copy link

simoneb commented Feb 20, 2015

Nothing too bad, just exceeded 1024 chars and got trimmed at the end.
Considering that the limit is easily exceeded I would definitely not mess
up with the system path. Do you think you can change it? I think setting
the user path is more than enough for all practical purposes.

On Fri, Feb 20, 2015 at 3:22 PM, Yoann Moinet [email protected]
wrote:

Oh sorry for your Path, what happened?

I'll try to see if I can set it into the User Path instead.


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

I agree with you.
I really thought that by exceeding 1024 chars it wouldn't apply the modification.
Really sorry for that.

I'll check this evening for the User Path.

@simoneb
Copy link

simoneb commented Feb 20, 2015

No worries, I'm grateful that you've been doing this actually!

On Fri, Feb 20, 2015 at 3:27 PM, Yoann Moinet [email protected]
wrote:

I agree with you.
I really thought that by exceeding 1024 chars it wouldn't apply the
modification.
Really sorry for that.

I'll check this evening for the User Path.


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

Ok, now it should persit into User Path instead.
We don't touch to System Path anymore.
It was way harder than I thought 😩

@simoneb
Copy link

simoneb commented Feb 21, 2015

Not there yet unfortunately

Now using node v0.10.36 x64
find: /i: No such file or directory

@yoannmoinet
Copy link
Author

How did you get this error? Can't reproduce it on my side.
Do you have repro-steps?

@simoneb
Copy link

simoneb commented Feb 21, 2015

Not really, I just pulled-in your changes, got rid of the entries pointing
at the .nvmw folder in the system path and I started getting this error.
Maybe you still have those entries in the system path?

On Sun, Feb 22, 2015 at 12:28 AM, Yoann Moinet [email protected]
wrote:

How did you get this error? Can't reproduce it on my side.
Do you have repro-steps?


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

Do you have Cygwin installed?
Sometimes, find from Cygwin can interfer with the cmd's.
I may have found a fix for this.

@simoneb
Copy link

simoneb commented Feb 21, 2015

I have Git for Windows installed and its bin folder in the path, which
includes find.exe. So yes, that might be the issue, although I would say
quite a common one, considering that most people using nvmw will probably
be in my same condition.

On Sun, Feb 22, 2015 at 12:44 AM, Yoann Moinet [email protected]
wrote:

Do you have Cygwin installed?
Sometimes, find from Cygwin can interfer with the cmd's.
I may have found a fix for this
http://stackoverflow.com/questions/19642548/batch-find-command-not-working
.


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

Let me know if that fix your problem.
I am now using find via %windir%\system32\FIND.exe.
So it can't be conflicted with anything else.
I hope it's the only one.

@simoneb
Copy link

simoneb commented Feb 21, 2015

No wait, there must be something wrong in the script, it works the first
time I run in within a console session, then it fails:

C:\Users>nvmw use 0.12.0
Now using node v0.12.0 x64

C:\Users>nvmw use 0.12.0
Now using node v0.12.0 x64
find: /i: No such file or directory
find: .nvmw: No such file or directory

On Sun, Feb 22, 2015 at 12:48 AM, Yoann Moinet [email protected]
wrote:

Let me know if that fix your problem.
I am know using fix via %windir%\system32\FIND.exe.
So it can't be conflicted with anything else.


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

Is it still applying correct changes into the user path?
It could be just a warning saying that there is no '.nvmw' reference found.
If it's that, I could simply hide it.
On Feb 21, 2015 6:52 PM, "Simone Busoli" [email protected] wrote:

No wait, there must be something wrong in the script, it works the first
time I run in within a console session, then it fails:

C:\Users>nvmw use 0.12.0
Now using node v0.12.0 x64

C:\Users>nvmw use 0.12.0
Now using node v0.12.0 x64
find: /i: No such file or directory
find: .nvmw: No such file or directory

On Sun, Feb 22, 2015 at 12:48 AM, Yoann Moinet [email protected]
wrote:

Let me know if that fix your problem.
I am know using fix via %windir%\system32\FIND.exe.
So it can't be conflicted with anything else.


Reply to this email directly or view it on GitHub
#43 (comment).


Reply to this email directly or view it on GitHub
#43 (comment).

@simoneb
Copy link

simoneb commented Feb 22, 2015

Only the first time it is executed in a shell, the following times it fails if run again. Can you reproduce it?

@yoannmoinet
Copy link
Author

I can't reproduce it.
But it could be a simple warning log.
Are the changes still applied onto the User Path?

@simoneb
Copy link

simoneb commented Feb 22, 2015

As I said, only the first time it is run within a console session, the following times it fails

@yoannmoinet
Copy link
Author

Oh, sorry, I mis-understood your previous comment.
I'm trying to reproduce it.

@yoannmoinet
Copy link
Author

So, I've tried with a clean install, a clean Path with no reference to .nvmw whatsoever.
I've used nvmw first from the installation folder and it gets added to User Path correctly.
Then I've tried multiple combinations, installing, uninstalling, switching between version, between iojs and node.
No luck, I cannot find a way to reproduce your issue.

Can you show me what you've installed with nvmw? nvmw ls

I'm a little out of idea, it's quite hard to fix what I cannot reproduce, maybe we'll have to wait for someone else to test it, see if they can reproduce it.

@simoneb
Copy link

simoneb commented Feb 22, 2015

Ok thanks for the effort, I'll try to figure out why it fails and get back
to you then.

On Sun, Feb 22, 2015 at 5:43 PM, Yoann Moinet [email protected]
wrote:

So, I've tried with a clean install, a clean Path with no reference to
.nvmw whatsoever.
I've used nvmw first from the installation folder and it gets added to
User Path correctly.
Then I've tried multiple combinations, installing, uninstalling, switching
between version, between iojs and node.
No luck, I cannot find a way to reproduce your issue.

Can you show me what you've installed with nvmw? nvmw ls

I'm a little out of idea, it's quite hard to fix what I cannot reproduce,
maybe we'll have to wait for someone else to test it, see if they can
reproduce it.


Reply to this email directly or view it on GitHub
#43 (comment).

@simoneb
Copy link

simoneb commented Feb 22, 2015

Ok I think I found the root cause, although I didn't look into the details yet. It has to do with what you mentioned earlier, issues with Git's find.exe. I have Git's bin directory in the path, and it basically seems to depend on the order of inclusion compared to the system directory.

The reason why it worked the first time and then stopped working after the first nvmw use is that the script apparently modified the path in a way that gave Git's find precedence over Windows' find. Didn't figure out how exactly though. The fix seems to have been moving Git's bin folder at the end of system's path. The script now seems to work fine, but maybe (if at all possible) it's better to come up with a general solution, which doesn't depend on whatever find.exe it, (ehm..) finds first!

@yoannmoinet
Copy link
Author

yoannmoinet commented Feb 22, 2015 via email

@simoneb
Copy link

simoneb commented Feb 22, 2015

I didn't try the last commit, I didn't know there was one. I'll check it
out.
On 22 Feb 2015 19:55, "Yoann Moinet" [email protected] wrote:

It could definitely be the problem.
But I thought my latest commit would fix this by referencing 'find' via its
absolute path.


Reply to this email directly or view it on GitHub
#43 (comment).

@simoneb
Copy link

simoneb commented Feb 22, 2015

Looks good to me now. Thanks a lot!

On Sun, Feb 22, 2015 at 8:03 PM, Simone Busoli [email protected]
wrote:

I didn't try the last commit, I didn't know there was one. I'll check it
out.
On 22 Feb 2015 19:55, "Yoann Moinet" [email protected] wrote:

It could definitely be the problem.
But I thought my latest commit would fix this by referencing 'find' via
its
absolute path.


Reply to this email directly or view it on GitHub
#43 (comment).

@yoannmoinet
Copy link
Author

Thank you for taking the time to test it.

@yoannmoinet
Copy link
Author

@hakobera, do you have time to review this?
Thank you !

@am11
Copy link

am11 commented Mar 11, 2015

This is going to be a nice addition! 👍

won't work if System Path is more than 1024 characters, silly windows

Isn't it even less; 260 char limit? We can't be too optimistic about Windows. 😉

@yoannmoinet
Copy link
Author

Thanks @am11.

The 260 char limit is for file paths, not system path.
Meaning that your file can't be too deep into a directory.

For the system path, it is actually 2048 but we bypass this limitation by setting the user path via the registry instead.

@tssajo
Copy link

tssajo commented Apr 7, 2015

Wouldn't it be simpler to create a "directory symbolic link" with the mklink Windows command ( see http://www.howtogeek.com/howto/16226/complete-guide-to-symbolic-links-symlinks-on-windows-or-linux/ ) and place that into the path? Then just change this "directory symbolic link" when performing an nvmw use command. This way there would be no need to mess with the path setting at all...

@dallonf
Copy link

dallonf commented Sep 4, 2015

Did this PR ever go anywhere? Would be very handy to use.

@yoannmoinet
Copy link
Author

Thanks for your interest @dallonf.

I'm using it with my own fork since the beginning.
Never had any issue with it, works as expected and like a charm.

But it seemed handy only to me until you show up 😄

@Jameskmonger
Copy link

@yoannmoinet Sorry to bump this. I have checked your branch out and it works really well, but it looks like it's missing something. nvmw ls gives some information about the "current" version of node being used (stored in variable NVMW_CURRENT_TYPE), however this information doesn't seem to be displayed properly in your pull request.

For example, here is the output of nvmw ls immediately after setting a version with nvmw use:

$ nvmw use 5.12.0
Now using node v5.12.0 x64

$ nvmw ls
node:
v5.12.0

iojs:

Current: node/v5.12.0 x64

Here is the output of nvmw ls after closing and re-opening the command prompt (node -v gives v5.12.0):

$ nvmw ls
node:
v5.12.0

iojs:

Current: /none

@yoannmoinet
Copy link
Author

@Jameskmonger thank you for reporting this.
I'll have a look.
Unfortunately, I don't have much free time.
Feel free to swoop in, and make a PR if you wish.
Cheers!

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

Successfully merging this pull request may close these issues.

6 participants