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

repl: error thrown while writing function statement in '.editor' mode #9189

Closed
jcpst opened this issue Oct 19, 2016 · 11 comments
Closed

repl: error thrown while writing function statement in '.editor' mode #9189

jcpst opened this issue Oct 19, 2016 · 11 comments
Labels
repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.

Comments

@jcpst
Copy link

jcpst commented Oct 19, 2016

  • Version: 6.9.0
  • Platform: 64-bit windows 10

When using the REPL's .editor mode, the following error occurs when hitting the 'ENTER' key after a return statement. Tested in both CMD and Git Bash

C:\Users\joe>node -v
v6.9.0

C:\Users\joe>node
> var a = 1
undefined
> .editor
// Entering editor mode (^D to finish, ^C to cancel)
function thing (x) {
  return x
readline.js:982
            throw err;
            ^

Error: write EPIPE
    at exports._errnoException (util.js:1026:11)
    at ReadStream.Socket._writeGeneric (net.js:710:26)
    at ReadStream.Socket._write (net.js:729:8)
    at doWrite (_stream_writable.js:333:12)
    at writeOrBuffer (_stream_writable.js:319:5)
    at ReadStream.Writable.write (_stream_writable.js:246:11)
    at ReadStream.Socket.write (net.js:656:40)
    at REPLServer.<anonymous> (repl.js:479:26)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
@cjihrig
Copy link
Contributor

cjihrig commented Oct 19, 2016

cc: @princejwesley

@princejwesley
Copy link
Contributor

Works fine in macOS. I'll try this in windows 10.

@mscdex mscdex added repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform. labels Oct 19, 2016
@princejwesley
Copy link
Contributor

princejwesley commented Oct 19, 2016

Works fine in v6.5.0(irrelevant). Writing white spaces to input stream throws this error in v6.9.0.

Update: Pipe is closed when write to input stream (#8241). Disable auto alignment feature in windows will fix this issue.

@princejwesley
Copy link
Contributor

@addaleax @Fishrock123 Thinking of protecting input stream from writing with inputStream.isTTY && (inputStream.writable || process.platform !== 'win32') predicate. Any suggestion?

@addaleax
Copy link
Member

Hm, why does the REPL try to write to the input stream here in the first place? Is there any reason not to use the output stream for that?

@princejwesley
Copy link
Contributor

@addaleax User can use backspace to remove/adjust the alignment.

@addaleax
Copy link
Member

@princejwesley I’m not sure if I’m unaware of something Windows-specific here, but generally, that should work fine when writing to the output stream?

@princejwesley
Copy link
Contributor

princejwesley commented Oct 20, 2016

@addaleax can we undo the characters written to output stream? In the case of input stream, current line data is still in buffer and user can use backspace to erase white spaces added for alignment

My understanding is, output stream is read only in ttys perspective.

@addaleax
Copy link
Member

current line data is still in buffer

Whose buffer? In the context of the REPL, the TTY is generally in raw mode, so buffering is not really an issue here, and the current line is only stored by our readline.Interface instance.

So, yeah, you’re right, we shouldn’t directly write to the output stream, but call the readline write(). Something like this seems like a correct patch to me:

diff --git a/lib/repl.js b/lib/repl.js
index 620addc5ef53..283adfeb78be 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -476,7 +476,7 @@ function REPLServer(prompt,
       const matches = self._sawKeyPress ? cmd.match(/^\s+/) : null;
       if (matches) {
         const prefix = matches[0];
-        self.inputStream.write(prefix);
+        self.write(prefix);
         self.line = prefix;
         self.cursor = prefix.length;
       }
@@ -601,6 +601,7 @@ function REPLServer(prompt,
   // Wrap readline tty to enable editor mode
   const ttyWrite = self._ttyWrite.bind(self);
   self._ttyWrite = (d, key) => {
+    key = key || {};
     if (!self.editorMode || !self.terminal) {
       ttyWrite(d, key);
       return;

That last piece should probably be added anyway, too, to match the check in readline.js.

What do you think?

@princejwesley
Copy link
Contributor

@addaleax Yea, its perfect 👍

@addaleax
Copy link
Member

Okay, I’ll PR with that then :)

addaleax added a commit to addaleax/node that referenced this issue Oct 20, 2016
Instead of writing to the REPL’s input stream for the alignment
spaces in `.editor` mode, let `readline` handle the spaces
properly (echoing them using `_ttyWrite` and adding them to the
current line buffer).

Fixes: nodejs#9189
evanlucas pushed a commit that referenced this issue Nov 3, 2016
Instead of writing to the REPL’s input stream for the alignment
spaces in `.editor` mode, let `readline` handle the spaces
properly (echoing them using `_ttyWrite` and adding them to the
current line buffer).

Fixes: #9189
PR-URL: #9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 22, 2016
Instead of writing to the REPL’s input stream for the alignment
spaces in `.editor` mode, let `readline` handle the spaces
properly (echoing them using `_ttyWrite` and adding them to the
current line buffer).

Fixes: #9189
PR-URL: #9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants