-
Notifications
You must be signed in to change notification settings - Fork 574
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
Fix prettylog piping #644
Fix prettylog piping #644
Conversation
|
||
fmt.Printf("%s\n", bytesToWrite) | ||
} | ||
} |
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.
Could you please make this a function and reuse it for the code below (for file arg)?
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 thought about this, but it's slightly different in terms of how the errors are treated. What do you think would be the best way to do in both cases if error occurs, crash the app and display the error message, or display the output as is?
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.
Showing the line as-is is probably sensible for such a tool.
@rs fixed your comments above, here's how it works now:
My concerns here:
?
(IMO it makes the tool extra complex without the reason, but there may be reasoning I am not aware of, wonder what's your opinion on all of that) |
Fixes #628.
Basically, instead of directly piping the output from pipe, it now reads it via bufio.Scanner, which splits events by newline, so if the input is buffered, it groups logs by splitting events by newline, and in case it fails, it would just output the logs directly.
Example on testing it: