-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add result.stdout
and result.stderr
#6
Conversation
subprocess[streamName].on('data', chunk => { | ||
result[streamName] += chunk; | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot use stream[Symbol.asyncIterator]
(including through stdoutLines
or through node:stream/consumers
text()
) because it uses stream.read()
under-the-hood, which means it allows only a single consumer.
|
||
const stripNewline = input => input.at(-1) === '\n' | ||
? input.slice(0, input.at(-2) === '\r' ? -2 : -1) | ||
: input; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not adding an option to disable final newline stripping.
Not sure whether that's a good idea or not. Please let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be added if there is an actual need. I prefer to keep it out until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean keeping the option out (as opposed to the newline stripping logic itself), is that correct? If so, sounds good. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I meant leaving out the option.
Conflict |
Fixed. 👍 |
This adds
result.stdout
andresult.stderr
to retrieve the full output.This also adds
error.stdout
anderror.stderr
, since knowing the output on errors is very useful.I tried to implement it in as few lines as possible, but it's a little difficult to do so.