[Messaging] Add global output routing and configurable timestamps - #2212
[Messaging] Add global output routing and configurable timestamps#2212VeithMetro wants to merge 3 commits into
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR enhances Thunder’s messaging subsystem by adding a global default output routing option and making timestamps in direct/text outputs configurable, while updating generated configs and documentation to reflect the new schema.
Changes:
- Add global
messaging.outputrouting (e.g., handler/direct/all) and apply it across message types via wildcard routing entries. - Introduce
messaging.directsettings for direct output formatting (abbreviated,time) and propagate the newtimeflag throughMessageInfo::ToString(). - Update example/test configs, config generation templates (CMake +
Thunder.conf.in), and plugin documentation for the new configuration model.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/comrpctester/comprc-test-config.json | Removes legacy per-section abbreviated keys from test config. |
| Source/Thunder/Thunder.conf.in | Adds template support for global output and direct settings; removes legacy abbreviated defaults. |
| Source/Thunder/GenericConfig.cmake | Adds CMake options and mapping for output and direct settings; removes legacy abbreviated mappings. |
| Source/Thunder/ExampleConfigWindows.json | Removes legacy logging.abbreviated from Windows example config. |
| Source/messaging/MessageUnit.h | Adds new config fields (direct, output), a NO_TIME mode bit, and global routing application. |
| Source/messaging/MessageUnit.cpp | Wires time-enabled flag into direct output mode initialization. |
| Source/messaging/DirectOutput.h | Extends Mode() to accept the time-enabled flag. |
| Source/messaging/DirectOutput.cpp | Uses the new ToString(abbreviate, time) overload when emitting direct output. |
| Source/core/MessageStore.h | Extends ToString() APIs with a time parameter (defaulting to enabled). |
| Source/core/MessageStore.cpp | Implements time-suppression formatting across message metadata types. |
| docs/plugin/messaging.md | Documents time support and new output/direct configuration examples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
Source/core/MessageStore.cpp:351
- When
time == falseandabbreviate != ABBREVIATEDfor tracing messages, the formatted prefix drops theModule()segment entirely (unlike the time-enabled branch and other message types). This makes the output inconsistent and likely loses important context.
if (time == true) {
const string timestamp(Core::Time(TimeStamp()).ToRFC1123(true));
result = Core::Format("[%s]:[%s]:[%s:%u]:[%s]:[%s]: ", timestamp.c_str(), Module().c_str(), Core::FileNameOnly(FileName().c_str()), LineNumber(), ClassName().c_str(), Category().c_str());
}
else {
result = Core::Format("[%s:%u]:[%s]:[%s]: ", Core::FileNameOnly(FileName().c_str()), LineNumber(), ClassName().c_str(), Category().c_str());
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
Source/Thunder/GenericConfig.cmake:37
- Spelling: the CMake option descriptions say "rederict"; should be "redirect".
set(MESSAGING_STDOUT false CACHE STRING "Enable message rederict from stdout")
set(MESSAGING_STDERR false CACHE STRING "Enable message rederict from stderr")
Source/core/MessageStore.cpp:351
- When
time == falseinIStore::Tracing::ToString, the non-abbreviated branch drops the module name entirely. Disabling timestamps should not remove other metadata; it makes the output inconsistent with thetime == trueformat and with other message types.
result = Core::Format("[%s:%u]:[%s]:[%s]: ", Core::FileNameOnly(FileName().c_str()), LineNumber(), ClassName().c_str(), Category().c_str());
Source/messaging/MessageUnit.h:416
Config::Sectionstill parses anabbreviatedfield (LegacyAbbreviated), but it is never read anywhere when building_modeor applying config. This makesmessaging.*.abbreviateda silent no-op for users. Either wire it up (or map it todirect.abbreviated) or remove the field and document the deprecation explicitly.
Add(_T("settings"), &Settings);
Add(_T("abbreviated"), &LegacyAbbreviated);
Add(_T("output"), &Output);
No description provided.