feat: oxidize streaming_brief feature#419
Conversation
0462a04 to
8204def
Compare
8204def to
bc54b3a
Compare
There was a problem hiding this comment.
Pull request overview
This PR reintroduces the streaming_brief behavior in the Rust-backed emitter/logging path, and adds infrastructure to prefix terminal output (notably for open_stream) while simplifying the printer’s message model.
Changes:
- Add
streaming_briefplumbing intoEmitterandLogListenerto allow streaming library logs in brief mode. - Refactor printer message handling to rely on a new
Message.permanentflag (and simplifyMessageTypeusage). - Add a global, printer-managed prefix that can be applied to emitted messages.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/emitter.rs |
Adds streaming_brief, updates verbosity comparisons, and introduces open_stream(prefix=...) plus prefix helpers. |
src/logs.rs |
Adds streaming_brief handling for brief-mode logging decisions and sets Message.permanent for log records. |
src/printer.rs |
Introduces Message.permanent, adds prefix support, and simplifies routing to be target-based. |
src/streams.rs |
Updates stream-to-printer messages to use permanent + MessageType::Text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tigarmo
left a comment
There was a problem hiding this comment.
LGTM thanks. I'll point out that what you called a 'niche' circumstance is really the main way that users get feedback from our apps todays (streaming brief is the default, and the build messages are shown as 'info' logs)
|
I'm only calling it niche in the sense that the exact set of requirements "must be in brief mode, with streaming_brief, and only via logging records of the right verbosity level" makes it a surprisingly narrow path a message must follow to see this behavior. When I was reviewing the original code, I doubted my own interpretation because it seemed like there should've been more to it than that, heh |
The MessageType enum gave the printer a need to understand what level of importance messages had, and had it making decisions on where a message should go. This behavior should instead belong to any object using the printer, so the permanence of a message is now communicated as an attribute on the message.\n\nThis had the nice side effect of fixing a bug where brief mode would sometimes overwrite one too many lines and clear the previous command in the shell.
2e30a07 to
e8bc46d
Compare
|
Force-pushed to clean up commit history. |
make lint && make test?I recommend reviewing per-commit.
Adds back the
streaming_brieffeature, which really only is noticeable in a niche circumstance wherestreaming_brief=Trueand a logging event with a verbosity level> logging.DEBUGis received. Below is a minimal script to demonstrate this behavior.As a fly-by, this adds back the ability to prefix messages piped with
Emitter.open_stream, as mentioned at #416 (comment). This behavior is shown in the testing script as well.