-
Notifications
You must be signed in to change notification settings - Fork 434
Use stdio log prefixing for process.stdio #5024
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
Conversation
3638bf4
to
ba8e106
Compare
fefa521
to
d27954a
Compare
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.
Few nits, but generally LGTM
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.
Tried this out locally with Wrangler and the output looks good to me.
Only tiny, non-blocking, thought is whether the configuration option could be to specify the prefixes, rather than just a boolean. This would make it really nice for people to be able to still have prefixes but avoid collisions with other prefixes.
37b98c1
to
6ad99bd
Compare
The generated output of |
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.
LGTM with one small nit on the config capnp
304792b
to
c9025c7
Compare
Actually since we are no longer changing how |
bc19e3c
to
5934c56
Compare
5934c56
to
f27a807
Compare
f27a807
to
808d523
Compare
This implements an alternative to #4965 that outputs to
console.log
with the prefix stringsstderr:
andstdout:
instead, along with a new configuration option:which can be used to both customize and disable prefixing.