Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

LogMessageTemplateFormatter - Also use IValueFormatter for positional templates #5955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
snakefoot merged 1 commit into from
Aug 9, 2025
Merged

LogMessageTemplateFormatter - Also use IValueFormatter for positional templates #5955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion 2 src/NLog/Config/LoggingConfigurationParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private void SetNLogElementSettings(ILoggingConfigurationElement nlogConfig)
var sortedList = CreateUniqueSortedListFromConfig(nlogConfig);

CultureInfo? defaultCultureInfo = DefaultCultureInfo ?? LogFactory._defaultCultureInfo;
bool? parseMessageTemplates = null;
bool? parseMessageTemplates = LogFactory.ServiceRepository.ResolveParseMessageTemplates();
bool internalLoggerEnabled = false;
bool autoLoadExtensions = false;
foreach (var configItem in sortedList)
Expand Down
10 changes: 3 additions & 7 deletions 10 src/NLog/Internal/LogMessageTemplateFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,13 @@ internal string Render(ref TemplateEnumerator templateEnumerator, IFormatProvide

private void RenderHolePositional(StringBuilder sb, in Hole hole, IFormatProvider? formatProvider, object? value)
{
if (value is null)
{
sb.Append("NULL");
}
else if (hole.CaptureType == CaptureType.Normal)
if (hole.CaptureType == CaptureType.Serialize)
{
MessageTemplates.ValueFormatter.FormatToString(value, hole.Format, formatProvider, sb);
RenderHole(sb, CaptureType.Serialize, hole.Format, formatProvider, value);
}
else
{
RenderHole(sb, hole.CaptureType, hole.Format, formatProvider, value);
RenderHole(sb, CaptureType.Stringify, hole.Format ?? string.Empty, formatProvider, value);
}
Comment on lines +282 to 289

@coderabbitai coderabbitai Bot Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Don’t force CaptureType.Stringify for positional holes; preserve the hole’s capture type and format

Forcing Stringify changes behavior (e.g., quotes strings) and diverges from the hole’s intended capture semantics. It also conflicts with the tests’ shift to treat null vs empty format distinctly.

Refactor to delegate directly to RenderHole(hole, ...) and stop coalescing format to empty string:

-        private void RenderHolePositional(StringBuilder sb, in Hole hole, IFormatProvider? formatProvider, object? value)
-        {
-            if (hole.CaptureType == CaptureType.Serialize)
-            {
-                RenderHole(sb, CaptureType.Serialize, hole.Format, formatProvider, value);
-            }
-            else
-            {
-                RenderHole(sb, CaptureType.Stringify, hole.Format ?? string.Empty, formatProvider, value);
-            }
-        }
+        private void RenderHolePositional(StringBuilder sb, in Hole hole, IFormatProvider? formatProvider, object? value)
+        {
+            // Preserve capture semantics and pass through original format (including null)
+            RenderHole(sb, hole, formatProvider, value);
+        }

This avoids a silent behavior change for positional templates. If forcing Stringify was intentional, please add/adjust tests to assert quoted string behavior for positional holes under the forced renderer.

🏁 Script executed:

#!/bin/bash
# List all occurrences of RenderHole in the formatter to inspect available overloads
grep -R "RenderHole(" -n src/NLog/Internal/LogMessageTemplateFormatter.cs

Length of output: 788

#!/bin/bash
# Display the RenderHolePositional method and related overloads for context
sed -n '250,330p' src/NLog/Internal/LogMessageTemplateFormatter.cs
#!/bin/bash
# Show the RenderHolePositional method for full context
sed -n '260,300p' src/NLog/Internal/LogMessageTemplateFormatter.cs

Preserve original CaptureType and Format in RenderHolePositional

In src/NLog/Internal/LogMessageTemplateFormatter.cs, the RenderHolePositional overload currently forces all non-Serialize holes to use CaptureType.Stringify and coalesces a null format to string.Empty, which:

  • Overrides any other capture semantics (e.g., if new CaptureType values are introduced in the future)
  • Conflates a deliberately blank format ("") with a missing one (null), against the intent of recent tests

Refactor RenderHolePositional to call the existing RenderHole(StringBuilder, in Hole, IFormatProvider?, object?) overload, preserving both the hole’s CaptureType and its raw Format (including null):

--- a/src/NLog/Internal/LogMessageTemplateFormatter.cs
+++ b/src/NLog/Internal/LogMessageTemplateFormatter.cs
@@ -279,10 +279,7 @@ namespace NLog.Internal
         private void RenderHolePositional(StringBuilder sb, in Hole hole, IFormatProvider? formatProvider, object? value)
         {
-            if (hole.CaptureType == CaptureType.Serialize)
-            {
-                RenderHole(sb, CaptureType.Serialize, hole.Format, formatProvider, value);
-            }
-            else
-            {
-                RenderHole(sb, CaptureType.Stringify, hole.Format ?? string.Empty, formatProvider, value);
-            }
+            // Preserve the hole’s original capture semantics and format (including null)
+            RenderHole(sb, hole, formatProvider, value);
         }

If the forced Stringify behavior was intentional for positional placeholders, please document this in a comment and add tests to explicitly assert quoted output under those conditions.

🤖 Prompt for AI Agents 
In src/NLog/Internal/LogMessageTemplateFormatter.cs around lines 282 to 289, the
RenderHolePositional method currently overrides the hole's CaptureType to
Stringify for non-Serialize cases and replaces null Format with string.Empty,
which loses the original CaptureType and conflates null and empty formats. To
fix this, refactor RenderHolePositional to call the existing RenderHole overload
that accepts the Hole struct directly, passing the hole as is to preserve its
original CaptureType and raw Format (including null). If the current forced
Stringify behavior is intentional, add a comment explaining this and include
tests to verify the expected output.

}

Expand Down
51 changes: 28 additions & 23 deletions 51 src/NLog/MessageTemplates/ValueFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,12 @@ public bool FormatValue(object? value, string? format, CaptureType captureType,
}
case CaptureType.Stringify:
{
builder.Append('"');
FormatToString(value, null, formatProvider, builder);
builder.Append('"');
bool includeQuotes = format is null || _legacyStringQuotes;
if (includeQuotes)
builder.Append('"');
FormatToString(value, format, formatProvider, builder);
if (includeQuotes)
builder.Append('"');
return true;
}
default:
Expand Down Expand Up @@ -165,34 +168,37 @@ private bool SerializeSimpleObject(object? value, string? format, IFormatProvide
return true;
}

if (value is null)
// Optimize for types that are pretty much invariant in all cultures when no format-string
if (value is IConvertible convertible)
{
builder.Append("NULL");
SerializeConvertibleObject(convertible, format, formatProvider, builder);
return true;
}

// Optimize for types that are pretty much invariant in all cultures when no format-string
if (value is IConvertible convertibleValue)
if (value is IFormattable formattable)
{
SerializeConvertibleObject(convertibleValue, format, formatProvider, builder);
#if !NETFRAMEWORK
if (string.IsNullOrEmpty(format))
builder.AppendFormat(formatProvider, "{0}", formattable); // Support ISpanFormattable
else
#endif
builder.Append(formattable.ToString(format, formatProvider));
return true;
}
else
{
if (!string.IsNullOrEmpty(format) && value is IFormattable formattable)
{
builder.Append(formattable.ToString(format, formatProvider));
return true;
}

if (convertToString)
{
SerializeConvertToString(value, formatProvider, builder);
return true;
}
if (value is null)
{
builder.Append("NULL");
return true;
}

return false;
if (convertToString)
{
SerializeConvertToString(value, formatProvider, builder);
return true;
}

return false;
}

private void SerializeConvertibleObject(IConvertible value, string? format, IFormatProvider? formatProvider, StringBuilder builder)
Expand Down Expand Up @@ -371,8 +377,7 @@ private void SerializeCollectionItem(object item, string? format, IFormatProvide
/// <param name="builder">Append to this</param>
public static void FormatToString(object? value, string? format, IFormatProvider? formatProvider, StringBuilder builder)
{
var stringValue = value as string;
if (stringValue != null)
if (value is string stringValue)
{
builder.Append(stringValue);
}
Expand Down
46 changes: 44 additions & 2 deletions 46 tests/NLog.UnitTests/MessageTemplates/RendererTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace NLog.UnitTests.MessageTemplates
{
using System;
using System.Globalization;
using NLog.Config;
using Xunit;

public class RendererTests
Expand Down Expand Up @@ -105,6 +106,23 @@ public void RenderDateTime(string input, string arg, string expected)
RenderAndTest(input, culture, new object[] { dt }, expected);
}

[Theory]
[InlineData("test {0}", "1970-01-01", "test 1970-01-01 00:00:00")]
[InlineData("test {0:u}", "1970-01-01", "test 1970-01-01 00:00:00Z")]
[InlineData("test {0:MM/dd/yy}", "1970-01-01", "test 01/01/70")]
public void RenderDateTimeOverride(string input, string arg, string expected)
{
var culture = CultureInfo.InvariantCulture;

var logFactory = new LogFactory();
var orgValueFormatter = logFactory.ServiceRepository.ResolveService<IValueFormatter>();
var newValueFormatter = new OverrideValueFormatter(orgValueFormatter);
logFactory.ServiceRepository.RegisterSingleton<IValueFormatter>(newValueFormatter);

DateTime dt = DateTime.Parse(arg, culture, DateTimeStyles.AdjustToUniversal);
RenderAndTest(input, culture, new object[] { dt }, expected, logFactory);
}

[Theory]
[InlineData("test {0:c}", "1:2:3:4.5", "test 1.02:03:04.5000000")]
[InlineData("test {0:hh\\:mm\\:ss\\.ff}", "1:2:3:4.5", "test 02:03:04.50")]
Expand All @@ -127,12 +145,36 @@ public void RenderDateTimeOffset(string input, string arg, string expected)
RenderAndTest(input, culture, new object[] { dto }, expected);
}

private static void RenderAndTest(string input, CultureInfo culture, object[] args, string expected)
private static void RenderAndTest(string input, CultureInfo culture, object[] args, string expected, LogFactory logFactory = null)
{
var logEventInfoAlways = new LogEventInfo(LogLevel.Info, "Logger", culture, input, args);
logEventInfoAlways.SetMessageFormatter(new NLog.Internal.LogMessageTemplateFormatter(LogManager.LogFactory, true, false).MessageFormatter, null);
logEventInfoAlways.SetMessageFormatter(new NLog.Internal.LogMessageTemplateFormatter(logFactory ?? new LogFactory(), true, false).MessageFormatter, null);
var templateAlways = logEventInfoAlways.MessageTemplateParameters;
Assert.Equal(expected, logEventInfoAlways.FormattedMessage);
}

private sealed class OverrideValueFormatter : IValueFormatter
{
private readonly IValueFormatter _orgValueFormatter;

public OverrideValueFormatter(IValueFormatter orgValueFormatter)
{
_orgValueFormatter = orgValueFormatter;
}

public bool FormatValue(object value, string format, NLog.MessageTemplates.CaptureType captureType, IFormatProvider formatProvider, System.Text.StringBuilder builder)
{
if (string.IsNullOrEmpty(format))
{
if (value is DateTime)
{
formatProvider = System.Globalization.CultureInfo.InvariantCulture;
format = "yyyy-MM-dd HH:mm:ss.FFFFFFFK";
}
}

return _orgValueFormatter.FormatValue(value, format, captureType, formatProvider, builder);
}
}
}
}
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.