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

Why does the input data got echoed in the output? #78

Closed
knight42 opened this issue Apr 26, 2017 · 17 comments
Closed

Why does the input data got echoed in the output? #78

knight42 opened this issue Apr 26, 2017 · 17 comments

Comments

@knight42
Copy link

knight42 commented Apr 26, 2017

Running the following script

const pty = require('node-pty')
const shell = pty.spawn('bash', ['-ri'])
const stdout = process.stdout
shell.on('data', stdout.write.bind(stdout))
process.stdin.on('data', shell.write.bind(shell))

I got:

[knight@Castle bash.js]$ pwd
pwd
/tmp/bash.js
[knight@Castle bash.js]$

Is it possible to turn off the echo?

@jrop
Copy link

jrop commented Apr 26, 2017

I believe you need this line in there somewhere:

process.stdin.setRawMode(true)

@jrop
Copy link

jrop commented Apr 26, 2017

The reason this is happening is that you have two TTY's: 1) your "master" terminal, and 2) the spawned terminal (ptw.spawn(...)). Each one of these has it's own "echo" logic, and both are echoing when you type. By setting process.stdin.setRawMode(true), you are telling (1) that you will handle the echo logic*.

*if I understand this correctly.

@knight42
Copy link
Author

@jrop Thank you so much! After some exploration, I found the solution:

const pty = require('node-pty')
const shell = pty.spawn('bash', ['-ri'])
const stdout = process.stdout
const TTY = process.binding('tty_wrap').TTY
const t = new TTY(shell.fd, true)
t.setRawMode(true)
shell.on('data', stdout.write.bind(stdout))
process.stdin.on('data', shell.write.bind(shell))

@knight42
Copy link
Author

@Tyriar I guess setRawMode may be exported as a member function of class Terminal?

@jrop
Copy link

jrop commented Apr 26, 2017

@knight42 out of curiosity, did it not work to use process.stdin.setRawMode(true) directly (instead of tty_wrap)?

@knight42
Copy link
Author

@jrop In fact, it works, but not the way I expected.

If I pressed CTRL-D, I would get an error:

/tmp/bash.js/node_modules/node-pty/lib/unixTerminal.js:77
                throw err;
                ^

Error: This socket is closed
    at PipeSocket.Socket._writeGeneric (net.js:692:19)
    at PipeSocket.Socket._write (net.js:743:8)
    at doWrite (_stream_writable.js:329:12)
    at writeOrBuffer (_stream_writable.js:315:5)
    at PipeSocket.Writable.write (_stream_writable.js:241:11)
    at PipeSocket.Socket.write (net.js:670:40)
    at UnixTerminal.write (/tmp/bash.js/node_modules/node-pty/lib/unixTerminal.js:137:21)
    at emitOne (events.js:96:13)
    at ReadStream.emit (events.js:191:7)
    at readableAddChunk (_stream_readable.js:178:18)

besides, I can no longer exit bash by CTRL-C.

After all, the allocated tty is redundant, not my "master" terminal.

@Tyriar
Copy link
Member

Tyriar commented May 3, 2017

I'd like to get rid of that tty_wrap hack if possible as it's modifying the internal node code.

@knight42 maybe to get around that you can switch stdin back when you the terminal gets input?

@corwin-of-amber
Copy link
Contributor

corwin-of-amber commented Nov 27, 2021

shell.fd is now shell._fd. So indeed this should be private data. It still works if you change it; but I haven't found a way to send EOF to the controlled process in this mode. '\x04' no longer terminates the input. Trying to close shell._fd directly results in EBADF error triggered in UnixTerminal.

@Tyriar
Copy link
Member

Tyriar commented Nov 29, 2021

@corwin-of-amber fyi #143

@corwin-of-amber
Copy link
Contributor

Does sound relevant although the issue is four years old with the last "bump" two years ago.

If anyone is interested in adding new features like this one, they are welcome to do so on the rust-port branch here: https://github.com/corwin-of-amber/node-pty/tree/rust-port
(hopefully will shortly get merged into https://github.com/daniel-brenot/node-pty/tree/rust-port, and if @daniel-brenot manages to convince Microsoft, then perhaps even a branch for it will be created in the official repo.)

@Tyriar
Copy link
Member

Tyriar commented Dec 7, 2021

#143 is a very easy change, it's been marked for 4 years with help wanted 🤷. The plan is to maintain a rust branch, but it would be ideal if the APIs were identical. Plus #143 will be a TS only change.

@daniel-brenot
Copy link
Contributor

If this is a TS only change, I see no reason to hold up doing this for the rust work. The rust changes most likely wont affect the TS API much, if at all.

@corwin-of-amber
Copy link
Contributor

I agree. The only reason I suggested the rust-port branch is because this seems like the only active branch right now. But definitely a PR against https://github.com/daniel-brenot/node-pty/tree/main should work just as well.

@Tyriar
Copy link
Member

Tyriar commented Dec 8, 2021

@corwin-of-amber a trivial change like that should get merged quickly, most PRs are limited by me though and sometimes I get behind due to the volume of notifications I get.

@lafkpages
Copy link

Is there a way to keep the master TTY in TTY mode, and set Node PTY to raw mode? Basically, I want to run commands without echoing back the input commands, shell prompt, etc.

@daniel-brenot
Copy link
Contributor

Does sound relevant although the issue is four years old with the last "bump" two years ago.

If anyone is interested in adding new features like this one, they are welcome to do so on the rust-port branch here: https://github.com/corwin-of-amber/node-pty/tree/rust-port (hopefully will shortly get merged into https://github.com/daniel-brenot/node-pty/tree/rust-port, and if @daniel-brenot manages to convince Microsoft, then perhaps even a branch for it will be created in the official repo.)

The only issue with the rust branch is that I need help completing it. From what I understand Microsoft doesn't have an issue with merging it in, but ATM I just don't have the ability to get all the functionality working

@Et7f3
Copy link

Et7f3 commented Dec 22, 2023

out of curiosity, did it not work to use process.stdin.setRawMode(true) directly

I have a use case where node stdin could be plugged to pipe or pty. This function is added only when stdin is pty. I could do a check if defined but in the case of stdin is a pipe then I would still have the echo. @jrop

'\x04' no longer terminates the input

If you want that you need to return it to cooked mode before sending it. Cooked mode mean those special sequence is interpreted by the kernel driver. It is the default mode when you open a pty with node-pty @corwin-of-amber

Is there a way to keep the master TTY in TTY mode, and set Node PTY to raw mode? Basically, I want to run commands without echoing back the input commands, shell prompt, etc.

@lafkpages I exprimed the same need in #650. My workaround is to call the stty binary with pty.ptsName getter (thanks for making it public).

stty -F ${pty.ptsName} raw -echo with execSync that way once stty give me control back I know my terminal is ready in raw mode. I tested my workaroung by using an helper program that write all 256 chars to stdout and receiving it also as input and echoing. It works pretty good.

A proper solution is to change value given at creation time

node-pty/src/unix/pty.cc

Lines 212 to 241 in d6ce76a

term->c_iflag = ICRNL | IXON | IXANY | IMAXBEL | BRKINT;
if (Nan::To<bool>(info[8]).FromJust()) {
#if defined(IUTF8)
term->c_iflag |= IUTF8;
#endif
}
term->c_oflag = OPOST | ONLCR;
term->c_cflag = CREAD | CS8 | HUPCL;
term->c_lflag = ICANON | ISIG | IEXTEN | ECHO | ECHOE | ECHOK | ECHOKE | ECHOCTL;
term->c_cc[VEOF] = 4;
term->c_cc[VEOL] = -1;
term->c_cc[VEOL2] = -1;
term->c_cc[VERASE] = 0x7f;
term->c_cc[VWERASE] = 23;
term->c_cc[VKILL] = 21;
term->c_cc[VREPRINT] = 18;
term->c_cc[VINTR] = 3;
term->c_cc[VQUIT] = 0x1c;
term->c_cc[VSUSP] = 26;
term->c_cc[VSTART] = 17;
term->c_cc[VSTOP] = 19;
term->c_cc[VLNEXT] = 22;
term->c_cc[VDISCARD] = 15;
term->c_cc[VMIN] = 1;
term->c_cc[VTIME] = 0;
#if (__APPLE__)
term->c_cc[VDSUSP] = 25;
term->c_cc[VSTATUS] = 20;
I don't have windows at hand but I would call SetConsoleMode on hpc and removing flag ENABLE_PROCESSED_OUTPUT, ENABLE_VIRTUAL_TERMINAL_PROCESSING from a value I got from GetConsoleMode
HRESULT hr = CreateNamedPipesAndPseudoConsole({cols, rows}, inheritCursor ? 1/*PSEUDOCONSOLE_INHERIT_CURSOR*/ : 0, &hIn, &hOut, &hpc, inName, outName, pipeName);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants