Skip to content
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

OStreamMessageHandler : Prefix every line with message level #1399

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

johnhaddon
Copy link
Member

This allows log parsers to correctly identify the type of each line for syntax highlighting and the like. The immediate motivation is for a new LocalJobs panel in Gaffer, that captures the output from LocalDispatcher subprocesses and turns it back into messages for display into a GafferUI.MessagesWidget.

@johnhaddon johnhaddon self-assigned this Dec 6, 2023
@johnhaddon johnhaddon marked this pull request as ready for review December 6, 2023 10:58
This allows log parsers to correctly identify the type of each line for syntax highlighting and the like. The immediate motivation is for a new LocalJobs panel
in Gaffer, that captures the output from LocalDispatcher subprocesses and turns it back into messages for display into a GafferUI.MessagesWidget.
@danieldresser-ie
Copy link
Contributor

All looks reasonable. If I have any nitpick, it would be that looking at the sample output in the test case, it is a bit confusing to immediately associate the second line line of the message with the context of the first when there are a mixture of single line and multiline messages coming out together. I would consider adding an extra newline after any message that covers more than 1 line, but I don't feel strongly about this.

@johnhaddon
Copy link
Member Author

I would consider adding an extra newline after any message that covers more than 1 line, but I don't feel strongly about this.

I also wonder about either repeating the context on each line, or indenting secondary lines by the length of the context, so they line up with the message on the first line. But I think this is good enough for now - see how you feel when you see it I the real world in the MessagesWidget in Gaffer, and we can consider any tweaks then.

@johnhaddon johnhaddon merged commit a9850fd into ImageEngine:RB-10.5 Dec 7, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants