Update how REPLServer uses contexts #851

Closed
wants to merge 1 commit into
from

2 participants

@weaver

Hi, I was trying to subclass REPLServer to not share a context across instances. It turns out there's no way to do this conveniently because the current REPL implementation refers to a global context variable in several places. This patch does the following:

  • Always use this.context or self.context instead of context.
  • Move resetContext to REPLServer.createContext.
  • Add REPLServer.resetContext, memoize context here.
  • Memoize exports.repl in start instead of REPLServer constructor.

My use case is a local "administration" REPL for a web service. If several people are connected to the REPL, I don't want their context shared because they may clobber each others bindings by defining variables with the same name or using ".clear".

With this patch, the following works as expected (each repl has a unique local context):

util.inherits(REPL, repl.REPLServer);
function REPL(prompt, stream) {
  repl.REPLServer.call(this, prompt, stream);
}

REPL.prototype.resetContext = function() {
  this.context = this.createContext();
};
@weaver weaver Update how REPLServer uses contexts
* Always use `this.context` or `self.context`.
* Move `resetContext` to `REPLServer.createContext`.
* Add `REPLServer.resetContext`, memoize `context` here.
* Memoize `exports.repl` in `start`.
7ff173f
@ry
ry commented Apr 11, 2011

Thanks Ben. Looks good. landed in master

@ry
ry commented Apr 12, 2011

landed in d63a551

@ry ry closed this Apr 12, 2011
@coolaj86 coolaj86 pushed a commit that referenced this pull request Apr 15, 2011
@weaver weaver Update how REPLServer uses contexts
* Always use `this.context` or `self.context`.
* Move `resetContext` to `REPLServer.createContext`.
* Add `REPLServer.resetContext`, memoize `context` here.
* Memoize `exports.repl` in `start`.

Closes GH-851.
d63a551
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment