LiteralWithRawValueLayoutRenderer - Only support RawValue when possible by snakefoot · Pull Request #6091 · NLog/NLog

snakefoot · 2026-02-16T14:09:14Z

No description provided.

coderabbitai · 2026-02-16T14:09:28Z

Caution

Review failed

The pull request is closed.

Walkthrough

Removed the raw-value success flag from LiteralWithRawValueLayoutRenderer and added [AppDomainFixedOutput]; IRawValue.TryGetRawValue now always returns true and returns the stored raw value. LayoutParser restricts conversions to types marked with AppDomainFixedOutput and updates literal merging. SimpleLayout ctor no longer explicitly nulls several fields.

Changes

Cohort / File(s)	Summary
LiteralWithRawValueLayoutRenderer `src/NLog/LayoutRenderers/LiteralWithRawValueLayoutRenderer.cs`	Added `[AppDomainFixedOutput]`. Removed `private readonly bool _rawValueSuccess`. Constructor changed from `(string text, bool rawValueSuccess, object? rawValue)` to `(string text, object? rawValue)` and now calls `: base(text)`. `IRawValue.TryGetRawValue` now sets `_rawValue` and unconditionally returns `true`.
LayoutParser & conversion logic `src/NLog/Layouts/LayoutParser.cs`	`CanBeConvertedToLiteral` now rejects renderers whose type lacks `AppDomainFixedOutputAttribute` and treats `SimpleLayout`/`LiteralLayoutRenderer` as already suitable. `MergeLiterals` iterates in reverse and promotes merged `LiteralWithRawValueLayoutRenderer` to `LiteralLayoutRenderer` when appropriate. `ConvertToLiteral` constructs `LiteralWithRawValueLayoutRenderer(text, rawValue)` using the new constructor shape when `IRawValue.TryGetRawValue` succeeds.
SimpleLayout initialization `src/NLog/Layouts/SimpleLayout.cs`	Removed explicit null assignments in the internal constructor for `_renderers`, `FixedText`, `_rawValueRenderer`, and `_stringValue_renderer` (relying on default null initialization).

Sequence Diagram(s)

sequenceDiagram
  participant Parser as LayoutParser
  participant Renderer as LayoutRenderer (candidate)
  participant RawIfc as IRawValue
  participant LiteralRaw as LiteralWithRawValueLayoutRenderer
  participant LiteralStr as LiteralLayoutRenderer

  Parser->>Renderer: CanBeConvertedToLiteral (check AppDomainFixedOutput)
  alt Renderer implements IRawValue and has AppDomainFixedOutput
    Parser->>RawIfc: TryGetRawValue()
    RawIfc-->>Parser: (true, rawValue)
    Parser->>LiteralRaw: construct(text, rawValue)
  else missing AppDomainFixedOutput or no raw value
    Parser->>LiteralStr: construct(text)
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my whiskers, drop a flag tonight,
One less bool beneath the moonlight.
Parsers hop and literals bind,
Raw values rest — neat by design. 🐇✨

🚥 Pre-merge checks | ✅ 1 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (4 files): ⚔️ `src/NLog/LayoutRenderers/LiteralWithRawValueLayoutRenderer.cs` (content) ⚔️ `src/NLog/LayoutRenderers/Wrappers/CachedLayoutRendererWrapper.cs` (content) ⚔️ `src/NLog/Layouts/LayoutParser.cs` (content) ⚔️ `src/NLog/Layouts/SimpleLayout.cs` (content) These conflicts must be resolved before merging into `dev`.	Resolve conflicts locally and push changes to this branch.
Description check	❓ Inconclusive	No pull request description was provided by the author. While this is minimal, the detailed commit messages and changes are related to the main objective of marking the renderer as AppDomainFixedOutput.	Consider adding a brief description explaining the rationale for marking LiteralWithRawValueLayoutRenderer as AppDomainFixedOutput and its performance implications.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: adding the AppDomainFixedOutput attribute to LiteralWithRawValueLayoutRenderer. It is specific, concise, and clearly summarizes the primary modification.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

src/NLog/LayoutRenderers/LiteralWithRawValueLayoutRenderer.cs (1)
49-49: Nit: xmldoc <see cref> references the base class instead of this class.

Pre-existing issue, but the doc comment says LiteralLayoutRenderer when it should say LiteralWithRawValueLayoutRenderer.
📝 Suggested fix
-        /// Initializes a new instance of the <see cref="LiteralLayoutRenderer" /> class.
+        /// Initializes a new instance of the <see cref="LiteralWithRawValueLayoutRenderer" /> class.

snakefoot · 2026-02-16T16:34:55Z

@coderabbitai full review

coderabbitai · 2026-02-16T16:35:04Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/NLog/Layouts/LayoutParser.cs`:
- Around line 693-704: The concatenation currently appends to the old lr1
instance after replacing list[i - 1] with a new LiteralLayoutRenderer when lr1
is a LiteralWithRawValueLayoutRenderer, causing the new list entry to miss the
appended text; update the merge logic in LayoutParser's loop so you append to
the actual stored renderer (e.g., rebind lr1 = (LiteralLayoutRenderer)list[i -
1] after creating the replacement, or perform the concatenation against list[i -
1].Text) before removing list[i], and ensure you only mutate the object
referenced in the list (references: LiteralLayoutRenderer,
LiteralWithRawValueLayoutRenderer, variables lr1 and lr2).

src/NLog/Layouts/LayoutParser.cs

sonarqubecloud · 2026-02-16T16:50:41Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

snakefoot added the performance label Feb 16, 2026

pull-request-size bot added the size/XS label Feb 16, 2026

snakefoot force-pushed the processid_boxing branch from b99cacf to c492587 Compare February 16, 2026 14:11

pull-request-size bot added size/S and removed size/XS labels Feb 16, 2026

snakefoot force-pushed the processid_boxing branch 2 times, most recently from b245a1f to ad2b0cc Compare February 16, 2026 14:16

snakefoot changed the title ~~ProcessIdLayoutRenderer - TryGetRawValue without boxing~~ LiteralWithRawValueLayoutRenderer - Marked as AppDomainFixedOutput Feb 16, 2026

snakefoot force-pushed the processid_boxing branch 4 times, most recently from b52d537 to 2468b65 Compare February 16, 2026 15:12

coderabbitai bot reviewed Feb 16, 2026

View reviewed changes

snakefoot force-pushed the processid_boxing branch from 2468b65 to 63306ba Compare February 16, 2026 15:19

pull-request-size bot added size/M and removed size/S labels Feb 16, 2026

snakefoot force-pushed the processid_boxing branch 7 times, most recently from 527fa7b to d33a0bd Compare February 16, 2026 16:10

coderabbitai bot reviewed Feb 16, 2026

View reviewed changes

src/NLog/Layouts/LayoutParser.cs Show resolved Hide resolved

snakefoot force-pushed the processid_boxing branch 3 times, most recently from a12b886 to 220c22b Compare February 16, 2026 16:41

LiteralWithRawValueLayoutRenderer - Marked as AppDomainFixedOutput

30c64e2

snakefoot force-pushed the processid_boxing branch from 220c22b to 30c64e2 Compare February 16, 2026 17:20

snakefoot merged commit 8769462 into NLog:dev Feb 16, 2026
1 of 4 checks passed

snakefoot mentioned this pull request Feb 16, 2026

LiteralWithRawValueLayoutRenderer - Only support RawValue when possible (Unit test) #6093

Merged

snakefoot changed the title ~~LiteralWithRawValueLayoutRenderer - Marked as AppDomainFixedOutput~~ LiteralWithRawValueLayoutRenderer - Only support RawValue when possible Feb 28, 2026

snakefoot added this to the 6.1.1 milestone Feb 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LiteralWithRawValueLayoutRenderer - Only support RawValue when possible#6091

LiteralWithRawValueLayoutRenderer - Only support RawValue when possible#6091
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:processid_boxing

snakefoot commented Feb 16, 2026

Uh oh!

coderabbitai bot commented Feb 16, 2026 •

edited

Loading

Review failed

❌ Failed checks (2 warnings, 1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Uh oh!

snakefoot commented Feb 16, 2026

Uh oh!

coderabbitai bot commented Feb 16, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

snakefoot commented Feb 16, 2026

Uh oh!

coderabbitai bot commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

❌ Failed checks (2 warnings, 1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

snakefoot commented Feb 16, 2026

Uh oh!

coderabbitai bot commented Feb 16, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 16, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai bot commented Feb 16, 2026 •

edited

Loading