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.

Sign up for GitHub

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

Jump to bottom

lib: repl for #14022 && readline fix debug warning #14494

Closed
wants to merge 4 commits into from
Closed

lib: repl for #14022 && readline fix debug warning #14494

wants to merge 4 commits into from

Conversation

robbinhan
Copy link

try to fix for #14022

multiline arrow function expressions

need input

const parseXlsx = (file) =>
    getFile(file) \
        .then(data => xlsx.read(data))

@nodejs-github-bot nodejs-github-bot added readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. labels Jul 26, 2017
@robbinhan robbinhan mentioned this pull request Jul 26, 2017
Closed
@XadillaX
Copy link
Contributor

XadillaX commented Jul 26, 2017 •
edited
Loading

Did you test your code by make -j<?> test?

vsemozhetbyt
vsemozhetbyt reviewed Jul 26, 2017
View reviewed changes
lib/repl.js Outdated
@@ -462,6 +462,12 @@ function REPLServer(prompt,
self.bufferedCommand += cmd + '\n';
self.displayPrompt();
return;
} else if (e && cmd.includes('\\')) {
cmd = cmd.substring(0, cmd.length - 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linter: missing semicolon.

vsemozhetbyt
vsemozhetbyt reviewed Jul 26, 2017
View reviewed changes
lib/readline.js Outdated
@@ -29,6 +29,7 @@

const errors = require('internal/errors');
const { debug, inherits } = require('util');
const debug = debuglog
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linter: Parsing error: Identifier 'debug' has already been declared

@robbinhan
Copy link
Author

I update code, please review it

@XadillaX
Copy link
Contributor

Have you ever ran test on your local computer? I think you'll get lint error while you running it.

TimothyGu
TimothyGu suggested changes Jul 26, 2017
View reviewed changes
lib/readline.js Outdated
@@ -28,7 +28,8 @@
'use strict';

const errors = require('internal/errors');
const { debug, inherits } = require('util');
const { debuglog, inherits } = require('util');
const debug = debuglog;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also not the way debuglog is supposed to be used.

@robbinhan
Copy link
Author

@XadillaX X Yes, I did.
@TimothyGu I update code, please check

@TimothyGu
Copy link
Member

@robbinhan These are some fairly unrelated changes. Can you split this PR for easier reviewing (one for load-editor, one for debuglog, maybe another one for the test change but only if it's independent of the other changes)?

@robbinhan
Copy link
Author

@TimothyGu OK

@robbinhan robbinhan closed this Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@TimothyGu TimothyGu TimothyGu requested changes

Assignees
No one assigned
Labels
readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@robbinhan @XadillaX @TimothyGu @vsemozhetbyt @nodejs-github-bot