You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adjusts Stringify capture handling across template formatting, JSON layout, and value formatting to consistently quote stringified values at appropriate layers, refine null handling, and prevent duplicate quotes. Updates unit tests to reflect new quoting behavior and add coverage for Stringify across types.
Refines RenderHolePositional and RenderHole: treats nulls explicitly, routes non-Stringify via serialization, and for Stringify appends quotes around formatted output; delegates to ValueFormatter accordingly.
When CaptureType.Stringify, explicitly wraps formatted value with quotes before JSON escaping, ensuring stringified properties are quoted.
Value formatting behavior src/NLog/MessageTemplates/ValueFormatter.cs
Tweaks Stringify quoting condition to add quotes only under legacy flag and when not duplicating trailing quote, reducing unintended double-quoting.
Unit tests: JSON layout tests/NLog.UnitTests/Layouts/JsonLayoutTests.cs
Adds parameterized tests to validate JSON encoding of stringified event properties across multiple types and null.
Unit tests: renderer tests/NLog.UnitTests/MessageTemplates/RendererTests.cs
Adds cases asserting "{$world}" outputs are quoted for null, numeric, and string inputs.
Unit tests: value formatter expectations tests/NLog.UnitTests/MessageTemplates/ValueFormatterTest.cs
Updates expectations for Stringify results (removal or adjustment of surrounding quotes) and null handling to align with new behavior.
Sequence Diagram(s)
sequenceDiagram
autonumber
participant App as Application
participant Logger as Logger
participant Formatter as LogMessageTemplateFormatter
participant VF as ValueFormatter
App->>Logger: Log(message, args/properties)
Logger->>Formatter: Format template holes
alt Hole CaptureType = Stringify
Formatter->>Formatter: RenderHolePositional(...)
Formatter->>Formatter: RenderHole(value, Stringify)
note right of Formatter: Adds opening quote
Formatter->>VF: FormatValue(value, Stringify)
VF-->>Formatter: Stringified content
note right of Formatter: Adds closing quote
else Other CaptureType
Formatter->>VF: FormatValue(value, captureType)
VF-->>Formatter: Serialized content or "NULL"
end
Formatter-->>Logger: Formatted message
Logger-->>App: Output
Loading
sequenceDiagram
autonumber
participant App as Application
participant Json as JsonLayout
participant VF as ValueFormatter
App->>Json: Render(LogEvent with properties)
alt Property placeholder = {$name} (Stringify)
Json->>Json: Append '"'
Json->>VF: FormatValue(property, Stringify)
VF-->>Json: Stringified content
Json->>Json: Append '"'
Json->>Json: PerformJsonEscapeIfNeeded(startPos)
else Other properties
Json->>VF: FormatValue(property, captureType)
VF-->>Json: Content
end
Json-->>App: JSON string
Loading
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
I nibble logs with tidy bites,
Quoting strings on starry nights;
Nulls now whisper, neat and small,
Curly braces, I hug them all.
Tests hop by—thump-thump, they cheer—
“Stringify’s clear!” with floppy ear.
Carrots for code, the path is dear.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%.
You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name
Status
Explanation
Title Check
✅ Passed
The title succinctly identifies the key change to ValueFormatter behavior by specifying that quotes are skipped for CaptureType.Stringify unless legacy mode is enabled, which accurately reflects the main purpose of the pull request.
Description Check
✅ Passed
The description directly references the change in CaptureType.Stringify behavior and the intent to adjust quoting logic following a previous issue, which aligns with the modifications in ValueFormatter and related tests. It is clearly related to the changeset and provides context for the behavioral change without being off-topic.
✨ Finishing touches
📝 Generate docstrings
🧪 Generate unit tests (beta)
Create PR with unit tests
Post copyable unit tests in a comment
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Followup to #5955 (Instead of magic-value logic around Format-parameter, then change the official contract)
CaptureType.Stringifyis kind of exotic, so not expecting much friction from the change (and legacy mode is available).