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: save repl session in file #442

Closed
wants to merge 1 commit into from
Closed

repl: save repl session in file #442

wants to merge 1 commit into from

Conversation

btd
Copy link

@btd btd commented Jan 15, 2015

Extract history in separate function to manage it all in one place
as a result add subclass that write entries to file (by default ~/.iojs_history)

@btd btd mentioned this pull request Jan 15, 2015
@btd
Copy link
Author

btd commented Jan 15, 2015

That is for #394 to begin discussion.

@rvagg rvagg added the semver-minor PRs that contain new features and should be released in the next minor version. label Jan 23, 2015

function ReadlineFileHistory(options) {
if (!path) path = require('path');
if (!fs) fs = require('fs');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any benefit to lazy loading these modules. Could you hoist this declaration to the top?

Extract history in separate function to manage it all in one place
as a result add subclass that write entries to file (by default ~/.iojs_history).

Also allow to disable history write via env variable NODE_DISABLE_HISTORY.
@btd
Copy link
Author

btd commented Jan 31, 2015

@brendanashworth fixed.

@vkurchatkin
Copy link
Contributor

I'm in favour of #596

@cjihrig
Copy link
Contributor

cjihrig commented Feb 17, 2015

Closing per #394 (comment)

@cjihrig cjihrig closed this Feb 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants