nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

get-metadata: Git Bash for Windows nits #112

Closed vsemozhetbyt closed 6 years ago

vsemozhetbyt commented 6 years ago

Windows 7 x64 Node.js 9.2.0 get-metadata 1.6.1

Please, compare the outputs in a cmd.exe:

cmd

and Git Bash for Windows:

git-bash

  1. Git Bash output lacks colors.
  2. Git Bash output duplicates metadata.
priyank-p commented 6 years ago

@vsemozhetbyt the color is probably because of git bash. Definitely duplication colors of metadata is a bug which should probably be related to git bash too.

priyank-p commented 6 years ago

@vsemozhetbyt can you let me know what does this cmd node -p -e "Boolean(process.stdout.isTTY)" prints out in Git Bash.

vsemozhetbyt commented 6 years ago

@cPhost

$ node -p -e "Boolean(process.stdout.isTTY)"
true
priyank-p commented 6 years ago

Basically the only way the metadata could be outputted to terminal two time is if if(!process.stdout.isTTY) which in this case we confirmed should not and the other time with lib/cli module. which works as expected as git bash is a terminal.

vsemozhetbyt commented 6 years ago

@cPhost I suppose this may be connected with this note here:

Note: On Windows, running Node.js in windows terminal emulators like mintty requires the usage of winpty for Node's tty channels to work correctly (e.g. winpty node.exe script.js). In "Git bash" if you call the node shell alias (node without the .exe extension), winpty is used automatically.

With node.exe:

$ node.exe -p -e "Boolean(process.stdout.isTTY)"
false
joyeecheung commented 6 years ago

Does redirecting work with Git bash? We can probably somehow check that in the code an don't write to stdout if that's the case...

joyeecheung commented 6 years ago

Or is it a bug of node core? Shouldn't it be false?

vsemozhetbyt commented 6 years ago

FWIW, with winpty and the full filename both nits are fixed:

winpty

$ winpty node.exe -p -e "Boolean(process.stdout.isTTY)"
true
priyank-p commented 6 years ago

There was a bug reported in core for this which was closed. The reason this happens is that Git Bash is using (maybe using?) pipes.

priyank-p commented 6 years ago

Finally we can add note to readme about this.