feat: Add TextSegment for composable nested string interpolation#249
Merged
feat: Add TextSegment for composable nested string interpolation#249
Conversation
📝 WalkthroughWalkthroughThis PR introduces TextSegment, a new interpolated string handler type that captures and composes text fragments for embedding in SourceTextWriter. It adds Write overloads for multiple collection types, introduces internal validation utilities, updates documentation, and includes comprehensive test coverage with version bump to 2.3. Changes
Sequence DiagramsequenceDiagram
participant Code as Client Code
participant Handler as TextSegment Handler
participant Writer as SourceTextWriter
Code->>Handler: Create(ref handler, appendLine=false)
activate Handler
Handler->>Handler: Initialize _provider, _parts, _length
deactivate Handler
Code->>Handler: AppendLiteral("text")
activate Handler
Handler->>Handler: Add to _parts array
deactivate Handler
Code->>Handler: AppendFormatted<T>(value)
activate Handler
Handler->>Handler: Format value using _provider
Handler->>Handler: Add formatted string to _parts
deactivate Handler
Code->>Handler: Create returns TextSegment
activate Handler
Handler->>Handler: Finalize _parts array
deactivate Handler
Code->>Writer: WriteTo(writer)
activate Writer
Writer->>Writer: Iterate through _parts
Writer->>Writer: Write each part (recursive for nested segments)
alt appendLine is true
Writer->>Writer: Write newline
end
deactivate Writer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 80.71% 89.08% +8.37%
==========================================
Files 15 18 +3
Lines 980 1329 +349
Branches 69 101 +32
==========================================
+ Hits 791 1184 +393
+ Misses 162 101 -61
- Partials 27 44 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fgodoy-lepaho
previously approved these changes
Mar 15, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 116: The C# sample in the TextSegment uses unescaped nested double quotes
in the interpolated writer.WriteLine call (the call to FormatTypeRef with
"System" and "Serializable"), which breaks compilation; fix it by escaping the
inner quotes (e.g., replace inner "System" and "Serializable" with escaped
double quotes) or by converting the outer string to a verbatim/interpolated form
so the call to FormatTypeRef("System", "Serializable") is represented with
properly escaped quotes; update the writer.WriteLine(...) line accordingly.
In `@src/InterpolationCodeWriter/SourceTextWriter.Write.cs`:
- Around line 536-544: The Write(string?[] textParts) and
Write(IReadOnlyList<string?> textParts) overloads should treat a null sequence
as a no-op; update each method (Write(string?[] textParts) and
Write(IReadOnlyList<string?> textParts)) to check if the incoming container is
null and return immediately instead of iterating, preserving the existing
null-tolerant behavior of Write(string?) / Write(object?) for elements when the
container is non-null.
In `@src/InterpolationCodeWriter/TextSegment.cs`:
- Around line 116-120: The struct can be copied so multiple instances may call
Clear()/ToListAndClear()/WriteToAndClear() and return the same rented _parts
array multiple times; fix this by introducing an ownership/consumed sentinel
object instead of the simple _isRented bool so only one instance can return the
rented buffer. Replace the primitive _isRented with a small reference type owner
(e.g., PartsOwner { string?[] Parts; int consumed; }) stored in the struct (keep
_parts for fast access if desired), update WithAppendLine to copy the same owner
reference, and in Clear(), ToListAndClear(), and WriteToAndClear() use an atomic
operation (Interlocked.Exchange or Interlocked.CompareExchange on owner.consumed
or swap the owner reference to null) to ensure only the first caller returns the
array to ArrayPool<Item>.Shared and subsequent callers are no-ops; reference the
symbols _parts, _isRented (replace), WithAppendLine, ToListAndClear,
WriteToAndClear, and Clear when making the change.
- Around line 161-166: The Clear method in TextSegment currently returns the
rented Item[] (_parts) while Item contains reference fields, so clear the array
entries before returning to avoid retaining references; inside TextSegment.Clear
(which checks _isRented) set the elements of _parts to default(Item) (e.g.,
Array.Clear(_parts, 0, _parts.Length) or a loop assigning default) and then call
ArrayPool<Item>.Shared.Return(_parts), and ensure you still handle the
non-rented case and any state resets (_isRented/_parts) as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b0b1eb0-fc09-4afc-a34f-e750f66bdfcb
📒 Files selected for processing (16)
README.mdmodules/.csharpierignoresrc/InternalPolyfill/InternalPolyfill.csprojsrc/InterpolationCodeWriter/Internals/Throws.cssrc/InterpolationCodeWriter/README.mdsrc/InterpolationCodeWriter/SourceTextWriter.Indentation.cssrc/InterpolationCodeWriter/SourceTextWriter.Interpolation.cssrc/InterpolationCodeWriter/SourceTextWriter.Write.cssrc/InterpolationCodeWriter/SourceTextWriter.cssrc/InterpolationCodeWriter/TextSegment.Interpolation.cssrc/InterpolationCodeWriter/TextSegment.cstests/Directory.Build.propstests/Directory.Packages.propstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.Interpolation.cstests/InterpolationCodeWriter.Tests/TextSegmentTest.csversion.json
fgodoy-lepaho
previously approved these changes
Mar 16, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs (1)
447-457: Consider adding explicit casts fornullliteral interpolation holes to prevent ambiguous overload resolution.The new
AppendFormatted(string?[])andAppendFormatted(IReadOnlyList<string?>)overloads alongside the existingAppendFormatted(string?)can create ambiguous overload resolution for{null}interpolation holes, sincenullis implicitly convertible to all reference types and neitherstring?[]norIReadOnlyList<string?>has a conversion relationship tostring?. Use explicit casts in such cases (e.g.,{(string?)null}) to avoid C# error CS0121. No current instances were found in the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs` around lines 447 - 457, The three overloads AppendFormatted(string?[]), AppendFormatted(IReadOnlyList<string?>) and the existing AppendFormatted(string?) create ambiguous overload resolution for interpolation holes like `{null}`; update the API docs/XML comments on both new overloads to explicitly warn callers to disambiguate null interpolation holes by casting (e.g. `{(string?)null}`), add a brief inline code comment at each method (AppendFormatted(string?[])/AppendFormatted(IReadOnlyList<string?>)) pointing to the casting requirement, and add a small unit test or comment demonstrating the correct cast so callers know to use `(string?)null` to avoid CS0121.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/InterpolationCodeWriter/SourceTextWriter.Write.cs`:
- Around line 538-564: The ambiguity occurs because both collection overloads
Write(string?[]?) and Write(IReadOnlyList<string?>?) accept null literals;
change their parameters to non-nullable collections (Write(string?[] textParts)
and Write(IReadOnlyList<string?> textParts)) so a bare null literal will no
longer be considered applicable to those overloads and will resolve to the
existing Write(string?) overload; update the two method signatures (the Write
overloads currently declared at/around the signatures for Write(string?[]?
textParts) and Write(IReadOnlyList<string?>? textParts)) accordingly and keep
their null checks inside the method bodies if you still want to guard against
callers passing null via variables.
In `@src/InterpolationCodeWriter/TextSegment.cs`:
- Around line 11-24: The XML summary for TextSegment incorrectly claims the type
is "immutable" even though it is mutated via AppendLiteral and AppendFormatted
during construction; update the doc comments for the TextSegment type and its
Create(ref TextSegment, bool) and Create(IFormatProvider?, ref TextSegment,
bool) overloads to state that instances are constructed via an interpolated
string handler and are mutated during the AppendLiteral/AppendFormatted calls
but become effectively immutable after construction (or describe their
mutability semantics clearly), and remove or reword the single-word "immutable"
claim so consumers are not misled.
---
Nitpick comments:
In `@src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs`:
- Around line 447-457: The three overloads AppendFormatted(string?[]),
AppendFormatted(IReadOnlyList<string?>) and the existing
AppendFormatted(string?) create ambiguous overload resolution for interpolation
holes like `{null}`; update the API docs/XML comments on both new overloads to
explicitly warn callers to disambiguate null interpolation holes by casting
(e.g. `{(string?)null}`), add a brief inline code comment at each method
(AppendFormatted(string?[])/AppendFormatted(IReadOnlyList<string?>)) pointing to
the casting requirement, and add a small unit test or comment demonstrating the
correct cast so callers know to use `(string?)null` to avoid CS0121.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2e0ea79-2fc2-494c-9e1a-59d69f03a2fd
📒 Files selected for processing (5)
src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cssrc/InterpolationCodeWriter/SourceTextWriter.Write.cssrc/InterpolationCodeWriter/TextSegment.Interpolation.cssrc/InterpolationCodeWriter/TextSegment.cstests/InterpolationCodeWriter.Tests/TextSegmentTest.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/InterpolationCodeWriter.Tests/TextSegmentTest.cs
fgodoy-lepaho
previously approved these changes
Mar 16, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs (1)
422-422: Minor: Inconsistent region directive formatting.There's a space between
#andregionhere, unlike all other region directives in this file.🔧 Suggested fix
- # region String formatting + `#region` String formatting🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs` at line 422, The region directive " # region String formatting" is formatted inconsistently; change the directive text to "#region String formatting" (remove the space between '#' and 'region') so it matches the other region directives in this file; look for the exact line containing " # region String formatting" in SourceTextWriter.Interpolation.cs and update it to "#region String formatting".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cs`:
- Line 422: The region directive " # region String formatting" is formatted
inconsistently; change the directive text to "#region String formatting" (remove
the space between '#' and 'region') so it matches the other region directives in
this file; look for the exact line containing " # region String formatting" in
SourceTextWriter.Interpolation.cs and update it to "#region String formatting".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cdaadf8e-af3e-43f6-9de6-ed48b4c635b2
📒 Files selected for processing (2)
src/InterpolationCodeWriter/SourceTextWriter.Interpolation.cssrc/InterpolationCodeWriter/TextSegment.cs
- Fix # region typo (space) in SourceTextWriter.Interpolation.cs - Add missing AppendFormatted(in TextSegment?) overload to SourceTextWriter handler - Add EnsureCapacityForAdditionalItems/Grow to guard against _parts overflow - Fix WriteToAndClear reference in README to use WriteTo - Add implicit string→TextSegment conversion - Improve ToList() doc comment to clarify null semantics - Move SuppressMessage to parameter level in TextSegment constructor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/InterpolationCodeWriter/Internals/MathEx.cs`:
- Around line 1-2: This file is missing the file-level nullable opt-in; add the
directive `#nullable enable` at the very top of
src/InterpolationCodeWriter/Internals/MathEx.cs (above the using directives) so
the file opts into nullable reference types; update any related XML comments or
nullable annotations in the MathEx class/methods if the compiler surfaces
warnings after enabling nullable.
In `@src/InterpolationCodeWriter/TextSegment.Interpolation.cs`:
- Around line 274-277: The AppendFormatted(bool? value) method lacks a capacity
check and can overflow _parts when _length grows; add a call to
EnsureCapacityForAdditionalItems(1) at the start of AppendFormatted(bool? value)
before writing _parts[_length++] = value?.ToString(_provider) so the array is
grown as needed (match the pattern used by other primitive nullable overloads).
In `@tests/InterpolationCodeWriter.Tests/SourceTextWriterTest.Interpolation.cs`:
- Around line 17-112: The test expectation for the "{(short)1000:N0}" case in
WriteInterpolationCases is using a comma thousand separator but SourceTextWriter
uses CultureInfo.InvariantCulture (which yields "1.000"); update the expected
value in WriteInterpolationCases from "1,000" to "1.000" or alternatively change
the test to create SourceTextWriter with an explicit en-US culture provider so
the output remains "1,000"; reference the WriteInterpolationCases data set and
the SourceTextWriter constructor/creation site when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffc3d302-a98e-4ffa-b246-6d3eeb29c6f6
📒 Files selected for processing (10)
README.mdsrc/InternalPolyfill/InternalPolyfill.csprojsrc/InterpolationCodeWriter/Internals/MathEx.cssrc/InterpolationCodeWriter/Internals/Throws.cssrc/InterpolationCodeWriter/InterpolationCodeWriter.csprojsrc/InterpolationCodeWriter/SourceTextWriter.Interpolation.cssrc/InterpolationCodeWriter/TextSegment.Interpolation.cssrc/InterpolationCodeWriter/TextSegment.cstests/InterpolationCodeWriter.Tests/SourceTextWriterTest.Interpolation.cstests/InterpolationCodeWriter.Tests/TextSegmentTest.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/InterpolationCodeWriter.Tests/TextSegmentTest.cs
fgodoy-lepaho
previously approved these changes
Mar 17, 2026
fgodoy-lepaho
approved these changes
Mar 17, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
TextSegment, a composable interpolated string handler that captures interpolated string segments for deferred or nested rendering withinSourceTextWriterSourceTextWriter.Writewith overloads acceptingReadOnlySpan<string?>,string?[], andIReadOnlyList<string?>for writing multiple text partsAppendFormattedoverloads forReadOnlySpan<char>,ReadOnlySpan<string?>,string?[],IReadOnlyList<string?>, andTextSegmentto the interpolation handler, enablingTextSegmentvalues to be embedded directly in interpolated stringsTest plan
dotnet testto verify all unit tests passTextSegmentTestcovers creation, appending, and rendering scenariosSourceTextWriterTestandInterpolationCodeWriter.CSharp.TestsSummary by CodeRabbit
New Features
TextSegment- a new interpolated string handler that captures text fragments for reuse in writer operations.TextSegment.Create()with optionalappendLineflag andIFormatProviderparameter.Write()method overloads accepting various string collection types.Documentation
TextSegmentdocumentation with usage examples and scenarios.