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

Nodejs STILL sends ANSI escape sequences to dumb terminals. #26187

Closed
j3pic opened this issue Feb 18, 2019 · 5 comments
Closed

Nodejs STILL sends ANSI escape sequences to dumb terminals. #26187

j3pic opened this issue Feb 18, 2019 · 5 comments

Comments

@j3pic
Copy link

j3pic commented Feb 18, 2019

My problem is described here: nodejs/node-v0.x-archive#5344

The only difference is I'm running the latest version of Node in 2019, presumably after patches have been applied that were supposed to have fixed the problem.

I'm running the REPL from the command line in an Emacs *shell* buffer. I have $NODE_DISABLE_COLORS set to 1, which doesn't matter because the �[3G escape sequence isn't a color code (those end in m).

#2712 doesn't look like it fixes the problem. That seems to just be the implementation of the $NODE_DISABLE_COLORS option.

@j3pic
Copy link
Author

j3pic commented Feb 18, 2019

As a workaround, I've created a C program to filter out the escape sequences. After compiling, you can invoke the Node REPL like this:

node | noesc
#include <stdio.h>
#include <ctype.h>

void
throw_away()
{
    for(int ch = fgetc(stdin); !feof(stdin) && !isalpha(ch); ch = fgetc(stdin));
}

main()
{
    setbuf(stdin, NULL);
    setbuf(stdout, NULL);
    while(!feof(stdin)) {
        int ch = fgetc(stdin);
	if(ch == '\033') throw_away();
	else fputc(ch, stdout);
    }
}

@mscdex
Copy link
Contributor

mscdex commented Feb 18, 2019

I can't duplicate this. If the output is a pipe or TERM=dumb is set, then REPL output is unstyled.

@j3pic
Copy link
Author

j3pic commented Feb 19, 2019

What is your stdout connected to? The escape sequences I'm seeing are for positioning the cursor, not styling the text, and they move the cursor to where it already is (just as seen in the original bug report), so if your stdout is a terminal that supports escape sequences there would be no visible effect. The REPL output would look unstyled.

@mscdex
Copy link
Contributor

mscdex commented Feb 19, 2019

Do you have a reproducible example, preferably without 3rd party modules?

@wlodzislav
Copy link
Contributor

wlodzislav commented Feb 22, 2019

I reproduced the behaviour, it's present when node is running in TTY .isTTY=true with TERM='dumb'.

There are missing checks in console and repl. And no direct support for dumb terminals in readline.

I made fix for the issue, with tests for each subsystem.

I added explicit support for dumb terminals in readline(use _ttyWriteDumb).
When TERM='dumb':

  • it doesn't use any ANSI escape codes for movement and output
  • it doesn't use color
  • it ignores all special keys except: escape, return and ctrl-c

My PR: #26261

About supported features of the dumb terminals:
infocmp dumb
wiki:Computer_terminal#Dumb_terminals

wlodzislav added a commit to wlodzislav/node that referenced this issue Mar 5, 2019
wlodzislav added a commit to wlodzislav/node that referenced this issue Mar 5, 2019
When TERM=dumb and .isTTY=true don't use ANSI escape codes
and ignore all keys, except 'escape', 'return' and 'ctrl-c'.

Fixes: nodejs#26187
wlodzislav added a commit to wlodzislav/node that referenced this issue Mar 5, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Mar 21, 2019
When TERM=dumb and .isTTY=true don't use ANSI escape codes
and ignore all keys, except 'escape', 'return' and 'ctrl-c'.

PR-URL: nodejs#26261
Fixes: nodejs#26187
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Mar 21, 2019
PR-URL: nodejs#26261
Fixes: nodejs#26187
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
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

3 participants