Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readline.question #4833

Closed
benjamingr opened this issue Jan 24, 2016 · 7 comments
Closed

readline.question #4833

benjamingr opened this issue Jan 24, 2016 · 7 comments
Labels
doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module.

Comments

@benjamingr
Copy link
Member

So, I found readline.question today, and it looks really odd.

First, it doesn't follow the (err, data) nodeback convention at all (where do errors go?).
Second, it doesn't really feel like something that should be in core.

Should it be soft-deprecated?
Should it be "fixed" to conform to the node-back convention (not very likely)?

@ChALkeR ChALkeR added the readline Issues and PRs related to the built-in readline module. label Jan 24, 2016
@ChALkeR
Copy link
Member

ChALkeR commented Jan 24, 2016

It's used by about 0.3% of modules, including babel, pm2, webdriverio, replace, atom-package-manager.

@benjamingr
Copy link
Member Author

Yes, it's definitely used in quite a bit of code. Still, the inconsistency bugs me. I wonder if soft deprecation would work here if it's even something we're interested in.

@Fishrock123
Copy link
Contributor

I've used it and it is useful. It is a bit odd I suppose, but you would never catch an error there afaik. It simply doesn't make any sense to me why there would ever be one.

@benjamingr
Copy link
Member Author

Well, even a method that doesn't raise errors (and whether or not readline can/should do that is debateable) - it should have an (err, data) signature for consistency.

I admit it's useful and I'm not for removing it.

Maybe put a big note in the docs saying it's inconsistenct with other node style APIs?

@Fishrock123
Copy link
Contributor

I wouldn't be against putting a note, but I don't think it should actually have an errback, since it would always be undefined.

@abdulhannanali
Copy link

Is there a synchronous substitute of readline.question

jasnell added a commit to jasnell/node that referenced this issue Jun 6, 2016
Clarify that readline's rl.question() callback does not
use the err back pattern.

Fixes: nodejs#4833
@jasnell jasnell closed this as completed in c300ba2 Jun 7, 2016
@abdulhannanali
Copy link

Good job closing this!

evanlucas pushed a commit that referenced this issue Jun 15, 2016
Clarify that readline's rl.question() callback does not
use the err back pattern.

Fixes: #4833
PR-URL: #7022
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@sam-github sam-github added doc Issues and PRs related to the documentations. and removed doc Issues and PRs related to the documentations. labels Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module.
Projects
None yet
Development

No branches or pull requests

6 participants