fix(logging): combine newline-split stdout messages into one log entry#9757
fix(logging): combine newline-split stdout messages into one log entry#9757rishika0212 wants to merge 3 commits intoflutter:masterfrom
Conversation
srawlins
left a comment
There was a problem hiding this comment.
Just a preliminary review so we can kick off CI.
| // is sent by the VM as two events: 'line1\n' and 'line2'). Combine them | ||
| // into a single log entry. | ||
| // See: https://github.com/flutter/devtools/issues/9557 | ||
| if (buffer!.details!.endsWith('\n')) { |
There was a problem hiding this comment.
We should do something to avoid the repeated buffer! here. Let's make buffer private, for one thing. But since it's mutable, that probably won't help promotion.
Up above, can we make a local variable from the non-null buffer result? Like
if (buffer case final currentBuffer?) {
// Here, `currentBuffer` is non-null.
// Read from `currentBuffer` for new `logData`,
// and write to `buffer` when writing
// `buffer = null;`.
}
We might even want to move lines 858-899 into a helper function, as handle() is getting quite long now.
There was a problem hiding this comment.
Implemented, thanks
Addressed this by making the stdout buffer private and using a promoted local (if (_buffer case final currentBuffer?)) to remove repeated non-null assertions.
Also extracted buffered-event handling into a helper to keep handle() shorter; tests still pass, including newline continuation coverage.
Fixes #9557
When
debugPrint('line1\nline2')is called, the VM sends the message as two separate stdout events:'line1\n'and'line2'. The existing 1ms buffer inStdoutEventHandleronly handled the case where the second event is a lone'\n'(e.g.foofollowed by\n). It did not handle embedded newlines, so each chunk became a separate log entryin the Logging screen.
The fix extends the buffer logic: if the buffered message ends with
'\n', the next incoming message is treated as a continuation of the same print statement and combined into a single log entry._StdoutEventHandlerwas also exposed asStdoutEventHandlerwith@visibleForTestingto allow direct unit testing of the handler logic.Before:
debugPrint('line1\nline2')appeared as two separate log entries.After: It appears as a single log entry.
Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcome] or [good-first-issue] label.contributions-welcome] or [good-first-issue] label. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.