Conversation
Fixes dotnet#51310 Address all the comments in this issue.
There was a problem hiding this comment.
Pull request overview
This pull request addresses all feedback points from issue #51310 regarding the C# interpolated string handler tutorial. The changes improve the tutorial by adding missing implementation details, clarifying behaviors, and simplifying examples.
Changes:
- Added implementation for
AppendFormattedwith alignment and format parameters - Replaced custom
GetFormattedText()method with standardToString()override in all logger versions - Added documentation notes about constant interpolated strings and empty
InterpolatedStringHandlerArgumentAttributebehavior - Simplified the side-effects demonstration from 5 increments to 1 increment per loop iteration
- Updated target framework from net8.0 to net10.0
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| logger-v4.cs | Added AppendFormatted overload with alignment parameter and replaced GetFormattedText with ToString override |
| logger-v3.cs | Replaced GetFormattedText with ToString override |
| Logger-v2.cs | Replaced GetFormattedText with ToString override |
| Version_4_Examples.cs | Simplified side-effects example to use single increment instead of five |
| interpolated-string-handler.md | Added two notes explaining constant interpolated string behavior and empty attribute behavior; updated expected output |
| interpolated-string-handler.csproj | Updated TargetFramework from net8.0 to net10.0 |
| :::code language="csharp" source="./snippets/interpolated-string-handler/Logger-v2.cs" id="CoreInterpolatedStringHandler"::: | ||
|
|
||
| > [!NOTE] | ||
| > When the interpolated string expression is a compile-time constant (that is, it contains no placeholders), the compiler uses the target type `string` instead of invoking a custom interpolated string handler. This means constant interpolated strings bypass custom handlers entirely. |
There was a problem hiding this comment.
According to the markdown writing style guidelines, use contractions for better readability. Change "it contains no placeholders" to "it doesn't contain placeholders" or rephrase to use a contraction.
| :::code language="csharp" source="./snippets/interpolated-string-handler/Logger-v2.cs" id="CoreInterpolatedStringHandler"::: | ||
|
|
||
| > [!NOTE] | ||
| > When the interpolated string expression is a compile-time constant (that is, it contains no placeholders), the compiler uses the target type `string` instead of invoking a custom interpolated string handler. This means constant interpolated strings bypass custom handlers entirely. |
There was a problem hiding this comment.
The note should use contractions according to the markdown writing style guidelines. Change "it contains no placeholders" to "it doesn't contain placeholders" or "it has no placeholders".
| :::code language="csharp" source="./snippets/interpolated-string-handler/Version_4_Examples.cs" id="TestSideEffects"::: | ||
|
|
||
| You can see the `index` variable is incremented five times each iteration of the loop. Because the placeholders are evaluated only for `Critical`, `Error` and `Warning` levels, not for `Information` and `Trace`, the final value of `index` doesn't match the expectation: | ||
| You can see the `index` variable is incremented each iteration of the loop. Because the placeholders are evaluated only for `Critical`, `Error` and `Warning` levels, not for `Information` and `Trace`, the final value of `index` doesn't match the expectation: |
There was a problem hiding this comment.
This sentence is missing an Oxford comma. Change "evaluated only for Critical, Error and Warning levels" to "evaluated only for Critical, Error, and Warning levels".
1 participant
Fixes #51310
Address all the comments in this issue.
Internal previews