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

Unable to disable REPL history on Windows #25661

Closed
ralish opened this issue Jan 23, 2019 · 7 comments
Closed

Unable to disable REPL history on Windows #25661

ralish opened this issue Jan 23, 2019 · 7 comments
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.

Comments

@ralish
Copy link
Contributor

ralish commented Jan 23, 2019

  • Version: 10.15.0
  • Platform: Windows 10 x64 (1809)
  • Subsystem: REPL (lib/internal/repl.js)

Per the REPL documentation the history file path can be changed by setting the NODE_REPL_HISTORY environment variable to a valid path or disabled by setting it to the empty string.

On Windows platforms an empty environment variable is difficult to set. From my own testing:

  • A variable cannot be saved with a blank value via the System Properties UI (sysdm.cpl).
  • Setting the value to '' or "" will set it to a literal two single quotes or double quotes when using the System Properties UI.
  • Setting the value to '' will set it to a literal two single quotes when using the setx Command Prompt utility.
  • The .NET Environment.SetEnvironmentVariable method notes that an empty variable value will result in the variable being deleted if it exists or a no-op if it doesn't.

I've found that a blank variable can be set using setx with two double quotes: setx NODE_REPL_HISTORY "". I haven't tried calling the underlying Win32 SetEnvironmentVariable function directly but given the behaviour of setx it presumably would work.

It seems clear to me that setting blank environment variables is discouraged and the support for doing so is patchy at best. This being the case, it would be desirable to support an alternate value that's legal on Windows to indicate that the REPL history should not be saved. One possible option would be recognising the value '' (i.e. two single quotes) which in my view is the most intuitive setting, but there's of course other options.

Thoughts?

@ralish
Copy link
Contributor Author

ralish commented Jan 23, 2019

I spoke too soon, the setx NODE_REPL_HISTORY "" method doesn't work either. It does create a blank variable, and I can see it gets added to the Registry, but it's not actually visible in any launched process. I've confirmed this using console.log(process.env); in Node and from viewing the process environment using Process Explorer. Presumably the Win32 loader removes blank variables when creating a process.

The blank variable can be seen in the System Properties UI, presumably because it enumerates them from the Registry rather than its process environment, but it can only be deleted or a value set.

So as it stands it seems disabling the REPL history is not possible on Windows until an alternate non-blank value is recognised for the NODE_REPL_HISTORY environment variable.

@ralish ralish changed the title Add alternate recognised value to disable REPL history on Windows Unable to disable REPL history on Windows Jan 23, 2019
@vsemozhetbyt vsemozhetbyt added repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform. labels Jan 23, 2019
@bzoz
Copy link
Contributor

bzoz commented Jan 23, 2019

Try set NODE_REPL_HISTORY=

@BridgeAR
Copy link
Member

@ralish setting the environment variable to an empty string or a string that contains only whitespace should work just fine (as documented). I guess the latter should work even if Windows does not recognize empty strings.

@BridgeAR BridgeAR added the question Issues that look for answers. label Jan 23, 2019
@bzoz
Copy link
Contributor

bzoz commented Jan 23, 2019

Sigh, my bad, missed a space: "set NODE_REPL_HISTORY= " (with a space after '='). That will disable repl.

@ralish
Copy link
Contributor Author

ralish commented Jan 23, 2019

@bzoz: You're right, this does work, thanks!
@BridgeAR: Yep, the fact whitespace is trimmed is the key.

Can I suggest at least a documentation update to make it a bit more explicit that on Windows one or more space characters should be used? Right now it's implied on closer reading with the whitespace trimming, but making it concrete would I think be helpful as the empty string is not valid on Windows.

@BridgeAR
Copy link
Member

@ralish please feel encouraged to open a pull request to improve the wording :-)

@ralish
Copy link
Contributor Author

ralish commented Jan 23, 2019

@BridgeAR: Sure, done in PR #25672.

@BridgeAR BridgeAR added doc Issues and PRs related to the documentations. and removed question Issues that look for answers. labels Jan 23, 2019
addaleax pushed a commit that referenced this issue Jan 28, 2019
Environment variables with empty values are not permitted on Windows. As
such, to disable persistent REPL history one or more spaces should be
used. Node will trim whitespace from the variable, resulting in a blank
variable at runtime and the desired behaviour.

PR-URL: #25672
Fixes: #25661
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
BethGriggs pushed a commit that referenced this issue Apr 29, 2019
Environment variables with empty values are not permitted on Windows. As
such, to disable persistent REPL history one or more spaces should be
used. Node will trim whitespace from the variable, resulting in a blank
variable at runtime and the desired behaviour.

PR-URL: #25672
Fixes: #25661
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
BethGriggs pushed a commit that referenced this issue May 10, 2019
Environment variables with empty values are not permitted on Windows. As
such, to disable persistent REPL history one or more spaces should be
used. Node will trim whitespace from the variable, resulting in a blank
variable at runtime and the desired behaviour.

PR-URL: #25672
Fixes: #25661
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
MylesBorins pushed a commit that referenced this issue May 16, 2019
Environment variables with empty values are not permitted on Windows. As
such, to disable persistent REPL history one or more spaces should be
used. Node will trim whitespace from the variable, resulting in a blank
variable at runtime and the desired behaviour.

PR-URL: #25672
Fixes: #25661
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants