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

node -p process.verions display issue on Windows Cmd #27819

Closed
nanixne opened this issue May 22, 2019 · 12 comments
Closed

node -p process.verions display issue on Windows Cmd #27819

nanixne opened this issue May 22, 2019 · 12 comments
Labels
confirmed-bug Issues with confirmed bugs. console Issues and PRs related to the console subsystem. windows Issues and PRs related to the Windows platform.

Comments

@nanixne
Copy link

nanixne commented May 22, 2019

{
node: �[32m'12.3.0'�[39m,
v8: �[32m'7.4.288.27-node.17'�[39m,
uv: �[32m'1.28.0'�[39m,
zlib: �[32m'1.2.11'�[39m,
brotli: �[32m'1.0.7'�[39m,
ares: �[32m'1.15.0'�[39m,
modules: �[32m'72'�[39m,
nghttp2: �[32m'1.38.0'�[39m,
napi: �[32m'4'�[39m,
llhttp: �[32m'1.1.3'�[39m,
http_parser: �[32m'2.8.0'�[39m,
openssl: �[32m'1.1.1b'�[39m,
cldr: �[32m'35.1'�[39m,
icu: �[32m'64.2'�[39m,
tz: �[32m'2019a'�[39m,
unicode: �[32m'12.1'�[39m
}

@vsemozhetbyt
Copy link
Contributor

Can reproduce for v12.3.0 on Windows 7 x64 with cmd.exe. Output in the REPL is OK.

@ZYSzys ZYSzys added the windows Issues and PRs related to the Windows platform. label May 22, 2019
@richardlau
Copy link
Member

12.2.0 also fails but 12.1.0 is okay.

{
  node: �[32m'12.2.0'�[39m,
  v8: �[32m'7.4.288.21-node.17'�[39m,
  uv: �[32m'1.28.0'�[39m,
  zlib: �[32m'1.2.11'�[39m,
  brotli: �[32m'1.0.7'�[39m,
  ares: �[32m'1.15.0'�[39m,
  modules: �[32m'72'�[39m,
  nghttp2: �[32m'1.38.0'�[39m,
  napi: �[32m'4'�[39m,
  llhttp: �[32m'1.1.3'�[39m,
  http_parser: �[32m'2.8.0'�[39m,
  openssl: �[32m'1.1.1b'�[39m,
  cldr: �[32m'35.1'�[39m,
  icu: �[32m'64.2'�[39m,
  tz: �[32m'2019a'�[39m,
  unicode: �[32m'12.1'�[39m
}
{
  node: '12.1.0',
  v8: '7.4.288.21-node.16',
  uv: '1.28.0',
  zlib: '1.2.11',
  brotli: '1.0.7',
  ares: '1.15.0',
  modules: '72',
  nghttp2: '1.38.0',
  napi: '4',
  llhttp: '1.1.1',
  http_parser: '2.8.0',
  openssl: '1.1.1b',
  cldr: '35.1',
  icu: '64.2',
  tz: '2019a',
  unicode: '12.1'
}

@BridgeAR
Copy link
Member

I had a look at the changelog and I can't find anything obvious. Could someone verify that c2a03d58c3 has no impact? It seems the only entry that might have caused this? Otherwise it's probably best to bisected the commit.

@richardlau
Copy link
Member

Latest nightly from master also fails:

{
  node: �[32m'13.0.0-nightly2019052247c5c3da86'�[39m,
  v8: �[32m'7.4.288.27-node.17'�[39m,
  uv: �[32m'1.29.1'�[39m,
  zlib: �[32m'1.2.11'�[39m,
  brotli: �[32m'1.0.7'�[39m,
  ares: �[32m'1.15.0'�[39m,
  modules: �[32m'72'�[39m,
  nghttp2: �[32m'1.38.0'�[39m,
  napi: �[32m'4'�[39m,
  llhttp: �[32m'1.1.3'�[39m,
  http_parser: �[32m'2.8.0'�[39m,
  openssl: �[32m'1.1.1b'�[39m,
  cldr: �[32m'35.1'�[39m,
  icu: �[32m'64.2'�[39m,
  tz: �[32m'2019a'�[39m,
  unicode: �[32m'12.1'�[39m
}

@richardlau
Copy link
Member

Reverting 2b24ffa on master restores the expected behaviour. I'll open a revert PR.

@richardlau
Copy link
Member

PR: #27823

@Fishrock123
Copy link
Contributor

Can someone check if process._rawDebug(message) has the same effect in Windows CMD?

@Fishrock123 Fishrock123 added the console Issues and PRs related to the console subsystem. label May 22, 2019
@joyeecheung
Copy link
Member

Do you actually see colors in the original version of node -p?

@targos
Copy link
Member

targos commented May 22, 2019

yes:
image

@joyeecheung
Copy link
Member

@targos

Thanks. It seems we have to use uv_write for TTY on Windows so that libuv converts the color codes for us. I won't be near a Windows machine anytime soon, so I guess we could leave the patch reverted for now (or unless someone is willing to pick it up).

@bzoz
Copy link
Contributor

bzoz commented May 22, 2019

we have to use uv_write for TTY on Windows so that libuv converts the color codes for us

That is exactly what is happening, ANSI codes are emulated on Windows with WinApi calls.

@richardlau richardlau added the confirmed-bug Issues with confirmed bugs. label May 22, 2019
richardlau added a commit to richardlau/node-1 that referenced this issue May 22, 2019
ANSI sequences are emulated on Windows, so use `console.log()` for
TTY's on Windows instead of directly writing to the filedescriptor.

Fixes: nodejs#27819
@richardlau
Copy link
Member

@targos

Thanks. It seems we have to use uv_write for TTY on Windows so that libuv converts the color codes for us. I won't be near a Windows machine anytime soon, so I guess we could leave the patch reverted for now (or unless someone is willing to pick it up).

I've submitted an alternative to the full revert that falls back to console.log() for TTY on Windows: #27829

targos pushed a commit that referenced this issue May 28, 2019
This reverts commit 2b24ffa.

Fixes: #27819

PR-URL: #27823
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. console Issues and PRs related to the console subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
9 participants