-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
repl: do not save history for non-terminal repl #1575
Conversation
When running in non-TTY mode - the `repl.history` is `undefined` and is not actually populated. Saving it will result in a crashes of subsequent repl runs. Fix: nodejs#1574
LGTM, sorry this slipped through. |
@chrisdickinson I had some time to think about it... What is your opinion on ignore |
I think my preference would be to note that we couldn't load history and that support will be disabled until the file is fixed (or removed.)
|
Why? Who cares about preserving the history? |
My reasoning is:
In any case I'd prefer to let the user be the one to blow away the file on error, in case they're relying on the persistent history to "save" their work. |
When running in non-TTY mode - the `repl.history` is `undefined` and is not actually populated. Saving it will result in a crashes of subsequent repl runs. Fix: #1574 PR-URL: #1575 Reviewed-By: Chris Dickinson <[email protected]>
Landed in ea5195c, thank you! |
Issue nodejs#1575 did introduce a check for options.terminal but this variable wasn't able to get truthy, which in turn broke persistent history completely. This changes the variable to get truthy on true terminals. Additionally, the docs and the code did differ on which environment variable was used for history. This changes the code to use NODE_REPL_HISTORY_FILE.
Issue #1575 did introduce a check for options.terminal but this variable wasn't able to get truthy, which in turn broke persistent history completely. This changes the variable to get truthy on true terminals. Additionally, the docs and the code did differ on which environment variable was used for history. This changes the code to use NODE_REPL_HISTORY_FILE. PR-URL: #1593 Reviewed-By: Chris Dickinson <[email protected]>
When running in non-TTY mode - the
repl.history
isundefined
andis not actually populated. Saving it will result in a crashes of
subsequent repl runs.
Fix: #1574