This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
Readline bugfix, and proposal. #2737
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
|
Can you please add a regression test for your Pull Request? |
|
Otherwise, lgtm |
|
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
added a commit
to Southern/node
that referenced
this pull request
Feb 16, 2012
Related: nodejs#2737 nodejs#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
indutny
pushed a commit
that referenced
this pull request
Feb 16, 2012
- 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
isaacs
added a commit
to isaacs/node-v0.x-archive
that referenced
this pull request
Feb 23, 2012
* 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 nodejs#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: nodejs#2737 nodejs#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)
isaacs
added a commit
that referenced
this pull request
Feb 23, 2012
* 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)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 withinput.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 intorl.resume()to resume the readline without having to runrl.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()runrl.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.