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

Cannot disable REPL history on Windows by setting NODE_REPL_HISTORY_FILE to blank string #4522

Closed
ghost opened this issue Jan 3, 2016 · 15 comments
Labels
repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.

Comments

@ghost
Copy link

ghost commented Jan 3, 2016

I attempted to set NODE_REPL_HISTORY_FILE to a blank string as a Windows System Environment Variable, but the dialog would not allow me to enter a blank string as a value, so I entered a space figuring that Node.js would probably trim the string before using it.

However, when I ran Node again, it instead created a file named (a blank space) that I cannot delete or move now. Anytime I try to delete the file, Explorer acts like I'm trying to delete my whole user profile.

@ghost ghost changed the title Attempting to NODE_REPL_HISTORY_FILE on Windows caused filesystem corruption Attempting to disable NODE_REPL_HISTORY_FILE on Windows caused filesystem corruption Jan 3, 2016
@Trott Trott added the windows Issues and PRs related to the Windows platform. label Jan 3, 2016
@ghost
Copy link
Author

ghost commented Jan 3, 2016

I was able to delete the file by opening a command prompt and running the following in my profile directory:

dir /a /x

There, I could see one file that had a space for a filename, but it also had an "8.2 short filename" that was not a space. Using that name, I was able to delete the file:

del 99A4~1

The file system was not actually corrupted according to a disk scan. Explorer was simply trimming the filename space and trying to delete my user profile. Luckily, I did not hit "continue" when it told me I needed Admin access to delete it, but some other user might not. So, it would probably be good to never create files with a space as a name on Windows.

@ghost ghost changed the title Attempting to disable NODE_REPL_HISTORY_FILE on Windows caused filesystem corruption Cannot disable REPL on Windows by setting NODE_REPL_HISTORY_FILE to blank string Jan 3, 2016
@ghost
Copy link
Author

ghost commented Jan 3, 2016

I was able to set the environment variable by opening an Admin command prompt and running:

SETX NODE_REPL_HISTORY_FILE "" /M

When I checked the Windows System Environment Variable dialog, I could see it there as well.

However, Node.js still creates a history file in my user profile when I open a new command prompt and run the REPL.

@ghost ghost changed the title Cannot disable REPL on Windows by setting NODE_REPL_HISTORY_FILE to blank string Cannot disable REPL history on Windows by setting NODE_REPL_HISTORY_FILE to blank string Jan 3, 2016
@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Jan 4, 2016
@evanlucas
Copy link
Contributor

Sorry for the trouble you are having. On which version of node are you seeing this issue?

@ghost
Copy link
Author

ghost commented Jan 4, 2016

I am using v4.2.3 currently (installed by nvm-windows).

@Fishrock123
Copy link
Contributor

I'll try to look at this tomorrow.

@evanlucas
Copy link
Contributor

What's weird about this is that we aren't opening process.env.NODE_REPL_HISTORY_FILE anymore. We simply call fs.readFileSync on it. We are now using process.env.NODE_REPL_HISTORY for the file path. I did notice that it is not being trimmed though, so I went ahead and submitted a PR to fix that.

@Fishrock123
Copy link
Contributor

I was able to set the environment variable by opening an Admin command prompt and running:

SETX NODE_REPL_HISTORY_FILE "" /M

When I checked the Windows System Environment Variable dialog, I could see it there as well.

However, Node.js still creates a history file in my user profile when I open a new command prompt and run the REPL.

@waynebloss What's process.env say when you do this?

@ghost
Copy link
Author

ghost commented Jan 7, 2016

@Fishrock123 It's not listed.

I tried the same with NODE_REPL_HISTORY and I could not see it listed in process.env either (each time, exiting my terminal after creating the environment variable and re-opening it, to make sure it would be picked up).

Then I tried creating a new system environment variable called ASDFASDF setting it to "hello" and when I ran node and looked at process.env I could see my new variable there.

@ghost
Copy link
Author

ghost commented Jan 7, 2016

I am also on Windows 10 and I am running 32-bit node. I have not tried this on Windows 8 or 7 but I doubt it's limited to 10.

No big deal for me though - I just wanted to delete the history when I close the REPL so that I don't confuse a past session's history with the current one.

@Fishrock123
Copy link
Contributor

@waynebloss So the issue here is seeing an "empty" ENV variable on windows? That could be an issue in libuv or something.

@ghost
Copy link
Author

ghost commented Jan 7, 2016

@Fishrock123 Yes, the issue occurs because any empty ENV vars in Windows do not show up in process.env. One more test setting ASDFASDF to "" evidenced that. Sorry - I wasn't even thinking about the actual problem before when I went through the motions of testing and reporting this here...(I'm doing other things!)

The technical problem probably lies with libuv/Windows - but is checking for a blank ENV var to disable a feature when no ENV var is needed to enable the feature in the first place really a good idea on any platform? I'd much much much rather Node.js require the ENV var to be set in order to enable the feature in the first place, else you only get per-session history. But that's just my opinion. So I'll STFU now and not complain - thanks for making stuff work on Windows guys! :)

@MylesBorins
Copy link
Contributor

@waynebloss are you still having this problem? it may have been fixed in v5.5.0

@ghost
Copy link
Author

ghost commented Jan 28, 2016

@thealphanerd I'm not using v5 yet. I'll probably just wait until v6. If it's not going to be fixed for v4 then I don't really care about it right now so everybody feel free to just close this issue if you want.

@MylesBorins
Copy link
Contributor

@waynebloss the commit in question has been backported to the staging branch for LTS. You should likely see it land in LTS in the next 2 weeks

@evanlucas
Copy link
Contributor

Closing as this was released in v4.3.1. Thanks for reporting @waynebloss! Please let us know if you have any more issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants