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

doc/bug: repl.start() prompt doesn't default to '> ' #5385

Closed
greg-js opened this issue Feb 23, 2016 · 3 comments
Closed

doc/bug: repl.start() prompt doesn't default to '> ' #5385

greg-js opened this issue Feb 23, 2016 · 3 comments
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem.

Comments

@greg-js
Copy link

greg-js commented Feb 23, 2016

Following the contents of ./doc/api/repl.markdown, lines 217-227, I put this in repl_test.js:

const repl = require('repl');

var replServer = repl.start();
replServer.defineCommand('sayhello', {
  help: 'Say hello',
  action: function(name) {
    this.write(`Hello, ${name}!\n`);
    this.displayPrompt();
  }
});

Running that, I get

repl.js:198
    throw new Error('An options Object, or a prompt String are required');

On line 252 of ./doc/api/repl.markdown, it is noted that prompt should default to > if an options object is passed in, but not what should happen if it is called without arguments.

I took a look at ./lib/readline.js and ./lib/repl.js and the culprit for the code snippet failing (if the docs represent the intended functionality) is line 197-198 of ./lib/repl.js.

Either way, it seems to me that either the docs should be updated to reflect the code, or those lines in ./lib/repl.js should be removed to make it fall back to an empty object and thus the default > prompt. No PR submitted because I don't know which of the directions to go in.

@ChALkeR ChALkeR added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Feb 23, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Feb 23, 2016

PR coming.

@julianduque
Copy link
Contributor

I think example in docs should be updated, the options parameter isn't optional.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 23, 2016

See #5388

cjihrig added a commit to cjihrig/node that referenced this issue Feb 25, 2016
Currently, there is a check to ensure that the user either
provides an object or a string to repl.start(). The string case
is used to set a REPL prompt. However, a default of '> ' already
exists, so forcing the user to specify a prompt is a bit
redundant. This commit removes this restriction.

Fixes: nodejs#5385
PR-URL: nodejs#5388
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
Currently, there is a check to ensure that the user either
provides an object or a string to repl.start(). The string case
is used to set a REPL prompt. However, a default of '> ' already
exists, so forcing the user to specify a prompt is a bit
redundant. This commit removes this restriction.

Fixes: #5385
PR-URL: #5388
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Fishrock123 pushed a commit that referenced this issue Mar 8, 2016
Currently, there is a check to ensure that the user either
provides an object or a string to repl.start(). The string case
is used to set a REPL prompt. However, a default of '> ' already
exists, so forcing the user to specify a prompt is a bit
redundant. This commit removes this restriction.

Fixes: #5385
PR-URL: #5388
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
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. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants