-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat: add stderr method #441
Conversation
src/command.ts
Outdated
@@ -199,14 +199,24 @@ export default abstract class Command { | |||
return Errors.error(input, options as any) | |||
} | |||
|
|||
log(message = '', ...args: any[]): void { | |||
stdout(message = '', ...args: any[]): void { |
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.
I think we should just keep this as log
. I don't want to confuse anyone by using the name stdout
since that's already a well known method in a node process and this method doesn't behave exactly like it.
src/command.ts
Outdated
message = typeof message === 'string' ? message : inspect(message) | ||
process.stdout.write(format(message, ...args) + '\n') | ||
} | ||
} | ||
|
||
stderr(message = '', ...args: any[]): void { |
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.
Similarly to my comment above I'd rather not name this stderr
. Perhaps we can go with use error()
or logError()
?
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 debated the name of this... Problem is that error
is already taken by another method that actually throws an error and exits the process. And then logError
sorta assumes that you're only ever logging errors to stderr, which isn't a great assumption. The person asking for this feature, for example, wants to send debug logs to stderr.
logToStderr
?
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.
I don't think using logError()
and the implicit assumption of only logging errors to stderr
is necessarily a bad thing. It's the same assumption the POSIX standard makes by naming that stream stderr
instead of nonstdout
or something that encompasses more than just errors.
If users want to use stderr
for something other than errors that's up to them.
That being said, I'm fine with using logToStderr
if you'd rather do that since my whole point here is just to make it clear this method is something different than process.stderr
and logToStderr
achieves that.
Adds
stderr
method toCommand
for logging to stderrFixes #392