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

Follow up: Readline proposal #2756

Closed
Southern opened this issue Feb 15, 2012 · 3 comments
Closed

Follow up: Readline proposal #2756

Southern opened this issue Feb 15, 2012 · 3 comments
Labels

Comments

@Southern
Copy link

This is a follow up for #2737.

I have talked with @indutny, and we have agreed on the following as changes for the readline module.

Closing the input with this.input.destroy() would not be sufficient to solve the problem with the readline module hanging when it's closed. This is because returning a new interface from rl.resume() would cause users to have to reassign the interface. For example: var rl = rl.resume(); rl.prompt();, instead of rl.prompt() (where prompt would cause the stream to resume on its own).

We think it would be best to get rid of rl.close() altogether, and instead use only rl.pause() and rl.resume(). rl.pause() and rl.resume() will also emit a pause and resume event, in case users may want to trigger some functionality when the stream is paused or resumed.

This will not effect a user wanting to catch the SIGINT event when running readline, as readline already emits a SIGINT event if there is one present. If there is not a SIGINT event to be emitted, it then defaults to attempting to close the stream (which will be changed to a pause in the proposal).

Feedback, disagreements, or commentary welcome.

/cc: @isaacs @bnoordhuis

@indutny
Copy link
Member

indutny commented Feb 15, 2012

@Southern can you please put this on nodejs email list too, I want to see if it changes things for someone

@Southern
Copy link
Author

Sure.

Edit: Added.

Southern added a commit to Southern/node that referenced this issue 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 issue 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
@Southern
Copy link
Author

Implemented as of #2757.

isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue 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 issue 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.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants