Skip to content

Commit 85799bb

Browse files
SimonCroppclaude
andauthored
Allow conversion splitters for text extensions (#1778)
* Allow conversion splitters for text extensions Allow conversion splitters for text extensions Previously RegisterStreamConverter threw for text extensions, and the splitter pipeline only handled stream targets, so a converter could not run against a text extension (eg html). - Drop the text-extension guard in RegisterStreamConverter - DoExtensionConversion now takes the Target itself and resolves the converter input from either a stream or a text (utf8) target, instead of reconstructing an illegal stream Target for text extensions - Scrub text targets before they are fed to a converter, so derived targets (eg rendered images) reflect the scrubbed content - Move converted-target scrubbing into DoExtensionConversion and pass already-converted targets through GetTargets untouched, so each text target is scrubbed exactly once (no double scrub of the re-emitted source) Add ExtensionConverterTests.TextSplitter covering a converter registered against a text extension. * . --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 15e2610 commit 85799bb

10 files changed

Lines changed: 159 additions & 44 deletions

docs/converter.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,45 @@ await VerifyFile("sample.tif");
146146
<!-- endSnippet -->
147147

148148

149+
### Text extensions
150+
151+
A stream converter can also be registered against a text extension. This is useful when a text document needs derived targets, for example rendering html to an image for visual verification.
152+
153+
The text target is scrubbed before being passed to the converter, so any derived targets (for example a rendered image) reflect the scrubbed content.
154+
155+
For a custom text extension, register it as text via `FileExtensions.AddTextExtension`. Built-in text extensions (for example `html` or `csv`) do not require this.
156+
157+
<!-- snippet: RegisterStreamConverterTextExtension -->
158+
<a id='snippet-RegisterStreamConverterTextExtension'></a>
159+
```cs
160+
// "texttoconvert" is a custom text extension, so register it as text first.
161+
// For built-in text extensions (eg html or csv) this step is not required.
162+
FileExtensions.AddTextExtension("texttoconvert");
163+
164+
// The input text is scrubbed before being passed to the converter, so any
165+
// derived targets (eg a rendered image) reflect the scrubbed content.
166+
VerifierSettings.RegisterStreamConverter(
167+
"texttoconvert",
168+
async (_, stream, _) =>
169+
new(
170+
null,
171+
[
172+
new("texttoconvert", await stream.ReadStringBuilderWithFixedLines()),
173+
new("txt", "derived from text")
174+
]));
175+
```
176+
<sup><a href='/src/Verify.Tests/Converters/ExtensionConverterTests.cs#L6-L24' title='Snippet source file'>snippet source</a> | <a href='#snippet-RegisterStreamConverterTextExtension' title='Start of snippet'>anchor</a></sup>
177+
<!-- endSnippet -->
178+
179+
<!-- snippet: TextExtensionConverterVerify -->
180+
<a id='snippet-TextExtensionConverterVerify'></a>
181+
```cs
182+
Verify("the source text", "texttoconvert");
183+
```
184+
<sup><a href='/src/Verify.Tests/Converters/ExtensionConverterTests.cs#L30-L32' title='Snippet source file'>snippet source</a> | <a href='#snippet-TextExtensionConverterVerify' title='Start of snippet'>anchor</a></sup>
185+
<!-- endSnippet -->
186+
187+
149188
### Cleanup
150189

151190
If cleanup needs to occur after verification a callback can be passes to `ConversionResult`:

docs/mdsource/converter.source.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,19 @@ snippet: RegisterStreamConverterExtension
5050
snippet: FileConverterExtensionVerify
5151

5252

53+
### Text extensions
54+
55+
A stream converter can also be registered against a text extension. This is useful when a text document needs derived targets, for example rendering html to an image for visual verification.
56+
57+
The text target is scrubbed before being passed to the converter, so any derived targets (for example a rendered image) reflect the scrubbed content.
58+
59+
For a custom text extension, register it as text via `FileExtensions.AddTextExtension`. Built-in text extensions (for example `html` or `csv`) do not require this.
60+
61+
snippet: RegisterStreamConverterTextExtension
62+
63+
snippet: TextExtensionConverterVerify
64+
65+
5366
### Cleanup
5467

5568
If cleanup needs to occur after verification a callback can be passes to `ConversionResult`:
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
the source text
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
derived from text

src/Verify.Tests/Converters/ExtensionConverterTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,36 @@
11
public class ExtensionConverterTests
22
{
3+
[ModuleInitializer]
4+
public static void TextSplitterInit()
5+
{
6+
#region RegisterStreamConverterTextExtension
7+
8+
// "texttoconvert" is a custom text extension, so register it as text first.
9+
// For built-in text extensions (eg html or csv) this step is not required.
10+
FileExtensions.AddTextExtension("texttoconvert");
11+
12+
// The input text is scrubbed before being passed to the converter, so any
13+
// derived targets (eg a rendered image) reflect the scrubbed content.
14+
VerifierSettings.RegisterStreamConverter(
15+
"texttoconvert",
16+
async (_, stream, _) =>
17+
new(
18+
null,
19+
[
20+
new("texttoconvert", await stream.ReadStringBuilderWithFixedLines()),
21+
new("txt", "derived from text")
22+
]));
23+
24+
#endregion
25+
}
26+
27+
// a conversion splitter registered against a text extension
28+
[Fact]
29+
public Task TextSplitter() =>
30+
#region TextExtensionConverterVerify
31+
Verify("the source text", "texttoconvert");
32+
#endregion
33+
334
[ModuleInitializer]
435
public static void RecursiveInit() =>
536
VerifierSettings.RegisterStreamConverter(

src/Verify.Tests/InnerVerifyChecksTests/VerifyChecksTests.GetExtensions.verified.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
nuspec,
77
props,
88
staticComparerExt,
9+
texttoconvert,
910
txt,
1011
xml
1112
]

src/Verify/Compare/Png/PngSsimComparer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ internal static Task<CompareResult> Compare(Stream received, Stream verified, do
1313
var receivedImage = PngDecoder.Decode(received);
1414
var verifiedImage = PngDecoder.Decode(verified);
1515

16-
if (receivedImage.Width != verifiedImage.Width || receivedImage.Height != verifiedImage.Height)
16+
if (receivedImage.Width != verifiedImage.Width ||
17+
receivedImage.Height != verifiedImage.Height)
1718
{
1819
return Task.FromResult(CompareResult.NotEqual(
1920
$"PNG dimensions differ. Received: {receivedImage.Width}x{receivedImage.Height}, Verified: {verifiedImage.Width}x{verifiedImage.Height}"));

src/Verify/Splitters/Settings_Extension.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@ public static void RegisterStreamConverter(
2323
{
2424
InnerVerifier.ThrowIfVerifyHasBeenRun();
2525
Guards.AgainstBadExtension(extension);
26-
if (FileExtensions.IsTextExtension(extension))
27-
{
28-
throw new("RegisterStreamConverter is only supported for non-text extensions");
29-
}
30-
3126
extensionConverters[extension] = conversion;
3227
}
3328
}

src/Verify/Verifier/InnerVerifier_Inner.cs

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -52,46 +52,54 @@ async Task<VerifyResult> VerifyInner(object? root, Func<Task>? cleanup, IEnumera
5252
{
5353
List<Target> list = [..targets, ..VerifierSettings.GetFileAppenders(settings)];
5454
var cleanup = () => Task.CompletedTask;
55-
if (doExtensionConversion)
56-
{
57-
var result = new List<Target>();
58-
foreach (var target in list)
59-
{
60-
if (!target.PerformConversion ||
61-
!VerifierSettings.HasStreamConverter(target.Extension))
62-
{
63-
result.Add(target);
64-
continue;
65-
}
66-
67-
var (info, converted, itemCleanup) = await DoExtensionConversion(target.Extension, target.StreamData, null, target.Name);
68-
cleanup = cleanup.Then(itemCleanup);
69-
if (info != null)
70-
{
71-
result.Add(
72-
new(
73-
settings.TxtOrJson,
74-
JsonFormatter.AsJson(
75-
settings,
76-
counter,
77-
info)));
78-
}
79-
80-
result.AddRange(converted);
81-
}
8255

83-
list = result;
56+
// When doExtensionConversion is false the targets have already been run through
57+
// conversion and scrubbing (the only caller is the post-conversion stream path),
58+
// so pass them through untouched to avoid double scrubbing.
59+
if (!doExtensionConversion)
60+
{
61+
return (list, cleanup);
8462
}
8563

64+
var result = new List<Target>();
8665
foreach (var target in list)
8766
{
88-
if (target.TryGetStringBuilder(out var builder))
67+
if (!target.PerformConversion ||
68+
!VerifierSettings.HasStreamConverter(target.Extension))
8969
{
90-
ApplyScrubbers.ApplyForExtension(target.Extension, builder, settings, counter);
70+
Scrub(target);
71+
result.Add(target);
72+
continue;
9173
}
74+
75+
var (info, converted, itemCleanup) = await DoExtensionConversion(target, null);
76+
cleanup = cleanup.Then(itemCleanup);
77+
if (info != null)
78+
{
79+
Target infoTarget = new(
80+
settings.TxtOrJson,
81+
JsonFormatter.AsJson(
82+
settings,
83+
counter,
84+
info));
85+
Scrub(infoTarget);
86+
result.Add(infoTarget);
87+
}
88+
89+
// converted targets are scrubbed within DoExtensionConversion
90+
result.AddRange(converted);
9291
}
9392

94-
return (list, cleanup);
93+
return (result, cleanup);
94+
}
95+
96+
// Scrubs a text target in place. Stream (binary) targets are left untouched.
97+
void Scrub(in Target target)
98+
{
99+
if (target.TryGetStringBuilder(out var builder))
100+
{
101+
ApplyScrubbers.ApplyForExtension(target.Extension, builder, settings, counter);
102+
}
95103
}
96104

97105
bool TryGetRootTarget(object? root,bool ignoreNullRoot, [NotNullWhen(true)] out Target? target)

src/Verify/Verifier/InnerVerifier_Stream.cs

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ public async Task<VerifyResult> VerifyStream(Stream? stream, string extension, o
9090
{
9191
if (VerifierSettings.HasStreamConverter(extension))
9292
{
93-
var (newInfo, converted, cleanup) = await DoExtensionConversion(extension, stream, info, null);
93+
var initial = await GetTarget(stream, extension);
94+
var (newInfo, converted, cleanup) = await DoExtensionConversion(initial, info);
9495

9596
return await VerifyInner(newInfo, cleanup, converted, false, true);
9697
}
@@ -122,10 +123,15 @@ static async Task<Target> GetTarget(Stream stream, string extension)
122123
return new(extension, stream);
123124
}
124125

125-
//TODO: possibly pass in the target here
126-
async Task<(object? info, List<Target> targets, Func<Task> cleanup)> DoExtensionConversion(string extension, Stream stream, object? info, string? name)
126+
async Task<(object? info, List<Target> targets, Func<Task> cleanup)> DoExtensionConversion(Target initial, object? info)
127127
{
128-
var cleanup = stream.DisposeAsyncEx;
128+
var cleanup = () => Task.CompletedTask;
129+
// the source stream of a stream target is owned here, so dispose it once consumed
130+
if (initial.IsStream)
131+
{
132+
cleanup = cleanup.Then(initial.StreamData.DisposeAsyncEx);
133+
}
134+
129135
var infos = new List<object>();
130136
if (info != null)
131137
{
@@ -135,19 +141,37 @@ static async Task<Target> GetTarget(Stream stream, string extension)
135141
var targets = new List<Target>();
136142

137143
var queue = new Queue<Target>();
138-
queue.Enqueue(new(extension, stream, name));
144+
queue.Enqueue(initial);
139145

140146
while (queue.Count > 0)
141147
{
142148
var target = queue.Dequeue();
143149

144150
if (!VerifierSettings.TryGetStreamConverter(target.Extension, out var conversion))
145151
{
152+
// terminal target: scrub text before it is finalized
153+
Scrub(target);
146154
targets.Add(target);
147155
continue;
148156
}
149157

150-
var targetStream = target.StreamData;
158+
// scrub text before conversion so derived targets (eg rendered images) reflect the scrubbed content
159+
Scrub(target);
160+
161+
Stream targetStream;
162+
if (target.IsStream)
163+
{
164+
targetStream = target.StreamData;
165+
}
166+
else
167+
{
168+
// a text target is fed to the converter as a utf8 stream
169+
target.TryGetStringBuilder(out var builder);
170+
var memory = new MemoryStream(Encoding.UTF8.GetBytes(builder!.ToString()));
171+
cleanup = cleanup.Then(memory.DisposeAsyncEx);
172+
targetStream = memory;
173+
}
174+
151175
var result = await conversion(target.Name, targetStream, settings.Context);
152176
if (result.Cleanup != null)
153177
{
@@ -163,7 +187,8 @@ static async Task<Target> GetTarget(Stream stream, string extension)
163187

164188
foreach (var resultTarget in resultTargets)
165189
{
166-
// if the same extension is returned. no need to re process
190+
// if the same extension is returned. no need to re process.
191+
// its content derives from the already scrubbed input, so it is not scrubbed again
167192
if (resultTarget.Extension == target.Extension)
168193
{
169194
targets.Add(resultTarget);

0 commit comments

Comments
 (0)