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

REPL started with replServer doesn't persist history #5730

Closed
victorb opened this issue Mar 15, 2016 · 17 comments
Closed

REPL started with replServer doesn't persist history #5730

victorb opened this issue Mar 15, 2016 · 17 comments
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem.

Comments

@victorb
Copy link

victorb commented Mar 15, 2016

I'm trying to create a REPL via the programmatic library provided by the built-in repl package (https://nodejs.org/api/repl.html).

I'm doing something like this[1]:

var repl = require('repl')
repl.start(' > ')

And I'm expecting the commands I enter to be preserved if I open the repl once again later. Not finding that this is the case, I have been trying to setting both NODE_REPL_HISTORY and NODE_REPL_HISTORY by doing something like this process.env.NODE_REPL_HISTORY = 'somepath'. I've tried setting something to relative path, absolute path, a folder, a file, a non-existing file and a already existing file. But in all cases, the history is not persisted.

I'm not sure if I found a bug or if I'm doing something wrong but would appreciate any I can get.

  • Version: v5.8.0 but reproduced the same issue on every version down to v4.0.0, haven't tested lower versions than that...
  • Platform: Darwin Mak.localdomain 15.0.0 Darwin Kernel Version 15.0.0: Wed Aug 26 16:57:32 PDT 2015; root:xnu-3247.1.106~1/RELEASE_X86_64 x86_64
  • Subsystem: REPL

[1] In reality I'm doing something bigger, https://github.com/victorbjelkholm/trymodule, but figured a small test-case is better for troubleshooting.

@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Mar 15, 2016
@mscdex
Copy link
Contributor

mscdex commented Mar 15, 2016

FWIW I would expect programmatic REPLs to have to opt-in as far as saving history goes.

@victorb
Copy link
Author

victorb commented Mar 15, 2016

@mscdex That makes sense for me as well, I just would like to be able to enable it somehow, setting the environment variable would be one way, accepting it being set in opts

PS. I can also reproduce this in a docker image (mhart/alpine-node) I frequently use for testing things.

@cjihrig
Copy link
Contributor

cjihrig commented Mar 15, 2016

@victorbjelkholm what if you pass terminal: true in the REPL options? Looking at the code, it looks like that causes the history to be setup.

@victorb
Copy link
Author

victorb commented Mar 15, 2016

@cjihrig Yeah, tried that as well but to no avail 👎

Extended example:

var repl = require('repl')

repl.start({
  terminal: true,
  prompt: '> '
})

@cjihrig
Copy link
Contributor

cjihrig commented Mar 15, 2016

Ah, yea that is only for the internal REPL, sorry.

@victorb
Copy link
Author

victorb commented Mar 16, 2016

@cjihrig saving history in general for programmatic repl or that specific snippet of code? (https://github.com/nodejs/node/blob/master/lib/internal/repl.js#L58-L61)

@victorb
Copy link
Author

victorb commented Mar 16, 2016

Seems like one option would be to use the relatively small library repl.history (https://github.com/tmpvar/repl.history) but I would much rather have it in node vanilla.

@cjihrig
Copy link
Contributor

cjihrig commented Mar 16, 2016

I meant that specific snippet of code that you linked to. It's exported at the top of that file as createInternalRepl(). It looks like it might not be supported for the programmatic REPL at this time.

@victorb
Copy link
Author

victorb commented Mar 16, 2016

Oh, I see.

I'll leave this issue open as a feature request to enable history persistence in the programmatic repl and continue my day with using repl.history instead.

Thanks for the help @cjihrig 👍

@mscdex mscdex added the feature request Issues that request new features to be added to Node.js. label Mar 16, 2016
@Fishrock123
Copy link
Contributor

Imo this is kind of up to the implementor. Maybe we can have it as an option? But it would be a very significant change the programatic API.

@lance
Copy link
Member

lance commented Mar 16, 2016

How big of a change is it really? The createInternalRepl() function just uses a boolean (an odd one, if you ask me[1]) to determine whether or not to call setupHistory(). It seems to me that this function could easily be moved to the public facing repl.js, and triggered on a similar, but probably more aptly named option. E.g. option.history = '/path/to/file.txt'.

It's minor, but there is already a little bleed over from internal to external. I don't see a disadvantage to moving history management into the public API and simply make the createInternalRepl function much simpler - setting up command line defaults and such.

Edit: This is a long way of saying, I'd be willing to submit a PR for this change if there's any chance of it being accepted.

[1] The fact that writing history is a side effect of setting opts.terminal = true seems strange, but I suspect it's a vestige of the fact that this createInternalRepl is secret and only ever used by node.js at startup.

@lance
Copy link
Member

lance commented Jul 6, 2016

Given that we ultimately decided not to merge the PR for this, I'm going to close it. Feel free to reopen if there is demonstrated need.

@lance lance closed this as completed Jul 6, 2016
@f0ster
Copy link

f0ster commented Nov 5, 2017

Just chiming in after some research but late to the party, I would definitely use this if it were available in the stdlib ^^ :)

@jeffs
Copy link

jeffs commented Jan 25, 2018

For others who find this issue:

  • Calling repl.start is a no-go because it misses history
  • Calling node -r ./noderc.js is a no-go because repl.repl is unavailable
    • require('repl').repl is undefined under -r, even for interactive sessions

My work-around is to have tmux start node and send it arbitrary initialization code:

$ cat bin/repl
#!/bin/sh

file="$HOME/path/to/my/noderc.js"

stty -echo
tmux send-keys 'node; stty echo' C-m ".load $file" C-m

UPDATE:

New hack: Use a command-line flag to require a module that configures the global object, and sets a callback to configure the repl once it's been initialized.

~ $ grep repl .bashrc
alias repl='node --use-strict -r ~/home/git/env/etc/noderc.js'

~ $ cat ~/home/git/env/etc/noderc.js
setTimeout(() => global.repl.repl.ignoreUndefined = true, 1000);

global.jenny = [8, 6, 7, 5, 3, 0, 9];

This has the advantage of not adding the rc file contents to .node_repl_history every time you start a REPL.

@joniba
Copy link

joniba commented Oct 8, 2018

Any examples of how to implement loading command history into the repl that is os- and terminal-independent? I looked at repl.history but don't see how to load that history back into the repl once it loads.

@joniba
Copy link

joniba commented Oct 8, 2018

@noinkling
Copy link

noinkling commented Jan 14, 2019

I'd very much like to see this reopened. It's strange that the built-in REPL has this functionality but programmatic REPLs have no access to it and instead depend on hacky workarounds.

Also, the workaround linked above requires extra effort in order to reimplement max history size, so it's at least a little harder than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants