Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Readline bugfix, and proposal. #2737

Closed
wants to merge 1 commit into from

2 participants

@Southern

Before editing readline.js, I was having problems with closing a readline interface. The interface would "close", but would still trigger the rl.on('line', ...) event, and also echoing the command that was being input.

I notice that rl.close() only emulates closing. It doesn't actually destroy the input with input.destroy(). I left it emulating closing for the proposal.

The proposal: since rl.close() is only emulating closing, and doesn't actually destroy the input, the input still keeps the program open. I added code into rl.resume() to resume the readline without having to run rl.createInterface() again.

However, the problem with doing this is (as I pointed out) the readline makes the program hang from not destroying the input. So, I would also propose that the input be destroyed, but rl.resume() run rl.createInterface() with the input, output, and completer that was already provided.

Already spoke about this with @isaacs and @bnoordhuis, but wanted to see what everyone else has to say/add.

@indutny indutny was assigned
@indutny
Owner

Can you please add a regression test for your Pull Request?

@indutny
Owner

Otherwise, lgtm

@Southern

Actually, instead of having to destroy the input.. pausing would work. Working on adding a few other checks and things in as well, while I'm at it. Will update the repo sometime later.

Going to close this request. Will open another when finished.

@Southern Southern closed this
@Southern Southern referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@Southern Southern referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@Southern Southern referenced this pull request from a commit in Southern/node
@Southern Southern Readline proposal and bugfixes.
Related: #2737 #2756

- Removed extra newline from .question(); Users can input a
  newline if it they require it.
- Removed .close() due to it only emulating closing, causing a bug where
  readline is left open to trigger events such as .on('line', ...').
- Removed ._attemptClose()
- .pause() now triggers event .on('pause', ...)
- .resume() now triggers event .on('resume', ...)
- CTRL-C (SIGINT) in readline will now default to .pause() if no SIGINT event
  is present.
- CTRL-D (delete right) will also default to .pause() if there is nothing to
  delete (signaling the end of the file).
- Added new event `SIGTSTP`
- Added new event `SIGCONT`
- Added `resume` to `write` to resume the stream if paused.
- Docs updated.
- Updated repl.js
a0f7221
@indutny indutny referenced this pull request from a commit
@Southern Southern Readline proposal and bugfixes. Related: #2737 #2756
- Removed extra newline from .question(); Users can input a
  newline if it they require it.
- Removed .close() due to it only emulating closing, causing a bug where
  readline is left open to trigger events such as .on('line', ...').
- Removed ._attemptClose()
- .pause() now triggers event .on('pause', ...)
- .resume() now triggers event .on('resume', ...)
- CTRL-C (SIGINT) in readline will now default to .pause() if no SIGINT event
  is present.
- CTRL-D (delete right) will also default to .pause() if there is nothing to
  delete (signaling the end of the file).
- Added new event `SIGTSTP`
- Added new event `SIGCONT`
- Added `resume` to `write` to resume the stream if paused.
- Docs updated.
- Updated repl.js
ce48579
@isaacs isaacs referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@isaacs isaacs referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@isaacs isaacs referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@isaacs isaacs referenced this pull request from a commit in isaacs/node
@isaacs isaacs 2012.02.23, Version 0.7.5 (unstable)
* startup speed improvements (Maciej Małecki)

* crypto: add function getDiffieHellman() (Tomasz Buchert)

* buffer: support decoding of URL-safe base64 (Ben Noordhuis)

* Make QueryString.parse() even faster (Brian White)

* url: decode url entities in auth section (Ben Noordhuis)

* http: support PURGE request method (Ben Noordhuis)

* http: Generate Date headers on responses (Mark Nottingham)

* Fix #2762: Add callback to close function. (Mikeal Rogers)

* dgram: fix out-of-bound memory read (Ben Noordhuis)

* repl: add automatic loading of built-in libs (Brandon Benvie)

* repl: remove double calls where possible (Fedor Indutny)

* Readline improvements. Related: #2737 #2756 (Colton Baker)

* build: support shared V8 properly (T.C. Hollingsworth)

* build: disable -fomit-frame-pointer on solaris (Dave Pacheco)

* build: arch detection improvements (Nathan Rajlich)

* build: Make a fat binary for the OS X `make pkg`. (Nathan Rajlich)

* jslint src/ and lib/ on 'make test' (isaacs)
6ce41cb
@isaacs isaacs referenced this pull request from a commit
@isaacs isaacs 2012.02.23, Version 0.7.5 (unstable)
* startup speed improvements (Maciej Małecki)

* crypto: add function getDiffieHellman() (Tomasz Buchert)

* buffer: support decoding of URL-safe base64 (Ben Noordhuis)

* Make QueryString.parse() even faster (Brian White)

* url: decode url entities in auth section (Ben Noordhuis)

* http: support PURGE request method (Ben Noordhuis)

* http: Generate Date headers on responses (Mark Nottingham)

* Fix #2762: Add callback to close function. (Mikeal Rogers)

* dgram: fix out-of-bound memory read (Ben Noordhuis)

* repl: add automatic loading of built-in libs (Brandon Benvie)

* repl: remove double calls where possible (Fedor Indutny)

* Readline improvements. Related: #2737 #2756 (Colton Baker)

* build: disable -fomit-frame-pointer on solaris (Dave Pacheco)

* build: arch detection improvements (Nathan Rajlich)

* build: Make a fat binary for the OS X `make pkg`. (Nathan Rajlich)

* jslint src/ and lib/ on 'make test' (isaacs)
d384b8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 11, 2012
  1. @Southern
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 2 deletions.
  1. +9 −2 lib/readline.js
View
11 lib/readline.js
@@ -126,6 +126,7 @@ Interface.prototype.setPrompt = function(prompt, length) {
Interface.prototype.prompt = function(preserveCursor) {
+ if (this._closed) this.resume();
if (this.enabled) {
if (!preserveCursor) this.cursor = 0;
this._refreshLine();
@@ -136,7 +137,7 @@ Interface.prototype.prompt = function(preserveCursor) {
Interface.prototype.question = function(query, cb) {
- if (cb) {
+ if (typeof cb === 'function') {
this.resume();
if (this._questionCallback) {
this.output.write('\n');
@@ -153,6 +154,7 @@ Interface.prototype.question = function(query, cb) {
Interface.prototype._onLine = function(line) {
+ if (this._closed) return;
if (this._questionCallback) {
var cb = this._questionCallback;
this._questionCallback = null;
@@ -165,6 +167,7 @@ Interface.prototype._onLine = function(line) {
Interface.prototype._addHistory = function() {
+ if (this._closed) return;
if (this.line.length === 0) return '';
this.history.unshift(this.line);
@@ -204,8 +207,8 @@ Interface.prototype.close = function(d) {
if (this.enabled) {
tty.setRawMode(false);
}
- this.emit('close');
this._closed = true;
+ this.emit('close');
};
@@ -220,6 +223,8 @@ Interface.prototype.resume = function() {
if (this.enabled) {
tty.setRawMode(true);
}
+ this._closing = false;
+ this._closed = false;
};
@@ -466,6 +471,8 @@ Interface.prototype._attemptClose = function() {
// handle a write from the tty
Interface.prototype._ttyWrite = function(s, key) {
+ if (this._closed) return;
+
var next_word, next_non_word, previous_word, previous_non_word;
key = key || {};
Something went wrong with that request. Please try again.