fix: node.js debugger adds stderr (but exit code is 0) -> shouldn't throw #179
Conversation
| ) | ||
| replacement = p_stdout.rstrip() | ||
| replacement = result.stdout.decode("utf-8").rstrip() |
There was a problem hiding this comment.
In this case, maybe concat stderr info too ?
There was a problem hiding this comment.
if it's merged, do I ignore this ?
concat stderr to what ?
to replacement ?
replacement = result.stdout.decode("utf-8").rstrip() + "\n" + result.stderr.decode("utf-8").rstrip()
the order will be changed, and I don't know how to keep original order as it appeared on screen
the original code did not concat stderr
There was a problem hiding this comment.
original code do this in sys.stderr.write(p_stderr)
replacement = result.stdout.decode("utf-8").rstrip() + "\n" + result.stderr.decode("utf-8").rstrip()
Maybe put stderr first.
if it's merged, do I ignore this ?
you can do a new PR.
There was a problem hiding this comment.
the original code is redundant:
it captures stderr,
stderr=subprocess.PIPE,
if stderr is not empty string
if p.wait() != 0 or p_stderr:
it writes it back to stderr
sys.stderr.write(p_stderr)
(it was only captured for condition checking)
the new code:
just don't capture it, stderr is written to stderr anyways
assuming replacement(stdout) is also written to stderr,
if you don't capture stderr, you don't have to concat it to stderr
Maybe put stderr first.
I don't see the point of this concatenation, what does it even change ?
nodejs/node-gyp#2719 (comment)
Debugger attached.
Waiting for the debugger to disconnect...