From 9c6aaa794ab69c9d1edea8ae323a2126adff9ea5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 17:52:11 +0000 Subject: [PATCH 1/5] Say whether a stored text can be saved back without loss; cut 2.19.0 MarkupTextSerializer.Inspect reads like Deserialize and reports Complete, Opaque, Unsupported or Damaged, with a diagnostic per problem and CanRewrite. It writes each codec-read layer back and compares structure, so an unknown property on a known kind, an unknown layout block or a dropped gradient stop is caught for any codec. The envelope gets a wire version: no "v" (or "v":1) is version 1, and Deserialize now throws for any other instead of reading it as version 1. Written bytes are unchanged; golden payloads from released 1.0.0, 2.0.0, 2.8.0 and 2.18.0 writers are pinned in WireCompatibilityTests. Refs #61 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BxoP4iqJzh4jXnBsnkAFt --- CHANGELOG.md | 19 ++ MarkupString.Tests/WireCompatibilityTests.cs | 288 ++++++++++++++++++ MarkupString/MarkupReadResult.cs | 68 +++++ MarkupString/MarkupTextSerializer.cs | 289 +++++++++++++++++-- MarkupString/PublicAPI.Shipped.txt | 15 + MarkupString/WireShape.cs | 63 ++++ docs/serialization.md | 66 ++++- 7 files changed, 786 insertions(+), 22 deletions(-) create mode 100644 MarkupString.Tests/WireCompatibilityTests.cs create mode 100644 MarkupString/MarkupReadResult.cs create mode 100644 MarkupString/WireShape.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0328943..9505d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,25 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +## 2.19.0 — 2026-10-13 + +### Added + +- **`MarkupTextSerializer.Inspect` says whether a stored text can be saved back without loss.** It + reads like `Deserialize` and returns a `MarkupReadResult`: the value, a `Status` (`Complete`, + `Opaque`, `Unsupported`, `Damaged`), one diagnostic per problem, and `CanRewrite`. It never throws + on bad input. It reports an unknown property on a known kind, an unknown layout block, a gradient + stop that would be dropped, an unknown envelope property, an unused palette entry, a cover that + overruns its text, a codec that throws, and payloads that are not JSON objects. See + [Serialization](docs/serialization.md#reading-stored-text-for-saving-inspect). +- **A wire version.** The stored shape is wire version 1 (`MarkupTextSerializer.WireVersion`), which + writes no `"v"`, as every release since 1.0.0 has. The bytes written are unchanged. + +### Changed + +- **`Deserialize` throws `JsonException` for a `"v"` other than 1,** instead of reading the payload + as version 1. No writer has produced one, so no stored row changes how it reads. + ## 2.18.1 — 2026-10-13 ### Changed diff --git a/MarkupString.Tests/WireCompatibilityTests.cs b/MarkupString.Tests/WireCompatibilityTests.cs new file mode 100644 index 0000000..e2f98fd --- /dev/null +++ b/MarkupString.Tests/WireCompatibilityTests.cs @@ -0,0 +1,288 @@ +using System.Text; +using System.Text.Json; +using MarkupString.Ansi; +using MarkupString.Html; +using MarkupString.Layout; + +/// +/// The stored form is a contract: rows written by every released writer are read by every later +/// reader, and a reader that cannot keep everything in a row says so instead of handing back a value +/// that would lose it on the next save. Assertions here compare JSON, not +/// equality, which is plain text only and cannot show that a write keeps what was read. +/// +public class WireCompatibilityTests +{ + private static readonly MarkupRegistry Registry = MarkupRegistry.Empty.WithAnsi().WithHtml(); + + // ── Golden payloads ────────────────────────────────────────────────────────── + + /// + /// Written by the released MarkupString, MarkupString.Ansi and MarkupString.Html packages 1.0.0, + /// 2.0.0, 2.8.0 and 2.18.0 with WithAnsi().WithHtml(); every one of them wrote these bytes. + /// The layout payload is 2.8.0's (the first release with block layout); 2.18.0 writes the same + /// bytes but for the random layout id. + /// + public static IEnumerable> Released() => + [ + () => """{"t":"plain text"}""", + () => "{}", + () => """{"t":"Hello world!","p":[null,[{"k":"ansi","f":9}]],"r":[6,0,5,1,1,0]}""", + () => """{"t":"Go north","p":[null,[{"k":"ansi","f":14},{"k":"html","h":"send","a":"href=\"north\""}]],"r":[8,1]}""", + () => """{"t":"日本語 ünï ✓","p":[null,[{"k":"ansi","f":4}]],"r":[3,1,6,0]}""", + () => """{"t":"n","p":[null,[{"k":"neutral"}]],"r":[1,1]}""", + () => Layout, + ]; + + private const string Layout = """{"t":"+==========< Byron >=========+\n+==========< Quote >=========+\n| Hooooo? |\n+============================+","p":[null,[{"k":"layout","w":30,"l":123,"h":"4d6cb6025c3413fb","id":"0e716bc9b9d807a4","n":{"t":"frame","b":{"t":"stack","ch":[{"t":"rule","ti":{"t":"Quote","p":[null,[{"k":"ansi","f":10}]],"r":[5,1]}},{"t":"text","c":{"t":"Hooooo?"}}]},"bs":{"n":"mush"},"ti":{"t":"Byron"}}}],[{"k":"ansi","f":10},{"k":"layout","w":30,"l":123,"h":"4d6cb6025c3413fb","id":"0e716bc9b9d807a4","n":{"t":"frame","b":{"t":"stack","ch":[{"t":"rule","ti":{"t":"Quote","p":[null,[{"k":"ansi","f":10}]],"r":[5,1]}},{"t":"text","c":{"t":"Hooooo?"}}]},"bs":{"n":"mush"},"ti":{"t":"Byron"}}}]],"r":[44,1,5,2,74,1]}"""; + + [Test] + [MethodDataSource(nameof(Released))] + public async Task Released_IsComplete_AndWritesBackByteForByte(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Complete); + await Assert.That(result.Diagnostics).IsEmpty(); + await Assert.That(MarkupTextSerializer.Serialize(result.Value!, Registry)).IsEqualTo(json); + } + + [Test] + [MethodDataSource(nameof(Released))] + public async Task Released_InspectAndDeserialize_ReadTheSameText(string json) + => await Assert.That(MarkupTextSerializer.Inspect(json, Registry).Value!.Text) + .IsEqualTo(MarkupTextSerializer.Deserialize(json, Registry).Text); + + /// Forms written before the "k" discriminator and the semantic colour model. + [Test] + [Arguments("""{"t":"x","p":[null,[{"n":1}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"f":[31]}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"ansi","f":[1,34],"g":[38,5,200],"bo":0}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"ansi","f":"#ff000080"}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"h":"send","a":"href=\"x\""}]],"r":[1,1]}""")] + public async Task Legacy_IsComplete(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Complete); + await Assert.That(result.CanRewrite).IsTrue(); + } + + // ── Opaque ─────────────────────────────────────────────────────────────────── + + [Test] + public async Task UnknownKind_IsOpaque_AndWritesBackByteForByte() + { + const string json = """{"t":"x","p":[null,[{"k":"zzz","f":1,"future":{"x":7}}]],"r":[1,1]}"""; + + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Opaque); + await Assert.That(result.CanRewrite).IsTrue(); + await Assert.That(MarkupTextSerializer.Serialize(result.Value!, Registry)).IsEqualTo(json); + } + + // ── Unsupported ────────────────────────────────────────────────────────────── + + [Test] + [Arguments("""{"v":99,"t":"x","future":true}""")] + [Arguments("""{"v":2,"text":"important"}""")] + [Arguments("""{"v":"1","t":"x"}""")] + public async Task UnknownVersion_IsUnsupported_AndNotRead(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(result.CanRewrite).IsFalse(); + await Assert.That(result.Value).IsNull(); + await Assert.That(result.Diagnostics.Single()).Contains("Wire version"); + } + + [Test] + public async Task UnknownVersion_Deserialize_Throws() + => await Assert.That(() => MarkupTextSerializer.Deserialize("""{"v":2,"text":"important"}""", Registry)) + .Throws(); + + [Test] + public async Task UnknownVersion_Utf8Deserialize_Throws() + => await Assert.That(() => MarkupTextSerializer.Deserialize(Encoding.UTF8.GetBytes("""{"v":2,"t":"x"}"""), Registry)) + .Throws(); + + [Test] + public async Task VersionOne_IsRead() + { + var result = MarkupTextSerializer.Inspect("""{"v":1,"t":"x"}""", Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Complete); + await Assert.That(result.Value!.Text).IsEqualTo("x"); + await Assert.That(MarkupTextSerializer.Deserialize("""{"v":1,"t":"x"}""", Registry).Text).IsEqualTo("x"); + } + + [Test] + public async Task UnknownEnvelopeProperty_IsUnsupported_ButShowsTheText() + { + var result = MarkupTextSerializer.Inspect("""{"t":"x","future":true}""", Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(result.Value!.Text).IsEqualTo("x"); + await Assert.That(result.Diagnostics.Single()).Contains("\"future\""); + } + + [Test] + public async Task KnownKind_WithAPropertyItDoesNotKnow_IsUnsupported() + { + var result = MarkupTextSerializer.Inspect("""{"t":"x","p":[null,[{"k":"ansi","f":1,"future":{"x":7}}]],"r":[1,1]}""", Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(result.Diagnostics.Single()).Contains("/future is dropped"); + } + + [Test] + public async Task Neutral_WithAPropertyItDoesNotKnow_IsUnsupported() + => await Assert.That(MarkupTextSerializer.Inspect("""{"t":"x","p":[null,[{"k":"neutral","future":1}]],"r":[1,1]}""", Registry).Status) + .IsEqualTo(MarkupReadStatus.Unsupported); + + [Test] + public async Task UnknownLayoutBlock_IsUnsupported() + { + var json = Layout.Replace("\"t\":\"rule\"", "\"t\":\"future\""); + + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(result.Diagnostics[0]).Contains("/n/b/ch/0/t changes from \"future\" to \"stack\""); + // Deserialize still shows the text the layout stands for. + await Assert.That(MarkupTextSerializer.Deserialize(json, Registry).Text) + .IsEqualTo(MarkupTextSerializer.Deserialize(Layout, Registry).Text); + } + + [Test] + public async Task UnknownLayoutBlock_WithNoRegistryForIt_IsUnsupported_EvenWhenLayoutIsKnown() + { + var result = MarkupTextSerializer.Inspect(Layout.Replace("\"t\":\"text\"", "\"t\":\"widget\""), Registry); + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + } + + [Test] + public async Task GradientStop_OfAnUnknownKind_IsUnsupported() + { + var red = AnsiMarkup.Create(foreground: new AnsiColor.Rgb(255, 0, 0)); + var lime = AnsiMarkup.Create(foreground: new AnsiColor.Rgb(0, 255, 0)); + var gauge = new Gauge(6, 12) { BarWidth = 4, Show = GaugeShow.None, Gradient = new ColorGradient([red, lime], GradientSpace.Oklch) }; + var json = MarkupTextSerializer.Serialize(BlockLayout.Build(gauge, 20), Registry); + await Assert.That(MarkupTextSerializer.Inspect(json, Registry).Status).IsEqualTo(MarkupReadStatus.Complete); + + var result = MarkupTextSerializer.Inspect(json.Replace("\"k\":\"ansi\",\"f\":\"#ff0000\"", "\"k\":\"zzz\",\"f\":\"#ff0000\""), Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(string.Join("\n", result.Diagnostics)).Contains("/st has 2 entries and would be written with 1"); + } + + [Test] + public async Task UnusedPaletteEntry_IsUnsupported() + { + var result = MarkupTextSerializer.Inspect("""{"t":"x","p":[null,[{"k":"neutral"}],[{"k":"zzz"}]],"r":[1,1]}""", Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); + await Assert.That(result.Diagnostics.Single()).Contains("Palette entry 2"); + } + + // ── Damaged ────────────────────────────────────────────────────────────────── + + [Test] + [Arguments("not json")] + [Arguments("[1,2]")] + [Arguments("\"t\"")] + [Arguments("{\"t\":")] + public async Task NotAnObject_IsDamaged_WithNoValue(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); + await Assert.That(result.Value).IsNull(); + await Assert.That(result.Diagnostics).IsNotEmpty(); + } + + [Test] + [Arguments("""{"t":"ab"} xyz""")] + [Arguments("""{"t":"ab"}{"t":"cd"}""")] + public async Task TrailingContent_IsDamaged_ButShowsTheFirstValue(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); + await Assert.That(result.Value!.Text).IsEqualTo("ab"); + } + + [Test] + [Arguments("""{"t":5}""")] + [Arguments("""{"t":"ab","p":5,"r":7}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":"neutral"}]]}""")] + [Arguments("""{"t":"ab","p":[1,[{"k":"neutral"}]],"r":[2,1]}""")] + [Arguments("""{"t":"ab","p":[null,7],"r":[2,1]}""")] + [Arguments("""{"t":"ab","p":[null,[]],"r":[2,1]}""")] + [Arguments("""{"t":"ab","p":[null,[7]],"r":[2,1]}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":"neutral"}]],"r":[9,1]}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":"neutral"}]],"r":[2,1,1,1]}""")] + [Arguments("""{"t":"abcd","p":[null,[{"k":"neutral"}]],"r":[2,1,2]}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":"neutral"}]],"r":[2,7]}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":"neutral"}]],"r":["x",1]}""")] + [Arguments("""{"t":"abcdef","p":[null,[{"k":"neutral"}]],"r":[3,1,-3,1,3,1]}""")] + [Arguments("""{"t":"ab","p":[null,[{"k":5,"f":1}]],"r":[2,1]}""")] + public async Task Malformed_IsDamaged(string json) + { + var result = MarkupTextSerializer.Inspect(json, Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); + await Assert.That(result.CanRewrite).IsFalse(); + await Assert.That(result.Value).IsNotNull(); + } + + private sealed record Boom : IMarkup; + + private sealed class ThrowingCodec : IMarkupCodec + { + public string Kind => "boom"; + public Type MarkupType => typeof(Boom); + public void Write(Utf8JsonWriter writer, IMarkup markup) { } + public IMarkup Read(JsonElement element) => throw new InvalidOperationException("no"); + } + + [Test] + public async Task ThrowingCodec_IsDamaged_AndKeepsTheLayerOpaque() + { + const string json = """{"t":"x","p":[null,[{"k":"boom","q":1}]],"r":[1,1]}"""; + var registry = Registry.With(new ThrowingCodec()); + + var result = MarkupTextSerializer.Inspect(json, registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); + await Assert.That(result.Value!.Runs[0].Markups[0]).IsEqualTo((IMarkup)new UnknownMarkup("boom", """{"k":"boom","q":1}""")); + await Assert.That(() => MarkupTextSerializer.Deserialize(json, registry)).Throws(); + } + + [Test] + public async Task WorstFinding_Wins() + { + var result = MarkupTextSerializer.Inspect("""{"t":"ab","future":1,"p":[null,[{"k":"zzz"}]],"r":[9,1]}""", Registry); + + await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); + await Assert.That(result.Diagnostics.Length).IsEqualTo(2); + } + + [Test] + public async Task Empty_IsComplete() + { + await Assert.That(MarkupTextSerializer.Inspect("", Registry).Value).IsSameReferenceAs(MarkupText.Empty); + await Assert.That(MarkupTextSerializer.Inspect(ReadOnlySpan.Empty, Registry).Status).IsEqualTo(MarkupReadStatus.Complete); + } + + [Test] + public async Task Utf8AndString_Agree() + { + const string json = """{"t":"日本","future":1}"""; + var fromString = MarkupTextSerializer.Inspect(json, Registry); + var fromBytes = MarkupTextSerializer.Inspect(Encoding.UTF8.GetBytes(json), Registry); + + await Assert.That(fromBytes.Status).IsEqualTo(fromString.Status); + await Assert.That(fromBytes.Value!.Text).IsEqualTo(fromString.Value!.Text); + } +} diff --git a/MarkupString/MarkupReadResult.cs b/MarkupString/MarkupReadResult.cs new file mode 100644 index 0000000..8405b15 --- /dev/null +++ b/MarkupString/MarkupReadResult.cs @@ -0,0 +1,68 @@ +using System.Collections.Immutable; +namespace MarkupString; + +/// What reading a stored found, from best to worst. +public enum MarkupReadStatus +{ + /// Every part of the payload was understood, and writing the value back keeps all of it. + Complete, + + /// + /// Some layers are of a kind no codec in the reading registry claims. They are held as + /// and written back verbatim, so writing the value back keeps all of it. + /// + Opaque, + + /// + /// The payload is well formed but carries something this reader does not understand and would + /// not write back: a wire version it does not know, a property it does not know on the envelope + /// or on a layer whose kind it does know, a layout block of an unknown kind. The value, when + /// there is one, is fit to show but not to save in place of the original. + /// + Unsupported, + + /// + /// The payload is not what any writer produces: not JSON, not an object, content after the + /// value, a field of the wrong type, a cover that overruns the text, a codec that failed. The + /// value, when there is one, is what survived, fit to show but not to save in place of the original. + /// + Damaged, +} + +/// +/// The result of : the value read, how much of +/// the payload it holds, and why. +/// +/// +/// is for showing text: it recovers what it +/// can and never says what it left out. A caller that may write the value back to where it came +/// from reads with Inspect and saves only when is true; otherwise +/// it keeps the stored bytes as they are. +/// +public sealed class MarkupReadResult +{ + internal MarkupReadResult(MarkupReadStatus status, MarkupText? value, ImmutableArray diagnostics) + { + Status = status; + Value = value; + Diagnostics = diagnostics; + } + + /// How much of the payload holds. + public MarkupReadStatus Status { get; } + + /// + /// The text read. Null when nothing could be read without guessing: the payload is not a JSON + /// object, or it names a wire version this reader does not know. + /// + public MarkupText? Value { get; } + + /// One line for each thing the reader did not understand or could not keep, in payload order. + public ImmutableArray Diagnostics { get; } + + /// + /// Whether serialising keeps everything the payload held. True for + /// and . + /// + public bool CanRewrite => Status is MarkupReadStatus.Complete or MarkupReadStatus.Opaque; +} diff --git a/MarkupString/MarkupTextSerializer.cs b/MarkupString/MarkupTextSerializer.cs index d9f5b0f..867d0da 100644 --- a/MarkupString/MarkupTextSerializer.cs +++ b/MarkupString/MarkupTextSerializer.cs @@ -230,7 +230,7 @@ public static MarkupText Deserialize(string json, MarkupRegistry? registry) if (json.Length == 0) return MarkupText.Empty; using var document = JsonDocument.Parse(json); - return Read(document.RootElement, registry); + return ReadVersioned(document.RootElement, registry); } /// @@ -258,23 +258,60 @@ public static MarkupText Deserialize(ReadOnlySpan utf8Json, MarkupRegistry if (reader.Read()) throw new JsonException("Unexpected content after the JSON value."); - return Read(document.RootElement, registry); + return ReadVersioned(document.RootElement, registry); } - internal static MarkupText Read(JsonElement root, MarkupRegistry? registry) + /// + /// Reads the top-level envelope after checking its wire version, which nested texts (inside a + /// layout) do not carry: a version is a property of the stored document, not of each text in it. + /// + private static MarkupText ReadVersioned(JsonElement root, MarkupRegistry? registry) + { + if (root.ValueKind == JsonValueKind.Object && !IsKnownVersion(root, out var version)) + throw new JsonException($"Markup wire version {version} is not one this reader knows; it reads version {WireVersion}."); + return Read(root, registry); + } + + /// + /// Whether is in a wire version this reader knows. No "v" is + /// version 1, the only one there is; is the raw value otherwise. + /// + private static bool IsKnownVersion(JsonElement root, out string version) + { + version = WireVersion.ToString(System.Globalization.CultureInfo.InvariantCulture); + if (!root.TryGetProperty("v", out var element)) return true; + version = element.GetRawText(); + return element.ValueKind == JsonValueKind.Number && element.TryGetInt32(out var number) && number == WireVersion; + } + + internal static MarkupText Read(JsonElement root, MarkupRegistry? registry) => Read(root, registry, check: null); + + /// + /// Reads the envelope. , when given, is told about everything the read + /// recovers from or leaves out; it is null on every path but . + /// + private static MarkupText Read(JsonElement root, MarkupRegistry? registry, ReadCheck? check) { // Every shape check below treats a malformed payload as a less marked-up one rather than // throwing: this reads rows written by older builds and bytes off a bus, and losing colour on // one corrupt attribute beats failing the read that carries it. - var text = root.TryGetProperty("t", out var textElement) && textElement.ValueKind == JsonValueKind.String + var hasText = root.TryGetProperty("t", out var textElement); + var text = hasText && textElement.ValueKind == JsonValueKind.String ? textElement.GetString() ?? string.Empty : string.Empty; + if (hasText && textElement.ValueKind != JsonValueKind.String) check?.Damage($"\"t\" is {Describe(textElement)}, not a string."); - if (!root.TryGetProperty("p", out var paletteElement) || paletteElement.ValueKind != JsonValueKind.Array - || !root.TryGetProperty("r", out var coverElement) || coverElement.ValueKind != JsonValueKind.Array) + var hasPalette = root.TryGetProperty("p", out var paletteElement); + var hasCover = root.TryGetProperty("r", out var coverElement); + if (!hasPalette || paletteElement.ValueKind != JsonValueKind.Array + || !hasCover || coverElement.ValueKind != JsonValueKind.Array) + { + if (hasPalette || hasCover) check?.Damage("\"p\" and \"r\" are not both arrays, so the text reads without its markup."); return MarkupText.Plain(text); + } - var palette = ReadPalette(paletteElement, registry); + var palette = ReadPalette(paletteElement, registry, check); + var used = check is null ? null : new bool[palette.Length]; var runs = ImmutableArray.CreateBuilder(); var position = 0; @@ -283,26 +320,59 @@ internal static MarkupText Read(JsonElement root, MarkupRegistry? registry) { // A non-numeric or negative length would desynchronise every later pair, so reading stops // there, as it does on a truncated pair rather than reading a length as an index. - if (!TryReadInt32(cover.Current, out var length) || length < 0) break; - if (!cover.MoveNext() || !TryReadInt32(cover.Current, out var index)) break; + if (!TryReadInt32(cover.Current, out var length) || length < 0) + { + check?.Damage($"\"r\" has {cover.Current.GetRawText()} where a length belongs; the rest of the cover is not read."); + break; + } + if (!cover.MoveNext() || !TryReadInt32(cover.Current, out var index)) + { + check?.Damage("\"r\" ends without the palette index of its last length."); + break; + } // A cover already at (or past) the end of the text has nothing left to describe, and an // unclipped length would let position overrun int range on the next add (two int.MaxValue // entries wrap it negative, which sorts runs out of order and trips the overlap check // below). Clipping here — rather than leaving it to the constructor — keeps position itself // bounded by text.Length across every iteration. - if (position >= text.Length) break; + if (position >= text.Length) + { + if (length > 0) check?.Damage("\"r\" covers more than the text."); + break; + } var runLength = Math.Min(length, text.Length - position); + if (runLength < length) check?.Damage("\"r\" covers more than the text."); // Slot 0 is the reserved "no markup" entry, and so is any index the palette does not have: // both leave the stretch as a gap, which is plain text. - if (index > 0 && index <= palette.Length) runs.Add(new Run(position, runLength, palette[index - 1])); + if (index > 0 && index <= palette.Length) + { + runs.Add(new Run(position, runLength, palette[index - 1])); + if (used is not null) used[index - 1] = true; + } + else if (index != 0) check?.Damage($"\"r\" names palette entry {index}, which \"p\" does not have."); position += runLength; } + // Writing builds the palette from the runs, so an entry nothing points at is not written back. + if (used is not null) + for (var i = 0; i < used.Length; i++) + if (!used[i]) check!.Loss($"Palette entry {i + 1} is not used by \"r\" and would not be written back."); + return new MarkupText(text, runs.ToImmutable()); } + private static string Describe(JsonElement element) => element.ValueKind switch + { + JsonValueKind.Object => "an object", + JsonValueKind.Array => "an array", + JsonValueKind.Number => "a number", + JsonValueKind.True or JsonValueKind.False => "a boolean", + JsonValueKind.Null => "null", + _ => "a string", + }; + private static bool TryReadInt32(JsonElement element, out int value) { value = 0; @@ -310,7 +380,7 @@ private static bool TryReadInt32(JsonElement element, out int value) } /// Reads the palette, skipping slot 0 (the reserved null entry); callers subtract one. - private static MarkupSet[] ReadPalette(JsonElement palette, MarkupRegistry? registry) + private static MarkupSet[] ReadPalette(JsonElement palette, MarkupRegistry? registry, ReadCheck? check) { var entries = new List(); var markups = new List(); @@ -320,14 +390,22 @@ private static MarkupSet[] ReadPalette(JsonElement palette, MarkupRegistry? regi if (first) { first = false; + if (entry.ValueKind != JsonValueKind.Null) check?.Damage("Palette entry 0 is not null; it is never read."); continue; } markups.Clear(); if (entry.ValueKind == JsonValueKind.Array) + { foreach (var markup in entry.EnumerateArray()) if (markup.ValueKind == JsonValueKind.Object) - markups.Add(ReadMarkup(markup, registry)); + markups.Add(check is null ? ReadMarkup(markup, registry) : ReadMarkup(markup, registry, check)); + else + check?.Damage($"Palette entry {entries.Count + 1} holds {Describe(markup)} where a markup object belongs."); + } + else check?.Damage($"Palette entry {entries.Count + 1} is {Describe(entry)}, not an array."); + if (markups.Count == 0 && entry.ValueKind == JsonValueKind.Array && entry.GetArrayLength() == 0) + check?.Damage($"Palette entry {entries.Count + 1} is empty."); // A set must hold at least one markup; an entry that read as none is a slot no cover index // can usefully point at, so it takes a neutral layer rather than dropping and shifting @@ -343,15 +421,190 @@ private static MarkupSet[] ReadPalette(JsonElement palette, MarkupRegistry? regi /// private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry) { - var kind = element.TryGetProperty("k", out var k) && k.ValueKind == JsonValueKind.String + var kind = KindOf(element); + if (kind == NeutralKind) return NeutralMarkup.Instance; + if (kind == Layout.LayoutJson.Kind) return Layout.LayoutJson.Read(element, registry); + var codec = ElementCodecs.Find(kind) ?? (registry ?? MarkupRegistry.Default).FindCodec(kind); + return codec is null ? new UnknownMarkup(kind, element.GetRawText()) : codec.Read(element); + } + + /// The kind of a markup object, inferred as described on this class for one with no "k". + private static string KindOf(JsonElement element) => + element.TryGetProperty("k", out var k) && k.ValueKind == JsonValueKind.String ? k.GetString()! : element.TryGetProperty("h", out _) ? "html" : element.TryGetProperty("n", out _) ? NeutralKind : "ansi"; - if (kind == NeutralKind) return NeutralMarkup.Instance; - if (kind == Layout.LayoutJson.Kind) return Layout.LayoutJson.Read(element, registry); - var codec = ElementCodecs.Find(kind) ?? (registry ?? MarkupRegistry.Default).FindCodec(kind); - return codec is null ? new UnknownMarkup(kind, element.GetRawText()) : codec.Read(element); + /// + /// for : a codec + /// that throws leaves the layer opaque instead of failing the read, and every layer a codec reads + /// is written straight back and compared with what it was read from, which is the only way to + /// know that a codec — core's, a package's or a game's — kept everything in it. + /// + private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry, ReadCheck check) + { + var kind = KindOf(element); + if (element.TryGetProperty("k", out var k) && k.ValueKind != JsonValueKind.String) + check.Damage($"A layer's \"k\" is {Describe(k)}, not a string; it reads as \"{kind}\"."); + IMarkup markup; + try + { + markup = ReadMarkup(element, registry); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + check.Damage($"The \"{kind}\" codec could not read a layer ({exception.Message}); it is kept as an unknown layer."); + return new UnknownMarkup(kind, element.GetRawText()); + } + + if (markup is UnknownMarkup) + { + check.Opaque(); + return markup; + } + + string written; + try + { + var buffer = new ArrayBufferWriter(); + using (var writer = new Utf8JsonWriter(buffer, WriterOptions)) WriteMarkup(writer, markup, registry); + written = Encoding.UTF8.GetString(buffer.WrittenSpan); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + check.Loss($"A \"{kind}\" layer could not be written back ({exception.Message})."); + return markup; + } + + using var rewritten = JsonDocument.Parse(written); + // A legacy neutral object is recognised by its "n", which the neutral kind does not write. + var ignore = kind == NeutralKind && !element.TryGetProperty("k", out _) ? "n" : null; + if (WireShape.Lost(element, rewritten.RootElement, ignore) is { } path) + check.Loss($"A \"{kind}\" layer would not be written back as read: {path}."); + return markup; + } + + // ── Inspecting ─────────────────────────────────────────────────────────────── + + /// The wire version this build reads and writes. Version 1 writes no "v". + public const int WireVersion = 1; + + /// The envelope properties of wire version 1. + private static readonly string[] EnvelopeProperties = ["t", "p", "r", "v"]; + + /// + /// Reads a stored using and reports + /// whether writing it back would keep everything the payload held. Never throws on bad input. + /// + /// The JSON produced by , or anything else. + public static MarkupReadResult Inspect(string json) => Inspect(json, registry: null); + + /// + /// Reads a stored and reports whether writing it back would keep + /// everything the payload held. Never throws on bad input. + /// + /// The JSON produced by , or anything else. + /// The codecs to use; when null. + public static MarkupReadResult Inspect(string json, MarkupRegistry? registry) + { + ArgumentNullException.ThrowIfNull(json); + return Inspect(Encoding.UTF8.GetBytes(json), registry); + } + + /// + /// Reads a stored from UTF-8 JSON using + /// and reports whether writing it back would keep everything the payload held. Never throws on bad input. + /// + /// The UTF-8 JSON produced by , or anything else. + public static MarkupReadResult Inspect(ReadOnlySpan utf8Json) => Inspect(utf8Json, registry: null); + + /// + /// Reads a stored from UTF-8 JSON and reports whether writing it back + /// would keep everything the payload held. Never throws on bad input. + /// + /// + /// An empty payload is , as reads it. + /// + /// The UTF-8 JSON produced by , or anything else. + /// The codecs to use; when null. + public static MarkupReadResult Inspect(ReadOnlySpan utf8Json, MarkupRegistry? registry) + { + if (utf8Json.IsEmpty) return new MarkupReadResult(MarkupReadStatus.Complete, MarkupText.Empty, []); + + var check = new ReadCheck(); + JsonDocument document; + var reader = new Utf8JsonReader(utf8Json); + try + { + document = JsonDocument.ParseValue(ref reader); + } + catch (JsonException exception) + { + check.Damage($"The payload is not JSON ({exception.Message})."); + return check.Result(value: null); + } + + using (document) + { + try + { + if (reader.Read()) check.Damage("There is content after the JSON value."); + } + catch (JsonException) + { + check.Damage("There is content after the JSON value."); + } + + var root = document.RootElement; + if (root.ValueKind != JsonValueKind.Object) + { + check.Damage($"The payload is {Describe(root)}, not an object."); + return check.Result(value: null); + } + + // The version decides what every other property means, so it is read before any of them, + // and a version this reader does not know is not read as one it does. + if (!IsKnownVersion(root, out var version)) + { + check.Unsupported($"Wire version {version} is not one this reader knows; it reads version {WireVersion}."); + return check.Result(value: null); + } + + foreach (var property in root.EnumerateObject()) + if (Array.IndexOf(EnvelopeProperties, property.Name) < 0) + check.Loss($"The envelope property \"{property.Name}\" is not one this reader knows and would not be written back."); + + return check.Result(Read(root, registry, check)); + } + } + + /// What one found. + private sealed class ReadCheck + { + private readonly List _diagnostics = []; + private MarkupReadStatus _status = MarkupReadStatus.Complete; + + public void Opaque() => Raise(MarkupReadStatus.Opaque); + + public void Damage(string diagnostic) => Add(MarkupReadStatus.Damaged, diagnostic); + + public void Unsupported(string diagnostic) => Add(MarkupReadStatus.Unsupported, diagnostic); + + /// Something well formed the reader does not keep: unsupported, not damaged. + public void Loss(string diagnostic) => Add(MarkupReadStatus.Unsupported, diagnostic); + + private void Add(MarkupReadStatus status, string diagnostic) + { + _diagnostics.Add(diagnostic); + Raise(status); + } + + private void Raise(MarkupReadStatus status) + { + if (status > _status) _status = status; + } + + public MarkupReadResult Result(MarkupText? value) => new(_status, value, [.. _diagnostics]); } } diff --git a/MarkupString/PublicAPI.Shipped.txt b/MarkupString/PublicAPI.Shipped.txt index 7572a5a..5dac760 100644 --- a/MarkupString/PublicAPI.Shipped.txt +++ b/MarkupString/PublicAPI.Shipped.txt @@ -1685,3 +1685,18 @@ virtual MarkupString.Layout.LayoutCell.PrintMembers(System.Text.StringBuilder! b ~override MarkupString.Run.ToString() -> string ~override MarkupString.ThemeColor.Equals(object obj) -> bool ~override MarkupString.ThemeColor.ToString() -> string +MarkupString.MarkupReadResult +MarkupString.MarkupReadResult.CanRewrite.get -> bool +MarkupString.MarkupReadResult.Diagnostics.get -> System.Collections.Immutable.ImmutableArray +MarkupString.MarkupReadResult.Status.get -> MarkupString.MarkupReadStatus +MarkupString.MarkupReadResult.Value.get -> MarkupString.MarkupText? +MarkupString.MarkupReadStatus +MarkupString.MarkupReadStatus.Complete = 0 -> MarkupString.MarkupReadStatus +MarkupString.MarkupReadStatus.Damaged = 3 -> MarkupString.MarkupReadStatus +MarkupString.MarkupReadStatus.Opaque = 1 -> MarkupString.MarkupReadStatus +MarkupString.MarkupReadStatus.Unsupported = 2 -> MarkupString.MarkupReadStatus +const MarkupString.MarkupTextSerializer.WireVersion = 1 -> int +static MarkupString.MarkupTextSerializer.Inspect(System.ReadOnlySpan utf8Json) -> MarkupString.MarkupReadResult! +static MarkupString.MarkupTextSerializer.Inspect(System.ReadOnlySpan utf8Json, MarkupString.MarkupRegistry? registry) -> MarkupString.MarkupReadResult! +static MarkupString.MarkupTextSerializer.Inspect(string! json) -> MarkupString.MarkupReadResult! +static MarkupString.MarkupTextSerializer.Inspect(string! json, MarkupString.MarkupRegistry? registry) -> MarkupString.MarkupReadResult! diff --git a/MarkupString/WireShape.cs b/MarkupString/WireShape.cs new file mode 100644 index 0000000..f800bc4 --- /dev/null +++ b/MarkupString/WireShape.cs @@ -0,0 +1,63 @@ +using System.Text.Json; +namespace MarkupString; + +/// +/// Compares a markup object as read with the same object written back, to find what the write +/// would leave out. +/// +/// +/// A codec may write a value in a different form from the one it read — a legacy colour array as +/// a palette index, "bo":0 as "bo":1 — so scalar values are not compared, and an array +/// may come back as a scalar. What may not change is the structure: every property read must be +/// written, an object must stay an object, an array that stays an array must keep its length, and +/// the discriminators ("k" for a markup, "t" for a block or a text) must keep their value. +/// +internal static class WireShape +{ + /// + /// Where leaves out part of , as a path from the + /// object's root, or null when it keeps all of it. names one top-level + /// property that is allowed to go. + /// + public static string? Lost(JsonElement read, JsonElement written, string? ignore) => Lost(read, written, ignore, ""); + + private static string? Lost(JsonElement read, JsonElement written, string? ignore, string path) + { + switch (read.ValueKind) + { + case JsonValueKind.Object: + if (written.ValueKind != JsonValueKind.Object) return $"{Where(path)} is no longer an object"; + foreach (var property in read.EnumerateObject()) + { + if (property.Name == ignore) continue; + var at = $"{path}/{property.Name}"; + if (!written.TryGetProperty(property.Name, out var value)) return $"{at} is dropped"; + if (property.Name is "k" or "t" && property.Value.ValueKind == JsonValueKind.String + && (value.ValueKind != JsonValueKind.String || value.GetString() != property.Value.GetString())) + return $"{at} changes from {property.Value.GetRawText()} to {value.GetRawText()}"; + if (Lost(property.Value, value, ignore: null, at) is { } lost) return lost; + } + return null; + + case JsonValueKind.Array: + if (written.ValueKind == JsonValueKind.Object) return $"{Where(path)} is no longer an array"; + if (written.ValueKind != JsonValueKind.Array) return null; + var count = read.GetArrayLength(); + if (written.GetArrayLength() != count) return $"{Where(path)} has {count} entries and would be written with {written.GetArrayLength()}"; + var index = 0; + using (var left = read.EnumerateArray()) + using (var right = written.EnumerateArray()) + while (left.MoveNext() && right.MoveNext()) + { + if (Lost(left.Current, right.Current, ignore: null, $"{path}/{index}") is { } lost) return lost; + index++; + } + return null; + + default: + return null; + } + } + + private static string Where(string path) => path.Length == 0 ? "the object" : path; +} diff --git a/docs/serialization.md b/docs/serialization.md index 6c41ceb..dc2e719 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -58,7 +58,10 @@ MarkupTextSerializer.Serialize(read, withoutThem); // byte-identical to the o ``` That is the property that makes a rolling deployment safe: an old reader that meets markup from a -newer writer neither loses it nor corrupts it. +newer writer neither loses it nor corrupts it. That holds for whole layers of an unknown kind. A +new property on a kind the reader does know is dropped when that layer is written back, and so is +a layout block of an unknown kind; `Inspect` (below) reports both, so a caller can keep the +original instead of saving the loss. Writing is stricter than reading. A markup with **no codec at write time** throws `InvalidOperationException` naming the type, because silently dropping markup you asked to store is @@ -68,11 +71,66 @@ a different class of problem from not styling it: No markup codec is registered for SpoilerMarkup. Add one with MarkupRegistry.With(IMarkupCodec). ``` +## Wire version + +The shape above is wire version 1, `MarkupTextSerializer.WireVersion`. Version 1 writes no `"v"` +property, and every released writer since 1.0.0 has written it. A reader treats a missing `"v"`, or +`"v":1`, as version 1. + +Any other `"v"` is a format this reader does not know. The reader checks it before it reads anything +else, because the version decides what the other properties mean. `Deserialize` throws a +`JsonException` for an unknown version, and `Inspect` reports it as `Unsupported` with no value. A +row in an unknown version is never read as version 1 and never saved back as an empty or plain text. + +The wire version is separate from the package version: a release that adds a markup kind or a +property does not change it. It changes only if the envelope itself changes in a way a version 1 +reader would misread, and a reader that knows the new version keeps reading version 1. + +## Reading stored text for saving: `Inspect` + +`Deserialize` is for showing text. It recovers what it can from a damaged or partly understood row +and does not say what it left out. A caller that might save the value back over the original +should read with `Inspect`, which reads the same way and also reports what it found: + +```csharp +var result = MarkupTextSerializer.Inspect(stored, registry); + +if (result.CanRewrite) + stored = MarkupTextSerializer.Serialize(Edit(result.Value!), registry); +else + Log(result.Status, result.Diagnostics); // keep the stored bytes; show result.Value if there is one +``` + +| `Status` | What it means | `CanRewrite` | +|---|---|---| +| `Complete` | Every part of the payload was understood, and writing it back keeps all of it. | yes | +| `Opaque` | Some layers are of kinds no codec in the registry claims. They are kept as `UnknownMarkup` and written back verbatim. | yes | +| `Unsupported` | Well formed, but it holds something this reader does not know and would not write back: an unknown wire version, an unknown envelope property, an unknown property on a known kind, a layout block of an unknown kind, a palette entry nothing uses. | no | +| `Damaged` | Not what any writer produces: not JSON, not an object, content after the value, a field of the wrong type, a cover that overruns the text or points past the palette, a codec that throws. | no | + +When several things are wrong, the worst one decides `Status`, and `Diagnostics` lists them all. + +`Value` is null only when nothing could be read without guessing: the payload is not a JSON object, +or its wire version is unknown. Otherwise it is the same text `Deserialize` returns, fit to show. +A codec that throws does not fail `Inspect`: its layer is kept as an `UnknownMarkup` and the result +is `Damaged`. `Deserialize` still lets the exception through. + +To find what a codec would leave out, `Inspect` writes each layer it read straight back and +compares the two. A codec may write a value in a newer form than the one it read, such as a legacy +colour array as a palette index, so values are not compared. Structure is compared: every property +read must be written, an object must stay an object, an array must keep its length, and the +`"k"` and `"t"` discriminators must keep their values. This catches a game's own codecs as well as +the shipped ones, without each codec having to report what it skipped. + +`Inspect` does more work than `Deserialize`, since it writes every layer a second time. Use it on +the path that saves a value back, not on every read. + ## Validation -Both `Deserialize` overloads reject trailing content after the document. A run cover that overflows -the text is clipped rather than rejected, so a truncated or slightly-wrong `r` degrades to less -markup instead of an exception. +Both `Deserialize` overloads reject trailing content after the document and an unknown wire +version. A run cover that overflows the text is clipped rather than rejected, so a truncated or +slightly-wrong `r` degrades to less markup instead of an exception; `Inspect` reports it as +`Damaged`. ## Writing a codec From bbbc017ed2552ed843e1bab15fe0366d946858ab Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:21:48 +0000 Subject: [PATCH 2/5] Never fail a read on a newer wire version or a throwing codec A payload with an unknown "v" now reads as its "t" in plain text, with nothing else read as version 1, instead of throwing; every later version keeps "t" as the document's plain text so that fallback always exists. A codec that throws leaves its layer as an UnknownMarkup holding the raw object, so Deserialize no longer fails and the layer writes back as it was. Inspect still reports both, so a caller that saves values back can keep the stored row. Refs #61 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BxoP4iqJzh4jXnBsnkAFt --- CHANGELOG.md | 8 ++- MarkupString.Tests/WireCompatibilityTests.cs | 44 ++++++++++--- MarkupString/MarkupReadResult.cs | 5 +- MarkupString/MarkupTextSerializer.cs | 65 +++++++++++++------- docs/serialization.md | 25 +++++--- 5 files changed, 101 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9505d2e..c0a1f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,12 +20,14 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). overruns its text, a codec that throws, and payloads that are not JSON objects. See [Serialization](docs/serialization.md#reading-stored-text-for-saving-inspect). - **A wire version.** The stored shape is wire version 1 (`MarkupTextSerializer.WireVersion`), which - writes no `"v"`, as every release since 1.0.0 has. The bytes written are unchanged. + writes no `"v"`, as every release since 1.0.0 has. The bytes written are unchanged. A payload with + any other `"v"` reads as its `"t"` in plain text, without the rest being read as version 1, and + every later version keeps `"t"` as the document's plain text so that this fallback always works. ### Changed -- **`Deserialize` throws `JsonException` for a `"v"` other than 1,** instead of reading the payload - as version 1. No writer has produced one, so no stored row changes how it reads. +- **A codec that throws no longer fails the read.** Its layer reads as an `UnknownMarkup` holding the + raw object, which renders as nothing and is written back as it was. ## 2.18.1 — 2026-10-13 diff --git a/MarkupString.Tests/WireCompatibilityTests.cs b/MarkupString.Tests/WireCompatibilityTests.cs index e2f98fd..1572e61 100644 --- a/MarkupString.Tests/WireCompatibilityTests.cs +++ b/MarkupString.Tests/WireCompatibilityTests.cs @@ -86,25 +86,35 @@ public async Task UnknownKind_IsOpaque_AndWritesBackByteForByte() [Arguments("""{"v":99,"t":"x","future":true}""")] [Arguments("""{"v":2,"text":"important"}""")] [Arguments("""{"v":"1","t":"x"}""")] - public async Task UnknownVersion_IsUnsupported_AndNotRead(string json) + public async Task UnknownVersion_IsUnsupported(string json) { var result = MarkupTextSerializer.Inspect(json, Registry); await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Unsupported); await Assert.That(result.CanRewrite).IsFalse(); - await Assert.That(result.Value).IsNull(); await Assert.That(result.Diagnostics.Single()).Contains("Wire version"); } + /// + /// A newer row shows as its plain text, and nothing else in it is read as version 1: its + /// palette and cover may mean something else in the version that wrote them. + /// [Test] - public async Task UnknownVersion_Deserialize_Throws() - => await Assert.That(() => MarkupTextSerializer.Deserialize("""{"v":2,"text":"important"}""", Registry)) - .Throws(); + public async Task UnknownVersion_ShowsItsPlainTextOnly() + { + const string json = """{"v":2,"t":"Hello world","p":[null,[{"k":"ansi","f":9}]],"r":[5,1]}"""; + + await Assert.That(MarkupTextSerializer.Inspect(json, Registry).Value).IsEqualTo(MarkupText.Plain("Hello world")); + var read = MarkupTextSerializer.Deserialize(json, Registry); + await Assert.That(read.Text).IsEqualTo("Hello world"); + await Assert.That(read.Runs).IsEmpty(); + await Assert.That(MarkupTextSerializer.Deserialize(Encoding.UTF8.GetBytes(json), Registry).Text).IsEqualTo("Hello world"); + } [Test] - public async Task UnknownVersion_Utf8Deserialize_Throws() - => await Assert.That(() => MarkupTextSerializer.Deserialize(Encoding.UTF8.GetBytes("""{"v":2,"t":"x"}"""), Registry)) - .Throws(); + public async Task UnknownVersion_WithNoText_ShowsAsEmpty() + => await Assert.That(MarkupTextSerializer.Deserialize("""{"v":2,"text":"important"}""", Registry)) + .IsSameReferenceAs(MarkupText.Empty); [Test] public async Task VersionOne_IsRead() @@ -256,7 +266,23 @@ public async Task ThrowingCodec_IsDamaged_AndKeepsTheLayerOpaque() await Assert.That(result.Status).IsEqualTo(MarkupReadStatus.Damaged); await Assert.That(result.Value!.Runs[0].Markups[0]).IsEqualTo((IMarkup)new UnknownMarkup("boom", """{"k":"boom","q":1}""")); - await Assert.That(() => MarkupTextSerializer.Deserialize(json, registry)).Throws(); + + // Deserialize reads it the same way rather than failing the read, and the layer writes back as it was. + var read = MarkupTextSerializer.Deserialize(json, registry); + await Assert.That(read.Runs[0].Markups[0]).IsEqualTo((IMarkup)new UnknownMarkup("boom", """{"k":"boom","q":1}""")); + await Assert.That(MarkupTextSerializer.Serialize(read, registry)).IsEqualTo(json); + } + + [Test] + public async Task ThrowingCodec_InsideALayout_DoesNotFailTheRead() + { + var json = Layout.Replace("{\"k\":\"ansi\",\"f\":10}", "{\"k\":\"boom\",\"f\":10}"); + var registry = Registry.With(new ThrowingCodec()); + + var read = MarkupTextSerializer.Deserialize(json, registry); + + await Assert.That(read.Text).IsEqualTo(MarkupTextSerializer.Deserialize(Layout, Registry).Text); + await Assert.That(MarkupTextSerializer.Inspect(json, registry).CanRewrite).IsFalse(); } [Test] diff --git a/MarkupString/MarkupReadResult.cs b/MarkupString/MarkupReadResult.cs index 8405b15..3bbd45d 100644 --- a/MarkupString/MarkupReadResult.cs +++ b/MarkupString/MarkupReadResult.cs @@ -52,8 +52,9 @@ internal MarkupReadResult(MarkupReadStatus status, MarkupText? value, ImmutableA public MarkupReadStatus Status { get; } /// - /// The text read. Null when nothing could be read without guessing: the payload is not a JSON - /// object, or it names a wire version this reader does not know. + /// The text read, the same text returns. + /// For a wire version this reader does not know it is the payload's plain text only. Null when + /// the payload is not a JSON object, so there is no text to show. /// public MarkupText? Value { get; } diff --git a/MarkupString/MarkupTextSerializer.cs b/MarkupString/MarkupTextSerializer.cs index 867d0da..df4aa19 100644 --- a/MarkupString/MarkupTextSerializer.cs +++ b/MarkupString/MarkupTextSerializer.cs @@ -265,12 +265,19 @@ public static MarkupText Deserialize(ReadOnlySpan utf8Json, MarkupRegistry /// Reads the top-level envelope after checking its wire version, which nested texts (inside a /// layout) do not carry: a version is a property of the stored document, not of each text in it. /// - private static MarkupText ReadVersioned(JsonElement root, MarkupRegistry? registry) - { - if (root.ValueKind == JsonValueKind.Object && !IsKnownVersion(root, out var version)) - throw new JsonException($"Markup wire version {version} is not one this reader knows; it reads version {WireVersion}."); - return Read(root, registry); - } + private static MarkupText ReadVersioned(JsonElement root, MarkupRegistry? registry) => + root.ValueKind == JsonValueKind.Object && !IsKnownVersion(root, out _) ? Fallback(root) : Read(root, registry); + + /// + /// What a payload in a wire version this reader does not know shows as: its "t" as plain + /// text. Every wire version keeps "t" as the plain text of the whole document so that an + /// older reader has something to show; nothing else in the payload is read, since its meaning + /// belongs to a version this reader does not know. + /// + private static MarkupText Fallback(JsonElement root) => + root.TryGetProperty("t", out var text) && text.ValueKind == JsonValueKind.String + ? MarkupText.Plain(text.GetString() ?? string.Empty) + : MarkupText.Empty; /// /// Whether is in a wire version this reader knows. No "v" is @@ -419,13 +426,30 @@ private static MarkupSet[] ReadPalette(JsonElement palette, MarkupRegistry? regi /// Reads one markup layer. As on the write side, falls back to /// only where a codec is actually looked up. /// - private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry) + private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry) => ReadMarkup(element, registry, out _); + + /// + /// Reads one markup layer. A codec that throws — a game's own, or one meeting a row it was never + /// written for — leaves the layer as an holding the raw object, so the + /// read still succeeds, the layer renders as nothing, and writing the text back keeps it. + /// is the codec's message when it threw. + /// + private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry, out string? failure) { + failure = null; var kind = KindOf(element); if (kind == NeutralKind) return NeutralMarkup.Instance; - if (kind == Layout.LayoutJson.Kind) return Layout.LayoutJson.Read(element, registry); - var codec = ElementCodecs.Find(kind) ?? (registry ?? MarkupRegistry.Default).FindCodec(kind); - return codec is null ? new UnknownMarkup(kind, element.GetRawText()) : codec.Read(element); + try + { + if (kind == Layout.LayoutJson.Kind) return Layout.LayoutJson.Read(element, registry); + var codec = ElementCodecs.Find(kind) ?? (registry ?? MarkupRegistry.Default).FindCodec(kind); + return codec is null ? new UnknownMarkup(kind, element.GetRawText()) : codec.Read(element); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + failure = exception.Message; + return new UnknownMarkup(kind, element.GetRawText()); + } } /// The kind of a markup object, inferred as described on this class for one with no "k". @@ -438,24 +462,19 @@ private static string KindOf(JsonElement element) => /// /// for : a codec - /// that throws leaves the layer opaque instead of failing the read, and every layer a codec reads - /// is written straight back and compared with what it was read from, which is the only way to - /// know that a codec — core's, a package's or a game's — kept everything in it. + /// that throws is reported, and every layer a codec reads is written straight back and compared + /// with what it was read from, which is the only way to know that a codec — core's, a package's or a game's — kept everything in it. /// private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry, ReadCheck check) { var kind = KindOf(element); if (element.TryGetProperty("k", out var k) && k.ValueKind != JsonValueKind.String) check.Damage($"A layer's \"k\" is {Describe(k)}, not a string; it reads as \"{kind}\"."); - IMarkup markup; - try + var markup = ReadMarkup(element, registry, out var failure); + if (failure is not null) { - markup = ReadMarkup(element, registry); - } - catch (Exception exception) when (exception is not OutOfMemoryException) - { - check.Damage($"The \"{kind}\" codec could not read a layer ({exception.Message}); it is kept as an unknown layer."); - return new UnknownMarkup(kind, element.GetRawText()); + check.Damage($"The \"{kind}\" codec could not read a layer ({failure}); it is kept as an unknown layer."); + return markup; } if (markup is UnknownMarkup) @@ -567,8 +586,8 @@ public static MarkupReadResult Inspect(ReadOnlySpan utf8Json, MarkupRegist // and a version this reader does not know is not read as one it does. if (!IsKnownVersion(root, out var version)) { - check.Unsupported($"Wire version {version} is not one this reader knows; it reads version {WireVersion}."); - return check.Result(value: null); + check.Unsupported($"Wire version {version} is not one this reader knows; it reads version {WireVersion}. The value is its plain text only."); + return check.Result(Fallback(root)); } foreach (var property in root.EnumerateObject()) diff --git a/docs/serialization.md b/docs/serialization.md index dc2e719..3181a89 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -78,9 +78,15 @@ property, and every released writer since 1.0.0 has written it. A reader treats `"v":1`, as version 1. Any other `"v"` is a format this reader does not know. The reader checks it before it reads anything -else, because the version decides what the other properties mean. `Deserialize` throws a -`JsonException` for an unknown version, and `Inspect` reports it as `Unsupported` with no value. A -row in an unknown version is never read as version 1 and never saved back as an empty or plain text. +else, because the version decides what the other properties mean. It does not fail the read: it +shows the payload's `"t"` as plain text, and reads nothing else, since a palette or cover from +another version may not mean what version 1 means by it. `Inspect` reports the same value as +`Unsupported`, so a caller keeps the stored row rather than saving the plain text over it. + +That fallback is a rule every later version has to keep: **`"t"` is always the plain text of the +whole document**, so a reader that knows only an earlier version always has something to show. +It is the same idea as the plain-text `body` a Matrix client falls back on for an event type it +does not know. The wire version is separate from the package version: a release that adds a markup kind or a property does not change it. It changes only if the envelope itself changes in a way a version 1 @@ -110,10 +116,10 @@ else When several things are wrong, the worst one decides `Status`, and `Diagnostics` lists them all. -`Value` is null only when nothing could be read without guessing: the payload is not a JSON object, -or its wire version is unknown. Otherwise it is the same text `Deserialize` returns, fit to show. -A codec that throws does not fail `Inspect`: its layer is kept as an `UnknownMarkup` and the result -is `Damaged`. `Deserialize` still lets the exception through. +`Value` is the same text `Deserialize` returns, fit to show. It is null only when the payload is +not a JSON object, so there is no text in it. A codec that throws fails neither read: its layer is +kept as an `UnknownMarkup` holding the raw object, renders as nothing and writes back as it was, +and `Inspect` reports the result as `Damaged`. To find what a codec would leave out, `Inspect` writes each layer it read straight back and compares the two. A codec may write a value in a newer form than the one it read, such as a legacy @@ -127,8 +133,9 @@ the path that saves a value back, not on every read. ## Validation -Both `Deserialize` overloads reject trailing content after the document and an unknown wire -version. A run cover that overflows the text is clipped rather than rejected, so a truncated or +Both `Deserialize` overloads reject trailing content after the document, and they throw for a +payload that is not JSON or not an object, since there is no text in it to show. Nothing else +fails a read. A run cover that overflows the text is clipped rather than rejected, so a truncated or slightly-wrong `r` degrades to less markup instead of an exception; `Inspect` reports it as `Damaged`. From 20dcde413224c545697a03b7d9127b6f42ead094 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:25:35 +0000 Subject: [PATCH 3/5] Keep a layout read from an edited copy from being interned as the original LayoutMarkup equality compared id, width, length and text hash but not the tree, so interning could hand one read another read's tree when a copy of a row kept the layout id but changed the tree. A layer read back now also carries a fingerprint of the tree's JSON, and equals another only when they share the tree or that fingerprint. The new wire tests hit this as a cross-test race on macOS CI. Refs #61 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BxoP4iqJzh4jXnBsnkAFt --- CHANGELOG.md | 7 +++++++ MarkupString.Tests/WireCompatibilityTests.cs | 15 +++++++++++++++ MarkupString/Layout/Blocks/LayoutJson.cs | 6 +++++- MarkupString/Layout/Blocks/LayoutMarkup.cs | 15 +++++++++++++-- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0a1f37..ec7c16d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,13 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - **A codec that throws no longer fails the read.** Its layer reads as an `UnknownMarkup` holding the raw object, which renders as nothing and is written back as it was. +### Fixed + +- **A layout read from a copy of a row with a different tree is no longer taken for the original.** + Layers are interned by equality, which compared a layout's id and text but not its tree, so after + reading an edited or damaged copy, reading the original could hand back the copy's tree, and + saving it wrote that tree. + ## 2.18.1 — 2026-10-13 ### Changed diff --git a/MarkupString.Tests/WireCompatibilityTests.cs b/MarkupString.Tests/WireCompatibilityTests.cs index 1572e61..94fd0bb 100644 --- a/MarkupString.Tests/WireCompatibilityTests.cs +++ b/MarkupString.Tests/WireCompatibilityTests.cs @@ -285,6 +285,21 @@ public async Task ThrowingCodec_InsideALayout_DoesNotFailTheRead() await Assert.That(MarkupTextSerializer.Inspect(json, registry).CanRewrite).IsFalse(); } + /// + /// A copy of a row with the same layout id but a different tree must not be interned as the + /// original, or saving the original after reading the copy would write the copy's tree. + /// + [Test] + public async Task LayoutWithTheSameId_AndAnotherTree_IsNotTakenForTheOriginal() + { + var copy = Layout.Replace("\"t\":\"Hooooo?\"", "\"t\":\"Hiiiiii\""); + _ = MarkupTextSerializer.Deserialize(copy, Registry); + + var read = MarkupTextSerializer.Deserialize(Layout, Registry); + + await Assert.That(MarkupTextSerializer.Serialize(read, Registry)).IsEqualTo(Layout); + } + [Test] public async Task WorstFinding_Wins() { diff --git a/MarkupString/Layout/Blocks/LayoutJson.cs b/MarkupString/Layout/Blocks/LayoutJson.cs index d5eea7c..0cc76c3 100644 --- a/MarkupString/Layout/Blocks/LayoutJson.cs +++ b/MarkupString/Layout/Blocks/LayoutJson.cs @@ -32,7 +32,11 @@ public static IMarkup Read(JsonElement element, MarkupRegistry? registry) // A tree that could not all be read must not be drawn in place of its text, or laid out again: // a length no text has keeps the block from ever counting as intact. var length = state.Incomplete ? -1 : layout.Int("l") ?? -1; - return new LayoutMarkup(root, layout.Int("w") ?? 0, layout.Bool("fl"), length, layout.Hex("h"), layout.Hex("id")); + var source = element.TryGetProperty("n", out var tree) ? LayoutMarkup.Fingerprint(tree.GetRawText()) : 0UL; + return new LayoutMarkup(root, layout.Int("w") ?? 0, layout.Bool("fl"), length, layout.Hex("h"), layout.Hex("id")) + { + Source = source == 0 ? 1 : source, + }; } public static BlockCodec? CodecFor(Type type, MarkupRegistry? registry) => diff --git a/MarkupString/Layout/Blocks/LayoutMarkup.cs b/MarkupString/Layout/Blocks/LayoutMarkup.cs index 09047f4..859fb48 100644 --- a/MarkupString/Layout/Blocks/LayoutMarkup.cs +++ b/MarkupString/Layout/Blocks/LayoutMarkup.cs @@ -8,7 +8,9 @@ namespace MarkupString.Layout; /// Two layers are equal when they are the same layout: one , laid out at the same /// width over the same text. A block that spans several styled runs is therefore one region to the /// renderer however it was read back, and two layouts built apart are two regions however alike they -/// look, so neither ever stands in for the other. +/// look, so neither ever stands in for the other. A layer read back is also equal only to one read +/// from the same tree (or holding the same ), so a copy of a row whose tree was +/// edited or damaged is never interned as the original, nor the original as it. /// public sealed class LayoutMarkup : IBlockMarkup, IEquatable { @@ -48,6 +50,14 @@ public LayoutMarkup(Block root, int width, bool fluid, int length, ulong hash, u /// The fingerprint of that text. public ulong Hash { get; } + /// + /// The fingerprint of the JSON the tree was read from, or zero for a tree built in this process. + /// Two layers read from the same row share it however many runs they were read into; a copy of + /// the row whose tree was edited or damaged does not, so interning never hands one read the + /// other's tree. + /// + internal ulong Source { get; init; } + /// public bool Covers(ReadOnlySpan text) => text.Length == Length && Fingerprint(text) == Hash; @@ -67,7 +77,8 @@ public static ulong Fingerprint(ReadOnlySpan text) /// public bool Equals(LayoutMarkup? other) => - other is not null && Id == other.Id && Width == other.Width && Fluid == other.Fluid && Length == other.Length && Hash == other.Hash; + other is not null && Id == other.Id && Width == other.Width && Fluid == other.Fluid && Length == other.Length && Hash == other.Hash + && (ReferenceEquals(Root, other.Root) || (Source != 0 && Source == other.Source)); /// public override bool Equals(object? obj) => obj is LayoutMarkup other && Equals(other); From bdf95f9a7b688b61875fc96e7eaf9be7e6228e0d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:38:28 +0000 Subject: [PATCH 4/5] VersionOf and NeedsUpgrade, and the rules an upgrade step keeps VersionOf reads a stored text's wire version from its top-level "v" without parsing the rest (1 when absent, null when unreadable), and NeedsUpgrade says whether it is earlier than WireVersion. The serialization guide sets out how later versions upgrade: one step from the version before, chained, working on the JSON, idempotent, leaving newer and damaged payloads alone, keeping "t", with fixtures; batching, backups and checkpoints belong to the store. Refs #61 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BxoP4iqJzh4jXnBsnkAFt --- CHANGELOG.md | 3 ++ MarkupString.Tests/WireCompatibilityTests.cs | 39 ++++++++++++++ MarkupString/MarkupTextSerializer.cs | 57 ++++++++++++++++++++ MarkupString/PublicAPI.Shipped.txt | 4 ++ docs/serialization.md | 27 ++++++++++ 5 files changed, 130 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec7c16d..da1fb26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). writes no `"v"`, as every release since 1.0.0 has. The bytes written are unchanged. A payload with any other `"v"` reads as its `"t"` in plain text, without the rest being read as version 1, and every later version keeps `"t"` as the document's plain text so that this fallback always works. +- **`MarkupTextSerializer.VersionOf` and `NeedsUpgrade`** report the wire version a stored text was + written in, without parsing the rest of it, so that a store can find what a later version would + upgrade. See [Upgrading stored text](docs/serialization.md#upgrading-stored-text). ### Changed diff --git a/MarkupString.Tests/WireCompatibilityTests.cs b/MarkupString.Tests/WireCompatibilityTests.cs index 94fd0bb..0461565 100644 --- a/MarkupString.Tests/WireCompatibilityTests.cs +++ b/MarkupString.Tests/WireCompatibilityTests.cs @@ -195,6 +195,45 @@ public async Task UnusedPaletteEntry_IsUnsupported() await Assert.That(result.Diagnostics.Single()).Contains("Palette entry 2"); } + // ── Version ────────────────────────────────────────────────────────────────── + + [Test] + [MethodDataSource(nameof(Released))] + public async Task Released_IsVersionOne_AndNeedsNoUpgrade(string json) + { + await Assert.That(MarkupTextSerializer.VersionOf(json)).IsEqualTo(1); + await Assert.That(MarkupTextSerializer.NeedsUpgrade(json)).IsFalse(); + await Assert.That(MarkupTextSerializer.VersionOf(Encoding.UTF8.GetBytes(json))).IsEqualTo(1); + } + + [Test] + [Arguments("", 1)] + [Arguments("""{"v":1,"t":"x"}""", 1)] + [Arguments("""{"t":"x","p":[null,[{"k":"zzz","v":7}]],"r":[1,1],"v":3}""", 3)] + [Arguments("""{"t":{"v":9},"v":2}""", 2)] + [Arguments("""{"v":0}""", 0)] + public async Task VersionOf_ReadsTheTopLevelVersion(string json, int version) + => await Assert.That(MarkupTextSerializer.VersionOf(json)).IsEqualTo(version); + + [Test] + [Arguments("not json")] + [Arguments("[1]")] + [Arguments("""{"v":"2"}""")] + [Arguments("""{"v":1.5}""")] + [Arguments("""{"t":"x" """)] + public async Task VersionOf_UnreadableVersion_IsNull(string json) + { + await Assert.That(MarkupTextSerializer.VersionOf(json)).IsNull(); + await Assert.That(MarkupTextSerializer.NeedsUpgrade(json)).IsFalse(); + } + + [Test] + public async Task NeedsUpgrade_OnlyForAnEarlierVersion() + { + await Assert.That(MarkupTextSerializer.NeedsUpgrade("""{"v":0}""")).IsTrue(); + await Assert.That(MarkupTextSerializer.NeedsUpgrade("""{"v":2}""")).IsFalse(); + } + // ── Damaged ────────────────────────────────────────────────────────────────── [Test] diff --git a/MarkupString/MarkupTextSerializer.cs b/MarkupString/MarkupTextSerializer.cs index df4aa19..7c5f784 100644 --- a/MarkupString/MarkupTextSerializer.cs +++ b/MarkupString/MarkupTextSerializer.cs @@ -509,6 +509,63 @@ private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry, /// The wire version this build reads and writes. Version 1 writes no "v". public const int WireVersion = 1; + /// + /// The wire version was written in, read without parsing the rest of it: + /// 1 when it has no "v" or is empty, null when it is not a JSON object or its "v" is + /// not a whole number. + /// + /// A stored text, in any version. + public static int? VersionOf(string json) + { + ArgumentNullException.ThrowIfNull(json); + return VersionOf(Encoding.UTF8.GetBytes(json)); + } + + /// + /// The wire version was written in, read without parsing the rest of + /// it: 1 when it has no "v" or is empty, null when it is not a JSON object or its + /// "v" is not a whole number. + /// + /// A stored text as UTF-8, in any version. + public static int? VersionOf(ReadOnlySpan utf8Json) + { + if (utf8Json.IsEmpty) return WireVersion; + var reader = new Utf8JsonReader(utf8Json); + try + { + if (!reader.Read() || reader.TokenType != JsonTokenType.StartObject) return null; + while (reader.Read() && reader.TokenType == JsonTokenType.PropertyName) + { + var isVersion = reader.ValueTextEquals("v"u8); + if (!reader.Read()) return null; + if (isVersion) + return reader.TokenType == JsonTokenType.Number && reader.TryGetInt32(out var version) ? version : null; + reader.Skip(); + } + return reader.TokenType == JsonTokenType.EndObject ? WireVersion : null; + } + catch (JsonException) + { + return null; + } + } + + /// + /// Whether is in an earlier wire version than , so + /// that an upgrade step would rewrite it. False for the current version, a newer one, and a + /// payload whose version cannot be read. + /// + /// A stored text, in any version. + public static bool NeedsUpgrade(string json) => VersionOf(json) < WireVersion; + + /// + /// Whether is in an earlier wire version than , + /// so that an upgrade step would rewrite it. False for the current version, a newer one, and a + /// payload whose version cannot be read. + /// + /// A stored text as UTF-8, in any version. + public static bool NeedsUpgrade(ReadOnlySpan utf8Json) => VersionOf(utf8Json) < WireVersion; + /// The envelope properties of wire version 1. private static readonly string[] EnvelopeProperties = ["t", "p", "r", "v"]; diff --git a/MarkupString/PublicAPI.Shipped.txt b/MarkupString/PublicAPI.Shipped.txt index 5dac760..7096222 100644 --- a/MarkupString/PublicAPI.Shipped.txt +++ b/MarkupString/PublicAPI.Shipped.txt @@ -1700,3 +1700,7 @@ static MarkupString.MarkupTextSerializer.Inspect(System.ReadOnlySpan utf8J static MarkupString.MarkupTextSerializer.Inspect(System.ReadOnlySpan utf8Json, MarkupString.MarkupRegistry? registry) -> MarkupString.MarkupReadResult! static MarkupString.MarkupTextSerializer.Inspect(string! json) -> MarkupString.MarkupReadResult! static MarkupString.MarkupTextSerializer.Inspect(string! json, MarkupString.MarkupRegistry? registry) -> MarkupString.MarkupReadResult! +static MarkupString.MarkupTextSerializer.NeedsUpgrade(System.ReadOnlySpan utf8Json) -> bool +static MarkupString.MarkupTextSerializer.NeedsUpgrade(string! json) -> bool +static MarkupString.MarkupTextSerializer.VersionOf(System.ReadOnlySpan utf8Json) -> int? +static MarkupString.MarkupTextSerializer.VersionOf(string! json) -> int? diff --git a/docs/serialization.md b/docs/serialization.md index 3181a89..4c996d0 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -92,6 +92,33 @@ The wire version is separate from the package version: a release that adds a mar property does not change it. It changes only if the envelope itself changes in a way a version 1 reader would misread, and a reader that knows the new version keeps reading version 1. +## Upgrading stored text + +`MarkupTextSerializer.VersionOf(stored)` reports the wire version a stored text was written in +without parsing the rest of it: 1 when there is no `"v"`, null when the payload is not a JSON object +or its `"v"` is not a whole number. `NeedsUpgrade(stored)` is true when that version is earlier than +`WireVersion`. Version 1 is the only version so far, so it is false for every row there is. + +Both are there so that a store can find what needs upgrading, at startup or in a scan, before a +later version exists. When one does, it comes with one upgrade step from the version before it, +and the steps run in order, so a world from any earlier version reaches the current one without +a step for every pair of versions. Each step keeps to these rules: + +- **It works on the stored JSON**, not through `MarkupText`, so that nothing the object model does + not hold is lost on the way. +- **It is idempotent.** A payload already in the step's target version comes back byte for byte, + so a migration can stop and resume, or run twice, without harm. +- **It changes nothing it cannot read.** A payload in a newer version, or one that is damaged, is + reported and left as it is. +- **It keeps `"t"`**, the plain text of the whole document, as every version does. +- **It ships with fixtures** written by the release before it, checked from the old version to the + new one and read back. + +Running the steps over a world is the store's work, not this package's: taking a backup first, +working in batches with a checkpoint, reporting what it skipped, and recording which version a +world needs before an older build opens it. Readers keep reading every earlier version all the +same, since backups, messages in flight and plugins never pass through a migration. + ## Reading stored text for saving: `Inspect` `Deserialize` is for showing text. It recovers what it can from a damaged or partly understood row From a85fe59c0e39ad8fc7e64a3186c0acc392bcec9b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 18:54:32 +0000 Subject: [PATCH 5/5] Count a value written back as another kind as lost WireShape let an array come back as any scalar and ignored scalar kind changes, so a sound source of [1,2] read as "" or a pane name of 5 read as "" still counted as rewritable. A value now has to keep its JSON kind (true and false count as one); the only array allowed to come back as a scalar is a legacy ANSI colour (f, g, fs) the codec wrote as a colour. Refs #61 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BxoP4iqJzh4jXnBsnkAFt --- MarkupString.Tests/WireCompatibilityTests.cs | 14 +++++ MarkupString/MarkupTextSerializer.cs | 2 +- MarkupString/WireShape.cs | 65 +++++++++++++------- docs/serialization.md | 10 +-- 4 files changed, 65 insertions(+), 26 deletions(-) diff --git a/MarkupString.Tests/WireCompatibilityTests.cs b/MarkupString.Tests/WireCompatibilityTests.cs index 0461565..f2ee80a 100644 --- a/MarkupString.Tests/WireCompatibilityTests.cs +++ b/MarkupString.Tests/WireCompatibilityTests.cs @@ -145,6 +145,20 @@ public async Task KnownKind_WithAPropertyItDoesNotKnow_IsUnsupported() await Assert.That(result.Diagnostics.Single()).Contains("/future is dropped"); } + /// + /// A value a codec cannot read is written back as something else: an array where a string + /// belongs, a number where a name belongs. Only the ANSI codec's legacy colour arrays are written + /// back as a scalar on purpose. + /// + [Test] + [Arguments("""{"t":"x","p":[null,[{"k":"sound","s":[1,2]}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"pane","n":5}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"html","h":"b","a":7}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"ansi","lt":[1]}]],"r":[1,1]}""")] + [Arguments("""{"t":"x","p":[null,[{"k":"ansi","f":[99,99]}]],"r":[1,1]}""")] + public async Task KnownKind_WithAValueOfTheWrongKind_IsUnsupported(string json) + => await Assert.That(MarkupTextSerializer.Inspect(json, Registry).Status).IsEqualTo(MarkupReadStatus.Unsupported); + [Test] public async Task Neutral_WithAPropertyItDoesNotKnow_IsUnsupported() => await Assert.That(MarkupTextSerializer.Inspect("""{"t":"x","p":[null,[{"k":"neutral","future":1}]],"r":[1,1]}""", Registry).Status) diff --git a/MarkupString/MarkupTextSerializer.cs b/MarkupString/MarkupTextSerializer.cs index 7c5f784..cac315b 100644 --- a/MarkupString/MarkupTextSerializer.cs +++ b/MarkupString/MarkupTextSerializer.cs @@ -499,7 +499,7 @@ private static IMarkup ReadMarkup(JsonElement element, MarkupRegistry? registry, using var rewritten = JsonDocument.Parse(written); // A legacy neutral object is recognised by its "n", which the neutral kind does not write. var ignore = kind == NeutralKind && !element.TryGetProperty("k", out _) ? "n" : null; - if (WireShape.Lost(element, rewritten.RootElement, ignore) is { } path) + if (WireShape.Lost(element, rewritten.RootElement, ignore, kind) is { } path) check.Loss($"A \"{kind}\" layer would not be written back as read: {path}."); return markup; } diff --git a/MarkupString/WireShape.cs b/MarkupString/WireShape.cs index f800bc4..45c5374 100644 --- a/MarkupString/WireShape.cs +++ b/MarkupString/WireShape.cs @@ -6,42 +6,62 @@ namespace MarkupString; /// would leave out. /// /// -/// A codec may write a value in a different form from the one it read — a legacy colour array as -/// a palette index, "bo":0 as "bo":1 — so scalar values are not compared, and an array -/// may come back as a scalar. What may not change is the structure: every property read must be -/// written, an object must stay an object, an array that stays an array must keep its length, and -/// the discriminators ("k" for a markup, "t" for a block or a text) must keep their value. +/// A codec may write a value in a different form from the one it read — "bo":0 as +/// "bo":1, a legacy "#rrggbbaa" as "#rrggbb" — so scalar values are not compared, +/// only their JSON kind. What may not change is the structure: every property read must be written, +/// a value keeps its kind (an object stays an object, an array keeps its length, a number stays a +/// number), and the discriminators ("k" for a markup, "t" for a block or a text) keep +/// their value. The one exception is the ANSI codec's legacy colour arrays ("f":[31]), which +/// it writes back as the palette index or hex colour they mean; a colour it cannot read is not +/// written at all, which counts as dropped. /// internal static class WireShape { + /// The ANSI properties whose legacy SGR arrays are written back as a single colour. + private static readonly string[] AnsiColorProperties = ["f", "g", "fs"]; + /// /// Where leaves out part of , as a path from the /// object's root, or null when it keeps all of it. names one top-level - /// property that is allowed to go. + /// property that is allowed to go; is the layer's kind. /// - public static string? Lost(JsonElement read, JsonElement written, string? ignore) => Lost(read, written, ignore, ""); + public static string? Lost(JsonElement read, JsonElement written, string? ignore, string kind) + { + if (read.ValueKind != JsonValueKind.Object || written.ValueKind != JsonValueKind.Object) return Lost(read, written, ""); + foreach (var property in read.EnumerateObject()) + { + if (property.Name == ignore) continue; + if (kind == "ansi" && property.Value.ValueKind == JsonValueKind.Array && Array.IndexOf(AnsiColorProperties, property.Name) >= 0 + && written.TryGetProperty(property.Name, out var colour) && colour.ValueKind is JsonValueKind.Number or JsonValueKind.String) + continue; + if (Property(property, written, "") is { } lost) return lost; + } + return null; + } - private static string? Lost(JsonElement read, JsonElement written, string? ignore, string path) + private static string? Property(JsonProperty property, JsonElement written, string path) { + var at = $"{path}/{property.Name}"; + if (!written.TryGetProperty(property.Name, out var value)) return $"{at} is dropped"; + if (property.Name is "k" or "t" && property.Value.ValueKind == JsonValueKind.String + && (value.ValueKind != JsonValueKind.String || value.GetString() != property.Value.GetString())) + return $"{at} changes from {property.Value.GetRawText()} to {value.GetRawText()}"; + return Lost(property.Value, value, at); + } + + private static string? Lost(JsonElement read, JsonElement written, string path) + { + if (Kind(read.ValueKind) != Kind(written.ValueKind)) + return $"{Where(path)} changes from {read.GetRawText()} to {written.GetRawText()}"; + switch (read.ValueKind) { case JsonValueKind.Object: - if (written.ValueKind != JsonValueKind.Object) return $"{Where(path)} is no longer an object"; foreach (var property in read.EnumerateObject()) - { - if (property.Name == ignore) continue; - var at = $"{path}/{property.Name}"; - if (!written.TryGetProperty(property.Name, out var value)) return $"{at} is dropped"; - if (property.Name is "k" or "t" && property.Value.ValueKind == JsonValueKind.String - && (value.ValueKind != JsonValueKind.String || value.GetString() != property.Value.GetString())) - return $"{at} changes from {property.Value.GetRawText()} to {value.GetRawText()}"; - if (Lost(property.Value, value, ignore: null, at) is { } lost) return lost; - } + if (Property(property, written, path) is { } lost) return lost; return null; case JsonValueKind.Array: - if (written.ValueKind == JsonValueKind.Object) return $"{Where(path)} is no longer an array"; - if (written.ValueKind != JsonValueKind.Array) return null; var count = read.GetArrayLength(); if (written.GetArrayLength() != count) return $"{Where(path)} has {count} entries and would be written with {written.GetArrayLength()}"; var index = 0; @@ -49,7 +69,7 @@ internal static class WireShape using (var right = written.EnumerateArray()) while (left.MoveNext() && right.MoveNext()) { - if (Lost(left.Current, right.Current, ignore: null, $"{path}/{index}") is { } lost) return lost; + if (Lost(left.Current, right.Current, $"{path}/{index}") is { } lost) return lost; index++; } return null; @@ -59,5 +79,8 @@ internal static class WireShape } } + /// A value's kind, with and as one. + private static JsonValueKind Kind(JsonValueKind kind) => kind == JsonValueKind.False ? JsonValueKind.True : kind; + private static string Where(string path) => path.Length == 0 ? "the object" : path; } diff --git a/docs/serialization.md b/docs/serialization.md index 4c996d0..5fad76c 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -149,10 +149,12 @@ kept as an `UnknownMarkup` holding the raw object, renders as nothing and writes and `Inspect` reports the result as `Damaged`. To find what a codec would leave out, `Inspect` writes each layer it read straight back and -compares the two. A codec may write a value in a newer form than the one it read, such as a legacy -colour array as a palette index, so values are not compared. Structure is compared: every property -read must be written, an object must stay an object, an array must keep its length, and the -`"k"` and `"t"` discriminators must keep their values. This catches a game's own codecs as well as +compares the two. A codec may write a value in a newer form than the one it read, such as +`"bo":0` as `"bo":1`, so values are not compared, only their kind. Every property read must be +written, each value must keep its kind (an object stays an object, an array keeps its length, a +number stays a number), and the `"k"` and `"t"` discriminators must keep their values. The one +exception is the ANSI codec's legacy colour arrays, such as `"f":[31]`, which it writes back as the +colour they mean. This catches a game's own codecs as well as the shipped ones, without each codec having to report what it skipped. `Inspect` does more work than `Deserialize`, since it writes every layer a second time. Use it on