Fix #99: Add customizable context format and value conversion#140
Fix #99: Add customizable context format and value conversion#140WarLikeLaux wants to merge 8 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #140 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 165 174 +9
===========================================
Files 11 12 +1
Lines 454 486 +32
===========================================
+ Hits 454 486 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds new configuration hooks to customize how log context is rendered and how context values are converted to strings, enabling user-defined context ordering and formatting while keeping the default output unchanged.
Changes:
- Add
setConvertToString(),setContextTemplate(), andsetContextFormat()APIs onTarget/Formatterand implement precedence rules in formatting. - Update formatter context assembly to support template-based section ordering and fully custom rendering via callable.
- Add/extend tests and documentation covering the new customization options.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Message/Formatter.php |
Implements custom context formatting/template logic and pluggable value-to-string conversion. |
src/Target.php |
Exposes new formatter customization methods at the target level. |
tests/Message/FormatterTest.php |
Adds coverage for new formatter customization behaviors and precedence/error cases. |
tests/TargetTest.php |
Adds target-level tests for new customization methods. |
tests/TestAsset/DummyTarget.php |
Ensures test target mirrors new formatter customization settings for exported formatting. |
README.md |
Documents new formatting customization options and examples. |
CHANGELOG.md |
Records the new feature in the upcoming release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@WarLikeLaux resolve conflicts, please |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR extends the logging system with three new customization methods. ChangesContext formatting and value conversion customization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/TargetTest.php (1)
261-320: ⚡ Quick winCover the non-exported path for these new setters.
All three tests go through
collectOneAndExport(), so they only exerciseDummyTarget::$exportFormatter. If a future change stopped delegating toparent::setContextFormat(),parent::setContextTemplate(), orparent::setConvertToString(), these tests would still pass while buffered, non-exported formatting regressed. Please add acollect(..., false)assertion here or a dedicated test for the non-exported path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/TargetTest.php` around lines 261 - 320, Tests only exercise the exported path via collectOneAndExport() and DummyTarget::$exportFormatter; add coverage for the non-exported (buffered) path by invoking collect(..., false) or a dedicated test that calls collect with export=false and then asserting the buffered formatting result (e.g. call formatMessages() or the buffer accessor) to match the same expected strings; update the tests testSetConvertToString, testSetContextTemplate, and testSetContextFormat (or add a new test) to call collect($level, $message, $context, false) or collect(..., false) after configuring setConvertToString, setContextTemplate, and setContextFormat so you verify parent::setConvertToString / parent::setContextTemplate / parent::setContextFormat behavior for the non-exported path as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Message/Formatter.php`:
- Around line 298-299: The current return expression in Formatter.php appends a
trailing "\n" unconditionally, causing a stray newline when $messageSection and
$commonContextString (and trace) are empty; update the return so the trailing
newline is only added when there is at least one non-empty context piece (i.e.
when $messageSection !== '' || $commonContextString !== '' || trace exists).
Concretely, adjust the return that uses $messageSection and $commonContextString
to concatenate the separator/newline only when one of those variables (or the
trace output) is non-empty so the method returns an empty string when there is
no context. Ensure you reference and use the existing $messageSection and
$commonContextString variables (and the trace check used earlier in this method)
when constructing the final string.
---
Nitpick comments:
In `@tests/TargetTest.php`:
- Around line 261-320: Tests only exercise the exported path via
collectOneAndExport() and DummyTarget::$exportFormatter; add coverage for the
non-exported (buffered) path by invoking collect(..., false) or a dedicated test
that calls collect with export=false and then asserting the buffered formatting
result (e.g. call formatMessages() or the buffer accessor) to match the same
expected strings; update the tests testSetConvertToString,
testSetContextTemplate, and testSetContextFormat (or add a new test) to call
collect($level, $message, $context, false) or collect(..., false) after
configuring setConvertToString, setContextTemplate, and setContextFormat so you
verify parent::setConvertToString / parent::setContextTemplate /
parent::setContextFormat behavior for the non-exported path as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bf644a5-c169-46ad-a11c-5e03017d83f4
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdsrc/Message/Formatter.phpsrc/Message/VarDumperValueConverter.phpsrc/Target.phptests/Message/FormatterTest.phptests/Message/VarDumperValueConverterTest.phptests/TargetTest.phptests/TestAsset/DummyTarget.php
WarLikeLaux
changed the title
|
Waiting #146 |
|
@WarLikeLaux would you please rebase it? |
|
@samdark done 🫡 |
| string|callable|null $contextFormat = null, | ||
| ?callable $stringConverter = null, |
There was a problem hiding this comment.
These could be moved to the end to keep it backwards compatible.
| string|callable|null $contextFormat = null, | ||
| ?callable $stringConverter = null, |
There was a problem hiding this comment.
These could be moved to the end to keep it backwards compatible.
| string|callable|null $contextFormat = null, | ||
| ?callable $stringConverter = null, |
There was a problem hiding this comment.
These could be moved to the end to keep it backwards compatible.
What does this PR do?
Add two constructor parameters to
Target(and its subclassesStreamTarget,PsrTarget) for customizing context output:$contextFormat(string|callable|null) - a template string for reordering context sections, or a callable for full control over context rendering$stringConverter(?callable) - custom value-to-string conversion (defaults to VarDumper-based conversion)Use cases
$stringConverterreplaces the default VarDumper-based conversion, for example to JSON-encode all context values:A
$contextFormattemplate string reorders sections via{trace},{message},{common}placeholders. Empty sections are omitted automatically:A
$contextFormatcallable gives full control over context rendering, including custom headers:$contextFormataccepts either a template string or a callable. Default output is unchanged when neither parameter is passed.Summary by CodeRabbit
New Features
Documentation