Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Adding support for .node_history and .noderc.js in $HOME and pwd #3178

Closed
wants to merge 2 commits into from

Conversation

regality
Copy link

This adds support for .node_history, which behaves like a .bash_history for the node repl. When an interactive node quits, it writes ~/.node_history and will load it the next time you start the node repl.

This patch also allows a user to create a file called .noderc.js and place it in their home directory, or the directory they start node from.

When node is started with the -i flag or without arguments, it fires up the REPL.
At this point it will check if ~/.noderc.js or <pwd>/.noderc.js exist. If they do, whatever objects they export will be copied into the repl context and be available for use.

A common use case for this will likely be loading debugging tools. For example I have the following as my noderc:

var util = require('util');

function inspect(obj) {
  console.log(util.inspect(obj, true, 10, true));
}

exports.p = inspect;

That way whenever I start the node repl, I can just type p({some:'huge object'}) without having to retype basic boilerplate every time I start node.

@vongruenigen
Copy link

+1 for that! But, why don't you just omit the .js extensions? Never saw an rc files with a file extension..

@regality
Copy link
Author

Adding the file extension seems like it would alleviate possible confusion, as well as play nice with editors. I'm not 100% sold on leaving the extension there, but is seems like the rational thing to do.

@alexdavid
Copy link

I really like this idea. +1

@regality
Copy link
Author

That last commit adds a history feature to the node REPL that behaves similiar to a bash history.

@milani
Copy link

milani commented Apr 27, 2012

+1

@Sannis
Copy link

Sannis commented Apr 28, 2012

That last commit adds a history feature to the node REPL that behaves similiar to a bash history.

Maybe it is better to chnage JSON format for history file for something more standard? grep-ing history should be allowed

if (!nodeHistory) return callback();

fs.writeFile(nodeHistory, history, function(err) {
callback();
Copy link
Member

Choose a reason for hiding this comment

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

This swallows errors. Is that intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, if someone doesn't want a history and does a chmod 000 ~/.node_history then we don't want to complain about it. Are there any use cases where we want to display errors when saving the history? I wouldn't mind it being a only displayed with a verbose flag, but I'm not aware of node having verbose logging at this time?

Copy link

Choose a reason for hiding this comment

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

We can show warning to user in this case, like it done for npm * usage in repl.

@regality
Copy link
Author

@Sannis Agreed, last commit puts each history entry on it's own line in the file. Good on call on supporting grep.

@bnoordhuis
Copy link
Member

@regality: Can you add some tests? Thanks.

@piscisaureus: Can you review this PR as well?

@Sannis
Copy link

Sannis commented Jun 7, 2012

Is therу any chance that this will be merged in near future?

@regality
Copy link
Author

regality commented Jun 7, 2012

@Sannis, I chatted with @isaacs about it a while back and he said we could expect this to be pulled in for v0.9, so probably not too soon, but it's on the todo-list.

@Sannis
Copy link

Sannis commented Jun 7, 2012

Nice!

@isaacs
Copy link

isaacs commented Jun 9, 2012

@Sannis @regality Just to clarify:

I don't recall saying "We'll merge this patch." I believe I said something to the effect of, "we want some kind of nicer experience on the repl, and not having history or setup is kind of painful."

I have not looked into this patch in depth, and we'll have to be very careful about it. A lot of people use the repl, and it's also used by the debugger, and as a utility to administer many production servers.

v0.8 is imminent, probably going to land in the next week or so. We'll discuss the best route forward once 0.8 is forked and we're into 0.9. A discussion on the nodejs-dev mailing list would be worthwhile as well, just to get some input.

@regality
Copy link
Author

regality commented Jun 9, 2012

@isaacs Sorry about the misunderstanding, I guess I just got a bit too excited.

As for this patch, I can't see it breaking anything, it should be reverse compatible with any existing node setup.

I'll go ahead and start a thread on the mailing list and we can revisit things after v0.8 lands.

@japj
Copy link

japj commented Jun 14, 2012

+1

this.history = history;
this.historyIndex = -1;
if (this.history.length > kHistorySize) {
this.history.splice(kHistorySize, Infinity);

Choose a reason for hiding this comment

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

Infinity not needed here.

Suggest replace whole function with:

Interface.prototype.setHistory = function(history) {
  this.historyIndex = -1;
  this.history = history.slice(0, kHistorySize);
}

so as to avoid side effects such as:

h = [1,2,3,4];
repl.setHistory(h);
h.unshift(0);
console.assert(repl.getHistory() === 1, "0 instead of 1"); // 0 instead of 1.

@coolaj86
Copy link

coolaj86 commented Feb 8, 2013

+1

@Nodejs-Jenkins
Copy link

Can one of the admins verify this patch?

@regality
Copy link
Author

regality commented Oct 3, 2013

Just updated this to work with master and got all tests passing. What's the chance of getting this merged in before 0.12?

@daviddias
Copy link

@regality have you run the tests for this patch? Seems failing on smartOS-x64, or at least it shows that.

@regality
Copy link
Author

regality commented Jan 2, 2014

I think the failing tests are unrelated tests that are also failing on master.

@ashaffer
Copy link

+1

@robertkowalski
Copy link

I think this is fixed in nodejs/node#1513

@jasnell
Copy link
Member

jasnell commented May 19, 2015

@robertkowalski ... should we backport or defer to the converged repo to pick it up?

@Fishrock123
Copy link
Member

Hmmm, might be best to at least choose one or the other with the combined TSC if you want to land here.

Edit: this PR is pretty old.. I'd suggest using the io.js one.

@jasnell
Copy link
Member

jasnell commented May 19, 2015

+1 ... at this point, recommend just closing this and picking up the io.js change in the converged stream.

@cjihrig
Copy link

cjihrig commented May 19, 2015

+1 to what @jasnell said.

@robertkowalski
Copy link

+1 on closing

btw: if we would backport it it would be in node 0.13 right? and 0.13 does not really exist i think or did i miss somthing?

@jasnell
Copy link
Member

jasnell commented May 19, 2015

Yes, I'd assume v0.13. It's not yet entirely clear.
On May 19, 2015 1:30 PM, "Robert Kowalski" notifications@github.com wrote:

+1 on closing

btw: if we would backport it it would be in node 0.13 right? and 0.13 does
not really exist i think or did i miss somthing?


Reply to this email directly or view it on GitHub
#3178 (comment).

@jasnell
Copy link
Member

jasnell commented May 19, 2015

Ok, closing. If we end up doing a backport we'll pick it up in a different PR.

@jasnell jasnell closed this May 19, 2015
@regality
Copy link
Author

I haven't stayed up to date on all the node.js/io.js politics, so I don't know if it would be better to keep this pull or not, plus it is has been neglected and become very old. But it is worth noting that nodejs/node#1513 requires you to set NODE_REPL_HISTORY instead of just looking in your home directory, which seems silly to me. This pull request also creates the .noderc.js functionality, which was not replicated in the other pull that handles node history.

In short, closing this may be the best course of action, but moving some of the changes to a new pull request might also be a good idea.

@Fishrock123
Copy link
Member

Yeah, perhaps it's best to open a new issue on io.js discussing that?

Do note getting the home directory is currently flawed until libuv/libuv#350 lands.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet