From 51b9d430b9704c5d1431bb78545cde133c49ac87 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Tue, 22 Sep 2026 18:58:59 -0400 Subject: [PATCH 1/7] fix(output): render JSON results as JSON data in human output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A handler returning System.Text.Json's JsonObject — or rows of JsonObject through IReplPageSource — got its CLR members in human output: Options, Parent, Root, Count, or ValueKind for a JsonElement. A JsonObject enumerates as key/value pairs, so it went down the collection path and its values down reflection. The Spectre renderer did worse: its inline path recursed through JsonNode's Root and Parent, which point back at each other, and overflowed the stack, taking the process with it. JsonHumanShape (Repl.Core, shared by both renderers) recognizes JsonNode and JsonElement and writes compact JSON literals. Each renderer checks for it before its enumerable and reflection branches: - an object becomes key: value lines; - rows of objects become a table whose columns are the union of their keys in first-seen order, with a missing key leaving an empty cell; - an array of scalars becomes one literal per line, JSON nulls included; - any JSON value nested in an ordinary result, or passed as a result's details, shows as a compact literal. The literal is written with Utf8JsonWriter and no serializer options, so a JsonValue wrapping a CLR object keeps its own type info. Options without a type resolver made that throw, where --json succeeds. The relaxed encoder keeps non-ASCII readable and escapes every control character, and format characters (bidirectional overrides, isolates, zero-width marks), which it lets through, are escaped afterwards, so a payload can neither drive the terminal nor disguise what it shows. The JSON row scan only runs once a collection's first item is JSON, so ordinary collections do not pay for it. docs/output-system.md gains a "JSON results" section. TDD: the human tests were red first showing the CLR members, and the Spectre ones crashed with a stack overflow. Review then found four more gaps, each with a test red before its fix: a JsonValue wrapping a CLR object threw, an all-null array rendered empty "- " lines, JSON details showed CLR members, and bidi characters passed raw. Each fix was then falsified with a compiling substitution that turned its own test red. Refs #92 --- docs/output-system.md | 21 ++ .../Output/HumanOutputTransformer.cs | 139 +++++++++--- src/Repl.Core/Output/JsonHumanShape.cs | 209 ++++++++++++++++++ .../SpectreHumanOutputTransformer.cs | 69 ++++++ .../Given_SpectreHumanOutputJson.cs | 159 +++++++++++++ src/Repl.Tests/Given_HumanOutputJson.cs | 202 +++++++++++++++++ 6 files changed, 772 insertions(+), 27 deletions(-) create mode 100644 src/Repl.Core/Output/JsonHumanShape.cs create mode 100644 src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs create mode 100644 src/Repl.Tests/Given_HumanOutputJson.cs diff --git a/docs/output-system.md b/docs/output-system.md index c267efd4..edb0e271 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -23,6 +23,27 @@ The active output format is resolved in this order: | `yaml` | YAML serialization. | | `markdown` | Markdown table/document rendering. | +### JSON results + +A handler can return `System.Text.Json.Nodes.JsonNode` (`JsonObject`, `JsonArray`, `JsonValue`) or a +`JsonElement`, including as rows of an `IReplPageSource`. `human` and `spectre` then render the JSON +data rather than the CLR members of those types: + +- An object becomes one `key: value` line per field. +- Rows of objects become a table. Its columns are the union of the rows' keys in first-seen order, and a + key missing from a row leaves that cell empty. +- An array of scalars becomes one value per line. +- Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and + a nested object or array shows as itself. +- A JSON value held by a property of an ordinary result object, or passed as a result's details, shows as + a compact literal too. + +In JSON strings and keys, control characters and Unicode format characters (such as bidirectional +overrides) are escaped. So a payload can neither drive the terminal nor make it display something other +than the data. Non-ASCII text stays readable. This covers JSON values only: a plain CLR `string` in a +result is still written as-is. The `json` format and MCP output are unchanged, and `markdown` does not +special-case JSON yet. + ### Format aliases The built-in aliases are: diff --git a/src/Repl.Core/Output/HumanOutputTransformer.cs b/src/Repl.Core/Output/HumanOutputTransformer.cs index 63340103..c000b67b 100644 --- a/src/Repl.Core/Output/HumanOutputTransformer.cs +++ b/src/Repl.Core/Output/HumanOutputTransformer.cs @@ -3,6 +3,7 @@ using System.ComponentModel.DataAnnotations; using System.Reflection; using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Nodes; namespace Repl; @@ -52,32 +53,16 @@ public ValueTask TransformAsync(object? value, CancellationToken cancell return ValueTask.FromResult(text); } - if (value is System.Collections.IEnumerable enumerable) + // Before the enumerable branch: a JsonObject enumerates as key/value pairs, and reflecting over + // those would show JsonNode's CLR members instead of the data. + if (JsonHumanShape.TryGetNode(value, out var node)) { - var lines = enumerable - .Cast() - .ToArray(); - if (lines.Length == 0) - { - return ValueTask.FromResult("No results."); - } - - if (TryRenderTable(lines, settings, includeHeader: true, out var tableText)) - { - return ValueTask.FromResult(tableText); - } - - var scalarLines = lines - .Select(item => RenderScalar(item, member: null, depth: 0, compactCollection: false, settings.Width, settings)) - .Where(item => !string.IsNullOrWhiteSpace(item)) - .ToArray(); - - if (scalarLines.Length == 0) - { - return ValueTask.FromResult("No results."); - } + return ValueTask.FromResult(RenderJson(node, settings)); + } - return ValueTask.FromResult(string.Join(Environment.NewLine, scalarLines)); + if (value is System.Collections.IEnumerable enumerable) + { + return ValueTask.FromResult(RenderTopLevelEnumerable(enumerable, settings)); } if (TryRenderObject(value, settings, out var objectText)) @@ -124,6 +109,79 @@ private static string RenderPage( : string.Concat(body, Environment.NewLine, footer); } + private static string RenderTopLevelEnumerable(System.Collections.IEnumerable enumerable, HumanRenderSettings settings) + { + var lines = enumerable + .Cast() + .ToArray(); + if (lines.Length == 0) + { + return "No results."; + } + + if (TryRenderTable(lines, settings, includeHeader: true, out var tableText)) + { + return tableText; + } + + var scalarLines = lines + .Select(item => RenderScalar(item, member: null, depth: 0, compactCollection: false, settings.Width, settings)) + .Where(item => !string.IsNullOrWhiteSpace(item)) + .ToArray(); + + return scalarLines.Length == 0 ? "No results." : string.Join(Environment.NewLine, scalarLines); + } + + private static string RenderJson(JsonNode? node, HumanRenderSettings settings) => node switch + { + JsonObject jsonObject => RenderJsonObject(jsonObject, settings), + JsonArray { Count: 0 } => "No results.", + // Its own path rather than the generic collection one, which recognizes JSON by its first non-null + // item and so has nothing to go on for an array of nulls. + JsonArray jsonArray => RenderJsonItems([.. jsonArray], settings, includeHeader: true), + _ => JsonHumanShape.Literal(node), + }; + + private static string RenderJsonItems(object?[] items, HumanRenderSettings settings, bool includeHeader) + { + if (!JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows)) + { + return string.Join( + Environment.NewLine, + items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) + ? literal + : RenderScalar(item, member: null, depth: 0, compactCollection: true, settings.Width, settings))); + } + + var tableRows = new List(rows.Length + (includeHeader ? 1 : 0)); + if (includeHeader) + { + tableRows.Add([.. columns.Select(JsonHumanShape.Label)]); + } + + tableRows.AddRange(rows.Select(row => columns.Select(column => JsonHumanShape.Cell(row, column)).ToArray())); + return FormatTable(tableRows, settings, includeHeader); + } + + private static string RenderJsonObject(JsonObject jsonObject, HumanRenderSettings settings) + { + if (jsonObject.Count == 0) + { + return "{}"; + } + + var entries = new List(jsonObject.Count); + foreach (var property in jsonObject) + { + entries.Add(new RenderedEntry( + JsonHumanShape.Label(property.Key), + JsonHumanShape.Literal(property.Value), + IsMultiline: false)); + } + + return RenderEntries(entries, settings); + } + private static bool TryRenderObject(object value, HumanRenderSettings settings, out string text) { var members = GetDisplayMembers(value.GetType()); @@ -137,6 +195,12 @@ private static bool TryRenderObject(object value, HumanRenderSettings settings, foreach (var member in members) { var memberValue = member.Property.GetValue(value); + if (JsonHumanShape.TryGetNode(memberValue, out var jsonValue)) + { + entries.Add(new RenderedEntry(member.Label, JsonHumanShape.Literal(jsonValue), IsMultiline: false)); + continue; + } + if (memberValue is System.Collections.IEnumerable collectionValue && memberValue is not string) { @@ -205,6 +269,13 @@ private static bool TryRenderTable( return false; } + // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. + if (JsonHumanShape.IsJson(firstNonNull)) + { + text = RenderJsonItems(values, settings, includeHeader); + return true; + } + if (IsSimpleValue(firstNonNull.GetType())) { text = string.Join( @@ -220,16 +291,20 @@ private static bool TryRenderTable( return false; } - var rows = BuildTableRows(values, members, settings, includeHeader); + text = FormatTable(BuildTableRows(values, members, settings, includeHeader), settings, includeHeader); + return true; + } + + private static string FormatTable(List rows, HumanRenderSettings settings, bool includeHeader) + { var style = includeHeader && settings.UseAnsi ? TextTableStyle.ForHeader(settings.Palette.TableHeaderStyle) : TextTableStyle.None; - text = TextTableFormatter.FormatRows( + return TextTableFormatter.FormatRows( rows, settings.Width, includeHeaderSeparator: includeHeader && !settings.UseAnsi, style); - return true; } private static List BuildTableRows( @@ -312,6 +387,11 @@ private static string RenderScalar( return text; } + if (JsonHumanShape.TryGetNode(value, out var node)) + { + return JsonHumanShape.Literal(node); + } + var valueType = value.GetType(); if (IsSimpleValue(valueType)) { @@ -426,6 +506,11 @@ private static string RenderReplResult(IReplResult result, HumanRenderSettings s return $"{message}{Environment.NewLine}{RenderPage(page, settings)}"; } + if (JsonHumanShape.TryGetNode(result.Details, out var jsonDetails)) + { + return $"{message}{Environment.NewLine}{RenderJson(jsonDetails, settings)}"; + } + if (TryRenderDictionary(result.Details, settings, out var dictionaryText)) { return $"{message}{Environment.NewLine}{dictionaryText}"; diff --git a/src/Repl.Core/Output/JsonHumanShape.cs b/src/Repl.Core/Output/JsonHumanShape.cs new file mode 100644 index 00000000..62040fc9 --- /dev/null +++ b/src/Repl.Core/Output/JsonHumanShape.cs @@ -0,0 +1,209 @@ +using System.Buffers; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Text; +using System.Text.Encodings.Web; +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace Repl; + +/// +/// Recognizes JSON data — and — for the human renderers, +/// which would otherwise reflect over the CLR members of those types (Options, Parent, +/// Root, Count, ValueKind) instead of showing the data. +/// +/// +/// Values render as compact JSON literals, so a string reads "x", an explicit JSON null reads +/// null, and a nested object or array stays visible as itself. The relaxed encoder keeps non-ASCII +/// text readable and escapes every control character; format characters (bidirectional overrides and +/// isolates, zero-width marks), which it lets through, are escaped here too, so a JSON string can neither +/// drive the terminal it is printed to nor make it display something other than the data. Nothing here +/// mutates the value it reads. +/// +internal static class JsonHumanShape +{ + private static readonly JsonWriterOptions LiteralWriterOptions = new() + { + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, + }; + + public static bool IsJson([NotNullWhen(true)] object? value) => value is JsonNode or JsonElement; + + /// + /// Reads as a JSON node. A is wrapped, and the element + /// itself is never modified. A JSON null comes back as a node with + /// . + /// + public static bool TryGetNode(object? value, out JsonNode? node) + { + switch (value) + { + case JsonNode jsonNode: + node = jsonNode; + return true; + case JsonElement element: + node = element.ValueKind switch + { + JsonValueKind.Object => JsonObject.Create(element), + JsonValueKind.Array => JsonArray.Create(element), + JsonValueKind.Null or JsonValueKind.Undefined => null, + _ => JsonValue.Create(element), + }; + return true; + default: + node = null; + return false; + } + } + + /// + /// The literal for an item of a JSON collection: — a JSON null — reads + /// null. for a value that is not JSON at all. + /// + public static bool TryLiteral(object? value, [NotNullWhen(true)] out string? literal) + { + if (value is null) + { + literal = Literal(node: null); + return true; + } + + if (TryGetNode(value, out var node)) + { + literal = Literal(node); + return true; + } + + literal = null; + return false; + } + + /// + /// The compact JSON text of . Written with no serializer options, so a + /// wrapping a CLR object keeps the type information it was created with; + /// options without a type resolver make that write throw. + /// + public static string Literal(JsonNode? node) + { + if (node is null) + { + return "null"; + } + + var buffer = new ArrayBufferWriter(); + // Synchronous on purpose: the writer targets an in-memory buffer, so disposing it only flushes there. +#pragma warning disable MA0045 + using (var writer = new Utf8JsonWriter(buffer, LiteralWriterOptions)) +#pragma warning restore MA0045 + { + node.WriteTo(writer, options: null); + } + + return EscapeFormatCharacters(Encoding.UTF8.GetString(buffer.WrittenSpan)); + } + + /// A property name as a label: escaped like a JSON string, without the quotes. + public static string Label(string key) => + EscapeFormatCharacters(JsonEncodedText.Encode(key, JavaScriptEncoder.UnsafeRelaxedJsonEscaping).ToString()); + + /// + /// Reads as rows of JSON objects. The columns are the union of their keys in + /// first-seen order, so a key missing from one row leaves its cell empty rather than dropping the row. + /// A value is an empty row. Fails if any other value is not a JSON object. + /// + public static bool TryGetObjectRows( + IReadOnlyList values, + [NotNullWhen(true)] out string[]? columns, + [NotNullWhen(true)] out JsonObject?[]? rows) + { + columns = null; + rows = null; + var converted = new JsonObject?[values.Count]; + var seen = new HashSet(StringComparer.Ordinal); + var ordered = new List(); + var anyRow = false; + for (var i = 0; i < values.Count; i++) + { + if (values[i] is null) + { + continue; + } + + if (!TryGetNode(values[i], out var node) || node is not JsonObject row) + { + return false; + } + + anyRow = true; + converted[i] = row; + foreach (var property in row) + { + if (seen.Add(property.Key)) + { + ordered.Add(property.Key); + } + } + } + + if (!anyRow) + { + return false; + } + + columns = [.. ordered]; + rows = converted; + return true; + } + + /// The cell for : empty when the row does not have the key. + public static string Cell(JsonObject? row, string column) => + row is not null && row.TryGetPropertyValue(column, out var value) ? Literal(value) : string.Empty; + + // Format characters can only appear inside a JSON string here, where \uXXXX is the same character + // to a JSON reader. Returns the input unchanged — no allocation — when it has none. + private static string EscapeFormatCharacters(string text) + { + var index = IndexOfFormatCharacter(text); + if (index < 0) + { + return text; + } + + var builder = new StringBuilder(text.Length + 12); + builder.Append(text, 0, index); + Span units = stackalloc char[2]; + foreach (var rune in text.AsSpan(index).EnumerateRunes()) + { + var written = rune.EncodeToUtf16(units); + if (Rune.GetUnicodeCategory(rune) != UnicodeCategory.Format) + { + builder.Append(units[..written]); + continue; + } + + for (var i = 0; i < written; i++) + { + builder.Append(CultureInfo.InvariantCulture, $"\\u{(int)units[i]:X4}"); + } + } + + return builder.ToString(); + } + + private static int IndexOfFormatCharacter(string text) + { + var index = 0; + foreach (var rune in text.EnumerateRunes()) + { + if (Rune.GetUnicodeCategory(rune) == UnicodeCategory.Format) + { + return index; + } + + index += rune.Utf16SequenceLength; + } + + return -1; + } +} diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index 81e73e8f..7b1d9a4d 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -5,6 +5,7 @@ using System.Globalization; using System.IO; using System.Reflection; +using System.Text.Json.Nodes; namespace Repl.Spectre; @@ -58,6 +59,9 @@ public ValueTask TransformAsync(object? value, CancellationToken cancell IReplPage page => RenderPage(page), IReplResult replResult => RenderReplResult(replResult), string text => text, + // Before the enumerable arm: a JsonObject enumerates as key/value pairs, and reflecting over + // those walks JsonNode's Root and Parent, which point back at each other. + _ when JsonHumanShape.TryGetNode(value, out var node) => RenderJson(node), System.Collections.IEnumerable enumerable => RenderEnumerable(enumerable), _ when TryRenderObject(value, out var objectText) => objectText, _ => Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty, @@ -215,6 +219,12 @@ private string RenderEnumerable( return "No results."; } + // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. + if (JsonHumanShape.IsJson(firstNonNull)) + { + return RenderJsonItems(items, includeTableHeader); + } + if (IsSimpleValue(firstNonNull.GetType())) { return string.Join( @@ -272,6 +282,55 @@ private static string RenderPageFooter(IReplPage page) return $"Showing {count.ToString(CultureInfo.InvariantCulture)} result(s). Next data page: rerun with {ResultFlowCursorPolicy.FormatCliContinuation(info.NextCursor)}."; } + private string RenderJson(JsonNode? node) => node switch + { + JsonObject { Count: 0 } => "{}", + JsonObject jsonObject => RenderToString(BuildLabelValueGrid( + [.. jsonObject.Select(static property => + (JsonHumanShape.Label(property.Key), JsonHumanShape.Literal(property.Value))),])), + JsonArray { Count: 0 } => "No results.", + // Its own path rather than RenderEnumerable, which recognizes JSON by its first non-null item and so + // has nothing to go on for an array of nulls. + JsonArray jsonArray => RenderJsonItems([.. jsonArray], includeHeader: true), + _ => JsonHumanShape.Literal(node), + }; + + private string RenderJsonItems(object?[] items, bool includeHeader) => + JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows) + ? RenderToString(BuildJsonTable(columns, rows, includeHeader)) + : string.Join( + Environment.NewLine, + items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) ? literal : RenderInlineValue(item))); + + private static Table BuildJsonTable(string[] columns, JsonObject?[] rows, bool includeHeaders) + { + var table = new Table() + .Border(TableBorder.None) + .Collapse(); + if (!includeHeaders) + { + table.HideHeaders(); + } + + foreach (var column in columns) + { + table.AddColumn(new TableColumn($"[bold]{Markup.Escape(JsonHumanShape.Label(column))}[/]")); + } + + foreach (var row in rows) + { + var cells = new IRenderable[columns.Length]; + for (var i = 0; i < columns.Length; i++) + { + cells[i] = CreateTableCell(JsonHumanShape.Cell(row, columns[i]), table.Rows.Count, i); + } + + table.AddRow(cells); + } + + return table; + } + private bool TryRenderObject(object value, out string text) { var members = GetDisplayMembers(value.GetType()); @@ -425,6 +484,11 @@ private IRenderable RenderValueRenderable( return new Text(text); } + if (JsonHumanShape.TryGetNode(value, out var node)) + { + return new Text(JsonHumanShape.Literal(node)); + } + if (value is System.Collections.IEnumerable enumerable) { var lines = RenderNestedEnumerableLines(enumerable); @@ -461,6 +525,11 @@ private static string RenderInlineValue(object? value, DisplayMember? member = n return text; } + if (JsonHumanShape.TryGetNode(value, out var node)) + { + return JsonHumanShape.Literal(node); + } + if (value is System.Collections.IEnumerable enumerable) { var lines = RenderNestedEnumerableLines(enumerable); diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs new file mode 100644 index 00000000..64faeeaf --- /dev/null +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -0,0 +1,159 @@ +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.RegularExpressions; + +namespace Repl.SpectreTests; + +/// +/// The Spectre human transformer renders and results as +/// JSON data, the same way the default human transformer does, rather than reflecting over their CLR +/// members. +/// +[TestClass] +public sealed partial class Given_SpectreHumanOutputJson +{ + private static readonly string[] ClrMembers = ["Options", "Parent", "Root", "Count", "ValueKind"]; + + [TestMethod] + [Description("A JsonObject result renders one row per JSON field, values as JSON literals, and none of JsonObject's CLR members.")] + public async Task When_AJsonObjectIsRendered_Then_ItsFieldsAreShown() + { + var output = await RenderAsync(new JsonObject { ["id"] = 1, ["name"] = "Example", ["active"] = true }).ConfigureAwait(false); + + output.Should().MatchRegex(@"id:\s+1"); + output.Should().MatchRegex(@"name:\s+""Example"""); + output.Should().MatchRegex(@"active:\s+true"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Paged JsonObject rows render as a table whose columns are the rows' JSON keys, with a key missing from one row left empty.")] + public async Task When_APageOfJsonObjectsIsRendered_Then_TheKeysBecomeColumns() + { + var page = new ReplPage( + [ + new JsonObject { ["id"] = 1, ["name"] = "first" }, + new JsonObject { ["id"] = 2, ["name"] = "second", ["extra"] = "only-here" }, + ], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: 2, PageSize: 2)); + + var output = await RenderAsync(page).ConfigureAwait(false); + + output.Should().MatchRegex(@"id\s+name\s+extra"); + output.Should().Contain("\"first\"").And.Contain("\"second\"").And.Contain("\"only-here\""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Nested objects and arrays stay visible as compact JSON, and an explicit JSON null reads as null.")] + public async Task When_AJsonObjectHasNestedValuesAndNulls_Then_TheyArePreserved() + { + var output = await RenderAsync(new JsonObject + { + ["owner"] = new JsonObject { ["login"] = "octo" }, + ["tags"] = new JsonArray("a", "b"), + ["deleted"] = null, + }).ConfigureAwait(false); + + output.Should().Contain("""{"login":"octo"}"""); + output.Should().Contain("""["a","b"]"""); + output.Should().MatchRegex(@"deleted:\s+null"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("A JsonElement is JSON data too, and renders the same way as the equivalent JsonNode.")] + public async Task When_AJsonElementIsRendered_Then_ItsFieldsAreShown() + { + using var document = JsonDocument.Parse("""{"id":7,"name":"from-element"}"""); + + var output = await RenderAsync(document.RootElement).ConfigureAwait(false); + + output.Should().MatchRegex(@"id:\s+7"); + output.Should().MatchRegex(@"name:\s+""from-element"""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Control characters in a JSON string, such as an ANSI escape, must reach the terminal escaped, not raw.")] + public async Task When_AJsonStringCarriesControlCharacters_Then_TheyAreEscaped() + { + var output = await RenderRawAsync(new JsonObject { ["note"] = "red\u001b[31malert" }).ConfigureAwait(false); + + output.Should().NotContain("\u001b[31m", "a raw escape would let the payload drive the operator's terminal"); + output.Should().Contain(@"\u001B[31m"); + } + + [TestMethod] + [Description("Rendering reads the payload; it must not change it.")] + public async Task When_AJsonObjectIsRendered_Then_ThePayloadIsUnchanged() + { + var payload = new JsonObject { ["id"] = 1, ["nested"] = new JsonObject { ["x"] = 2 } }; + var before = payload.ToJsonString(); + + await RenderAsync(payload).ConfigureAwait(false); + + payload.ToJsonString().Should().Be(before); + } + + [TestMethod] + [Description("A JsonValue can wrap an arbitrary CLR object. Rendering it must not throw where --json succeeds.")] + public async Task When_AJsonValueWrapsAClrObject_Then_ItRenders() + { + var output = await RenderAsync(new JsonObject { ["owner"] = JsonValue.Create(new Owner("octo")) }).ConfigureAwait(false); + + output.Should().Contain("""{"Login":"octo"}"""); + } + + [TestMethod] + [Description("A JSON array whose items are all null still renders each as null: there is no first value to recognize it by, so it cannot go through the generic collection path.")] + public async Task When_AJsonArrayOfNullsIsRendered_Then_EachItemReadsNull() + { + var output = await RenderAsync(new JsonArray(null, null)).ConfigureAwait(false); + + output.Split(Environment.NewLine).Should().Equal("null", "null"); + } + + [TestMethod] + [Description("A JsonNode held by an ordinary result object's property renders as a compact JSON literal, not as that node's CLR members.")] + public async Task When_AnObjectHasAJsonProperty_Then_ItRendersAsALiteral() + { + var output = await RenderAsync(new Holder("h1", new JsonObject { ["x"] = 1 })).ConfigureAwait(false); + + output.Should().Contain("""{"x":1}"""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Bidirectional format characters are escaped too: left raw they reorder what the terminal displays.")] + public async Task When_AJsonStringCarriesBidiControls_Then_TheyAreEscaped() + { + var output = await RenderRawAsync(new JsonObject { ["file\u202Egpj"] = "admin\u202Eexe.txt" }).ConfigureAwait(false); + + output.Should().NotContain("\u202E", "a raw override would reorder what the terminal displays"); + output.Should().Contain(@"\u202E"); + } + + private sealed record Holder(string Name, JsonObject Payload); + + private sealed record Owner(string Login); + + // Spectre bolds labels with ANSI; the assertions are about the data, so that styling is stripped. The + // control-character test reads the raw output instead, since what reaches the terminal is its point. + private static async Task RenderAsync(object value) => + AnsiStyling().Replace(await RenderRawAsync(value).ConfigureAwait(false), string.Empty); + + private static async Task RenderRawAsync(object value) => + await new SpectreHumanOutputTransformer().TransformAsync(value, CancellationToken.None).ConfigureAwait(false); + + [GeneratedRegex(@"\u001b\[[0-9;]*m", RegexOptions.CultureInvariant, matchTimeoutMilliseconds: 1000)] + private static partial Regex AnsiStyling(); + + private static void AssertNoClrMembers(string output) + { + foreach (var member in ClrMembers) + { + output.Should().NotContain(member, "JSON data must not render the CLR members of its container type"); + } + } +} diff --git a/src/Repl.Tests/Given_HumanOutputJson.cs b/src/Repl.Tests/Given_HumanOutputJson.cs new file mode 100644 index 00000000..f0cc3f88 --- /dev/null +++ b/src/Repl.Tests/Given_HumanOutputJson.cs @@ -0,0 +1,202 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace Repl.Tests; + +/// +/// A command that returns or is returning JSON data, so +/// human output shows the JSON keys and values — not the CLR members (Options, Parent, +/// Root, Count) that reflection finds on those types. +/// +[TestClass] +public sealed class Given_HumanOutputJson +{ + private static readonly string[] ClrMembers = ["Options", "Parent", "Root", "Count", "ValueKind"]; + + [TestMethod] + [Description("The issue's own repro: a JsonObject result renders one line per JSON field, values as JSON literals, and none of JsonObject's CLR members.")] + public async Task When_AJsonObjectIsRendered_Then_ItsFieldsAreShown() + { + var output = await RenderAsync(new JsonObject { ["id"] = 1, ["name"] = "Example", ["active"] = true }); + + output.Should().MatchRegex(@"id\s*:\s*1"); + output.Should().MatchRegex(@"name\s*:\s*""Example"""); + output.Should().MatchRegex(@"active\s*:\s*true"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Paged JsonObject rows render as a table whose columns are the rows' JSON keys, in first-seen order, with a key missing from one row left empty rather than dropping the row.")] + public async Task When_APageOfJsonObjectsIsRendered_Then_TheKeysBecomeColumns() + { + var page = new ReplPage( + [ + new JsonObject { ["id"] = 1, ["name"] = "first" }, + new JsonObject { ["id"] = 2, ["name"] = "second", ["extra"] = "only-here" }, + ], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: 2, PageSize: 2)); + + var output = await RenderAsync(page); + + var header = output.Split(Environment.NewLine)[0]; + header.Should().MatchRegex(@"id\s+name\s+extra"); + output.Should().Contain("\"first\"").And.Contain("\"second\"").And.Contain("\"only-here\""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Nested objects and arrays stay visible as compact JSON, and an explicit JSON null reads as null rather than as an empty value.")] + public async Task When_AJsonObjectHasNestedValuesAndNulls_Then_TheyArePreserved() + { + var output = await RenderAsync(new JsonObject + { + ["owner"] = new JsonObject { ["login"] = "octo" }, + ["tags"] = new JsonArray("a", "b"), + ["deleted"] = null, + }); + + output.Should().Contain("""{"login":"octo"}"""); + output.Should().Contain("""["a","b"]"""); + output.Should().MatchRegex(@"deleted\s*:\s*null"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("A JsonElement is JSON data too, and renders the same way as the equivalent JsonNode.")] + public async Task When_AJsonElementIsRendered_Then_ItsFieldsAreShown() + { + using var document = JsonDocument.Parse("""{"id":7,"name":"from-element"}"""); + + var output = await RenderAsync(document.RootElement); + + output.Should().MatchRegex(@"id\s*:\s*7"); + output.Should().MatchRegex(@"name\s*:\s*""from-element"""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("A JSON string is untrusted data: control characters in it, such as an ANSI escape, must reach the terminal escaped, not raw. Non-ASCII text stays readable.")] + public async Task When_AJsonStringCarriesControlCharacters_Then_TheyAreEscaped() + { + var output = await RenderAsync(new JsonObject { ["note"] = "red\u001b[31malert", ["city"] = "Montréal" }); + + output.Should().NotContain("\u001b", "a raw escape would let the payload drive the operator's terminal"); + output.Should().Contain("Montréal"); + } + + [TestMethod] + [Description("Rendering reads the payload; it must not change it.")] + public async Task When_AJsonObjectIsRendered_Then_ThePayloadIsUnchanged() + { + var payload = new JsonObject { ["id"] = 1, ["nested"] = new JsonObject { ["x"] = 2 } }; + var before = payload.ToJsonString(); + + await RenderAsync(payload); + + payload.ToJsonString().Should().Be(before); + } + + [TestMethod] + [Description("A key missing from one row leaves that row's cell empty: it must not read null, and the other cells must not shift into it.")] + public async Task When_ARowLacksAKey_Then_ItsCellIsEmpty() + { + var page = new ReplPage( + [ + new JsonObject { ["id"] = 1, ["name"] = "first" }, + new JsonObject { ["id"] = 2, ["name"] = "second", ["extra"] = "only-here" }, + ], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: 2, PageSize: 2)); + + var output = await RenderAsync(page); + + var firstRow = output.Split(Environment.NewLine).Single(line => line.Contains("\"first\"", StringComparison.Ordinal)); + firstRow.TrimEnd().Should().EndWith("\"first\"", "the extra column is empty for the row that does not have it"); + } + + [TestMethod] + [Description("A top-level JSON array of scalars renders one JSON literal per line, and its JSON nulls read as null — also when every item is null, where there is no first value to recognize the array by.")] + public async Task When_AJsonArrayOfScalarsIsRendered_Then_EachItemIsALiteral() + { + (await RenderAsync(new JsonArray(1, "two", null))).Split(Environment.NewLine) + .Should().Equal("1", "\"two\"", "null"); + (await RenderAsync(new JsonArray(null, null))).Split(Environment.NewLine) + .Should().Equal("null", "null"); + } + + [TestMethod] + [Description("A page of JsonElement rows renders like a page of JsonObject rows: the issue names JsonElement explicitly.")] + public async Task When_APageOfJsonElementsIsRendered_Then_TheKeysBecomeColumns() + { + using var document = JsonDocument.Parse("""[{"id":1,"name":"first"},{"id":2,"name":"second"}]"""); + var page = new ReplPage( + [.. document.RootElement.EnumerateArray()], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: 2, PageSize: 2)); + + var output = await RenderAsync(page); + + output.Split(Environment.NewLine)[0].Should().MatchRegex(@"id\s+name"); + output.Should().Contain("\"second\""); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("A JsonNode held by an ordinary result object's property renders as a compact JSON literal, not as that node's CLR members.")] + public async Task When_AnObjectHasAJsonProperty_Then_ItRendersAsALiteral() + { + var output = await RenderAsync(new Holder("h1", new JsonObject { ["x"] = 1 })); + + output.Should().MatchRegex(@"Payload\s*:\s*\{""x"":1\}"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("A JsonValue can wrap an arbitrary CLR object. Rendering it must not throw where --json succeeds: serializing through options that carry no type resolver does exactly that.")] + public async Task When_AJsonValueWrapsAClrObject_Then_ItRenders() + { + var output = await RenderAsync(new JsonObject { ["owner"] = JsonValue.Create(new Owner("octo")) }); + + output.Should().Contain("""{"Login":"octo"}"""); + } + + [TestMethod] + [Description("JSON carried as an IReplResult's details renders as JSON data too, not as JsonNode's CLR members.")] + public async Task When_AResultCarriesJsonDetails_Then_TheFieldsAreShown() + { + var output = await RenderAsync(Results.Success("done", new JsonObject { ["code"] = 42 })); + + output.Should().MatchRegex(@"code\s*:\s*42"); + AssertNoClrMembers(output); + } + + [TestMethod] + [Description("Bidirectional and other format characters (U+202E, U+2066 to U+2069, U+200B and others) are escaped too: left raw they reorder or hide what the terminal shows, so a value could display as something it is not.")] + public async Task When_AJsonStringCarriesBidiControls_Then_TheyAreEscaped() + { + var output = await RenderAsync(new JsonObject { ["file\u202Egpj"] = "admin\u202Eexe.txt" }); + + output.Should().NotContain("\u202E", "a raw override would reorder what the terminal displays"); + output.Should().Contain(@"\u202E"); + } + + private sealed record Holder(string Name, JsonObject Payload); + + private sealed record Owner(string Login); + + private static async Task RenderAsync(object value) + { + var transformer = new HumanOutputTransformer( + () => new HumanRenderSettings( + Width: 120, + UseAnsi: false, + Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + return await transformer.TransformAsync(value, CancellationToken.None).ConfigureAwait(false); + } + + private static void AssertNoClrMembers(string output) + { + foreach (var member in ClrMembers) + { + output.Should().NotContain(member, "JSON data must not render the CLR members of its container type"); + } + } +} From ac5cf1025b07243a59e5df38fdc2a9a486e67841 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Tue, 22 Sep 2026 19:33:50 -0400 Subject: [PATCH 2/7] fix(output): keep JSON continuation headers, empty-object rows, ordinal cells and null pages Addresses the review of #111. A JSON table takes its columns from its rows' keys, not from a type, so a continuation page can have other keys, or the same keys in another order, than the first page. Rendered without a header, its values sat under the first page's headings. JSON tables now always carry their own header, and the pager keeps a continuation header unless it repeats the pinned one. Before, it dropped every header it detected on a continuation page, so the transformer's header never reached the plain human More pager or the Spectre ANSI pager. The repeat check compares words, so the same header padded to other column widths is still dropped. A kept header keeps its separator line, even one identical to the pinned separator. CLR tables are unaffected: their continuation pages render no header. Also: - Rows that are all empty objects produced a table with no columns, which printed nothing. They now read {}. - Cells are looked up ordinally (IndexOf/GetAt plus an ordinal key check), the way the columns are collected. A JsonObject built with case-insensitive property names no longer fills a "name" column from its "Name" key. - A page declared with a JSON item type (IReplPage.ItemType, Nullable included) renders as JSON even when every item is a JSON null. Before, it rendered blank "- " rows or "No results.". - A JsonElement of kind Null in a page of objects is an empty row, like a CLR null, instead of turning the whole table into literals. Repl.Core grants InternalsVisibleTo to Repl.SpectreTests, so its tests can render continuation pages and drive PagerSession. TDD: each new test was red first, for its predicted reason. The pager-level tests drive the transformer output through PagerSession, which is where the user sees it. Each fix was falsified with a compiling substitution that turned its own test red. Falsifying the word comparison also broke the existing duplicate-header pager tests. Refs #92 --- docs/output-system.md | 10 +- src/Repl.Core/InternalsVisibleTo.cs | 1 + .../Output/HumanOutputTransformer.cs | 45 ++++--- src/Repl.Core/Output/JsonHumanShape.cs | 42 +++++-- .../ResultFlow/PagerPayloadParser.cs | 22 +++- .../SpectreHumanOutputTransformer.cs | 41 ++++--- .../Given_SpectreHumanOutputJson.cs | 91 ++++++++++++++ src/Repl.Tests/Given_HumanOutputJson.cs | 114 ++++++++++++++++-- src/Repl.Tests/Given_ResultFlowPager.cs | 30 +++++ 9 files changed, 342 insertions(+), 54 deletions(-) diff --git a/docs/output-system.md b/docs/output-system.md index edb0e271..b7ec31af 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -30,9 +30,13 @@ A handler can return `System.Text.Json.Nodes.JsonNode` (`JsonObject`, `JsonArray data rather than the CLR members of those types: - An object becomes one `key: value` line per field. -- Rows of objects become a table. Its columns are the union of the rows' keys in first-seen order, and a - key missing from a row leaves that cell empty. -- An array of scalars becomes one value per line. +- Rows of objects become a table. Its columns are the union of the rows' keys in first-seen order, matched + ordinally, and a key missing from a row leaves that cell empty. The rows' keys, not a type, define the + columns. So when a page the pager fetches has other columns than the first page, it keeps its own header + row, while a header that repeats the first page's is still dropped. +- An array of scalars becomes one value per line. So do rows that have no keys at all: each reads `{}`. +- A page declared with a JSON item type, such as `IReplPageSource`, renders as JSON even when + every item on it is a JSON null. - Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and a nested object or array shows as itself. - A JSON value held by a property of an ordinary result object, or passed as a result's details, shows as diff --git a/src/Repl.Core/InternalsVisibleTo.cs b/src/Repl.Core/InternalsVisibleTo.cs index 92e02d1d..446b01ca 100644 --- a/src/Repl.Core/InternalsVisibleTo.cs +++ b/src/Repl.Core/InternalsVisibleTo.cs @@ -8,3 +8,4 @@ [assembly: InternalsVisibleTo("Repl.Spectre")] [assembly: InternalsVisibleTo("Repl.Mcp")] [assembly: InternalsVisibleTo("Repl.McpTests")] +[assembly: InternalsVisibleTo("Repl.SpectreTests")] diff --git a/src/Repl.Core/Output/HumanOutputTransformer.cs b/src/Repl.Core/Output/HumanOutputTransformer.cs index c000b67b..e6003888 100644 --- a/src/Repl.Core/Output/HumanOutputTransformer.cs +++ b/src/Repl.Core/Output/HumanOutputTransformer.cs @@ -96,19 +96,33 @@ private static string RenderPage( ResultFlowPageRenderMode mode, bool includeFooter) { - var body = page.UntypedItems.Count == 0 - ? "No results." - : RenderCollection( - page.UntypedItems, - depth: 0, - settings, - includeTableHeader: mode == ResultFlowPageRenderMode.Initial); + var body = RenderPageBody(page, settings, mode); var footer = includeFooter ? ResultFlowPageFooterBuilder.RenderHuman(page) : string.Empty; return string.IsNullOrWhiteSpace(footer) ? body : string.Concat(body, Environment.NewLine, footer); } + private static string RenderPageBody(IReplPage page, HumanRenderSettings settings, ResultFlowPageRenderMode mode) + { + if (page.UntypedItems.Count == 0) + { + return "No results."; + } + + // By its declared item type: a page of JSON nulls has no item to be recognized by. + if (JsonHumanShape.IsJsonType(page.ItemType)) + { + return RenderJsonItems(page.UntypedItems, settings); + } + + return RenderCollection( + page.UntypedItems, + depth: 0, + settings, + includeTableHeader: mode == ResultFlowPageRenderMode.Initial); + } + private static string RenderTopLevelEnumerable(System.Collections.IEnumerable enumerable, HumanRenderSettings settings) { var lines = enumerable @@ -138,11 +152,13 @@ private static string RenderTopLevelEnumerable(System.Collections.IEnumerable en JsonArray { Count: 0 } => "No results.", // Its own path rather than the generic collection one, which recognizes JSON by its first non-null // item and so has nothing to go on for an array of nulls. - JsonArray jsonArray => RenderJsonItems([.. jsonArray], settings, includeHeader: true), + JsonArray jsonArray => RenderJsonItems([.. jsonArray], settings), _ => JsonHumanShape.Literal(node), }; - private static string RenderJsonItems(object?[] items, HumanRenderSettings settings, bool includeHeader) + // A JSON table always carries its header, continuation pages included: its columns come from its own rows' + // keys rather than from a type, so another page's headings could mislabel its cells. + private static string RenderJsonItems(IReadOnlyList items, HumanRenderSettings settings) { if (!JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows)) { @@ -153,14 +169,9 @@ private static string RenderJsonItems(object?[] items, HumanRenderSettings setti : RenderScalar(item, member: null, depth: 0, compactCollection: true, settings.Width, settings))); } - var tableRows = new List(rows.Length + (includeHeader ? 1 : 0)); - if (includeHeader) - { - tableRows.Add([.. columns.Select(JsonHumanShape.Label)]); - } - + var tableRows = new List(rows.Length + 1) { columns.Select(JsonHumanShape.Label).ToArray() }; tableRows.AddRange(rows.Select(row => columns.Select(column => JsonHumanShape.Cell(row, column)).ToArray())); - return FormatTable(tableRows, settings, includeHeader); + return FormatTable(tableRows, settings, includeHeader: true); } private static string RenderJsonObject(JsonObject jsonObject, HumanRenderSettings settings) @@ -272,7 +283,7 @@ private static bool TryRenderTable( // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. if (JsonHumanShape.IsJson(firstNonNull)) { - text = RenderJsonItems(values, settings, includeHeader); + text = RenderJsonItems(values, settings); return true; } diff --git a/src/Repl.Core/Output/JsonHumanShape.cs b/src/Repl.Core/Output/JsonHumanShape.cs index 62040fc9..589623d5 100644 --- a/src/Repl.Core/Output/JsonHumanShape.cs +++ b/src/Repl.Core/Output/JsonHumanShape.cs @@ -30,6 +30,10 @@ internal static class JsonHumanShape public static bool IsJson([NotNullWhen(true)] object? value) => value is JsonNode or JsonElement; + /// Whether items declared as are JSON data, nullable elements included. + public static bool IsJsonType(Type type) => + typeof(JsonNode).IsAssignableFrom(type) || (Nullable.GetUnderlyingType(type) ?? type) == typeof(JsonElement); + /// /// Reads as a JSON node. A is wrapped, and the element /// itself is never modified. A JSON null comes back as a node with @@ -110,7 +114,8 @@ public static string Label(string key) => /// /// Reads as rows of JSON objects. The columns are the union of their keys in /// first-seen order, so a key missing from one row leaves its cell empty rather than dropping the row. - /// A value is an empty row. Fails if any other value is not a JSON object. + /// A JSON null, as a value or a null , is an empty row. Fails if any other value is not a JSON object, or if no + /// row has a key: a table with no columns would show nothing of them. /// public static bool TryGetObjectRows( IReadOnlyList values, @@ -122,7 +127,6 @@ public static bool TryGetObjectRows( var converted = new JsonObject?[values.Count]; var seen = new HashSet(StringComparer.Ordinal); var ordered = new List(); - var anyRow = false; for (var i = 0; i < values.Count; i++) { if (values[i] is null) @@ -130,12 +134,23 @@ public static bool TryGetObjectRows( continue; } - if (!TryGetNode(values[i], out var node) || node is not JsonObject row) + if (!TryGetNode(values[i], out var node)) + { + return false; + } + + // A JsonElement of kind Null (or Undefined, a default element) is no data either: an empty row, like + // a CLR null. + if (node is null) + { + continue; + } + + if (node is not JsonObject row) { return false; } - anyRow = true; converted[i] = row; foreach (var property in row) { @@ -146,7 +161,7 @@ public static bool TryGetObjectRows( } } - if (!anyRow) + if (ordered.Count == 0) { return false; } @@ -156,9 +171,20 @@ public static bool TryGetObjectRows( return true; } - /// The cell for : empty when the row does not have the key. - public static string Cell(JsonObject? row, string column) => - row is not null && row.TryGetPropertyValue(column, out var value) ? Literal(value) : string.Empty; + /// + /// The cell for : empty when the row does not have the key. Matched ordinally, the + /// way the columns were collected, even in a row built with case-insensitive property names. + /// + public static string Cell(JsonObject? row, string column) + { + if (row is null || row.IndexOf(column) is not (>= 0 and var index)) + { + return string.Empty; + } + + var property = row.GetAt(index); + return string.Equals(property.Key, column, StringComparison.Ordinal) ? Literal(property.Value) : string.Empty; + } // Format characters can only appear inside a JSON string here, where \uXXXX is the same character // to a JSON reader. Returns the input unchanged — no allocation — when it has none. diff --git a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs index 81ffc2b4..e6570702 100644 --- a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs +++ b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs @@ -12,9 +12,17 @@ public static ParsedPagerPayload Parse(string payload, PagerHeader? header, bool var lines = SplitLines(payload); var payloadHeader = DetectHeader(lines); var resolvedHeader = header ?? payloadHeader; - var headerLineCount = payloadHeader.Lines.Count; var content = new List(); - for (var i = headerLineCount; i < lines.Count; i++) + // A continuation's own header is dropped only when it repeats the pinned one. One that names other + // columns stays in the content, separator included even when it matches the pinned one: JSON rows take + // their columns from their own keys, so a later page can have others, and its rows would otherwise sit + // under headings that are not theirs. + if (header is not null && !RepeatsHeader(header, payloadHeader)) + { + content.AddRange(payloadHeader.Lines); + } + + for (var i = payloadHeader.Lines.Count; i < lines.Count; i++) { var normalized = NormalizeLine(lines[i]); if (resolvedHeader.NormalizedLines.Contains(normalized) @@ -51,6 +59,16 @@ private static PagerHeader DetectHeader(List lines) : PagerHeader.Empty; } + // Word by word: a repeated header is padded to its own page's column widths, and its separator line with it. + // A label truncated to a different width on each page does not match, so that header is kept, not lost. + private static bool RepeatsHeader(PagerHeader pinned, PagerHeader candidate) => + pinned.Lines.Count > 0 + && candidate.Lines.Count > 0 + && HeaderWords(pinned.Lines[0]).SequenceEqual(HeaderWords(candidate.Lines[0]), StringComparer.Ordinal); + + private static string[] HeaderWords(string line) => + NormalizeLine(line).Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries); + private static PagerHeader CreateHeader(string[] lines) => new( lines, diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index 7b1d9a4d..1a74e446 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -222,7 +222,7 @@ private string RenderEnumerable( // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. if (JsonHumanShape.IsJson(firstNonNull)) { - return RenderJsonItems(items, includeTableHeader); + return RenderJsonItems(items); } if (IsSimpleValue(firstNonNull.GetType())) @@ -251,17 +251,31 @@ private string RenderPage( ResultFlowPageRenderMode mode, bool includeFooter) { - var body = page.UntypedItems.Count == 0 - ? "No results." - : RenderEnumerable( - page.UntypedItems, - includeTableHeader: mode == ResultFlowPageRenderMode.Initial); + var body = RenderPageBody(page, mode); var footer = includeFooter ? RenderPageFooter(page) : string.Empty; return string.IsNullOrWhiteSpace(footer) ? body : string.Concat(body, Environment.NewLine, footer); } + private string RenderPageBody(IReplPage page, ResultFlowPageRenderMode mode) + { + if (page.UntypedItems.Count == 0) + { + return "No results."; + } + + // By its declared item type: a page of JSON nulls has no item to be recognized by. + if (JsonHumanShape.IsJsonType(page.ItemType)) + { + return RenderJsonItems(page.UntypedItems); + } + + return RenderEnumerable( + page.UntypedItems, + includeTableHeader: mode == ResultFlowPageRenderMode.Initial); + } + private static string RenderPageFooter(IReplPage page) { var info = page.PageInfo; @@ -291,27 +305,24 @@ [.. jsonObject.Select(static property => JsonArray { Count: 0 } => "No results.", // Its own path rather than RenderEnumerable, which recognizes JSON by its first non-null item and so // has nothing to go on for an array of nulls. - JsonArray jsonArray => RenderJsonItems([.. jsonArray], includeHeader: true), + JsonArray jsonArray => RenderJsonItems([.. jsonArray]), _ => JsonHumanShape.Literal(node), }; - private string RenderJsonItems(object?[] items, bool includeHeader) => + // A JSON table always carries its header, continuation pages included: its columns come from its own rows' + // keys rather than from a type, so another page's headings could mislabel its cells. + private string RenderJsonItems(IReadOnlyList items) => JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows) - ? RenderToString(BuildJsonTable(columns, rows, includeHeader)) + ? RenderToString(BuildJsonTable(columns, rows)) : string.Join( Environment.NewLine, items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) ? literal : RenderInlineValue(item))); - private static Table BuildJsonTable(string[] columns, JsonObject?[] rows, bool includeHeaders) + private static Table BuildJsonTable(string[] columns, JsonObject?[] rows) { var table = new Table() .Border(TableBorder.None) .Collapse(); - if (!includeHeaders) - { - table.HideHeaders(); - } - foreach (var column in columns) { table.AddColumn(new TableColumn($"[bold]{Markup.Escape(JsonHumanShape.Label(column))}[/]")); diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index 64faeeaf..7fc92156 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -134,6 +134,97 @@ public async Task When_AJsonStringCarriesBidiControls_Then_TheyAreEscaped() output.Should().Contain(@"\u202E"); } + [TestMethod] + [Description("A continuation page carries its own JSON header: its rows' keys can come in another order, or be other keys, than the first page's, whose headings would then mislabel its values.")] + public async Task When_AContinuationPageHasOtherKeys_Then_ItCarriesItsOwnHeader() + { + var page = new ReplPage( + [new JsonObject { ["name"] = "b", ["id"] = 2 }], + new ReplPageInfo(Cursor: "1", NextCursor: null, TotalCount: null, PageSize: 1)); + + var raw = await new SpectreHumanOutputTransformer() + .TransformPageAsync(page, ResultFlowPageRenderMode.Continuation, CancellationToken.None) + .ConfigureAwait(false); + + AnsiStyling().Replace(raw, string.Empty).Should().MatchRegex(@"name\s+id"); + } + + [TestMethod] + [Description("Rows that are all empty objects have no keys to make columns of: each reads {} rather than being dropped from a table with no columns.")] + public async Task When_EveryRowIsAnEmptyObject_Then_EachReadsAsAnEmptyObject() + { + var output = await RenderAsync(new JsonArray(new JsonObject(), new JsonObject())).ConfigureAwait(false); + + output.Split(Environment.NewLine).Should().Equal("{}", "{}"); + } + + [TestMethod] + [Description("Cells are looked up ordinally, the way the columns were collected: a case-insensitive row must not also fill the 'name' column from its 'Name' key.")] + public async Task When_ARowIsCaseInsensitive_Then_ItsCellsStillMatchKeysOrdinally() + { + var caseInsensitive = new JsonObject(new JsonNodeOptions { PropertyNameCaseInsensitive = true }) { ["Name"] = "upper" }; + + var output = await RenderAsync(new JsonArray(caseInsensitive, new JsonObject { ["name"] = "lower" })).ConfigureAwait(false); + + output.Split("\"upper\"").Should().HaveCount(2, "the value belongs to its own column only"); + } + + [TestMethod] + [Description("A page of JSON nulls is recognized from its declared item type: with no non-null item to go on, it would otherwise read as having no results.")] + public async Task When_APageOfJsonNullsIsRendered_Then_EachItemReadsNull() + { + var page = new ReplPage( + [null, null], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + + var output = await RenderAsync(page).ConfigureAwait(false); + + output.Split(Environment.NewLine).Should().Equal("null", "null"); + } + + [TestMethod] + [Description("Through the pager, which pins the first page's header and drops a repeated one (a bold first line counts as one): a continuation page with other keys must keep its own header, or its rows sit under headings that are not theirs.")] + public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOwnHeader() + { + var transformer = new SpectreHumanOutputTransformer(); + var first = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) + .ConfigureAwait(false); + var next = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["name"] = "b", ["id"] = 2 }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) + .ConfigureAwait(false); + + var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); + session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + + var lines = session.Lines.Select(line => AnsiStyling().Replace(line, string.Empty)).ToList(); + var row = lines.FindIndex(line => line.Contains("\"b\"", StringComparison.Ordinal)); + row.Should().BePositive(); + lines.Take(row).Should().Contain(line => line.Contains("name", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("Through the pager, a continuation page with the same keys at other widths repeats the pinned header, so it adds its data row only.")] + public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded() + { + var transformer = new SpectreHumanOutputTransformer(); + var first = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) + .ConfigureAwait(false); + var next = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) + .ConfigureAwait(false); + + var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); + session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + + session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); + AnsiStyling().Replace(session.Lines[1], string.Empty).Should().Contain("\"bbbbbb\""); + } + + private static ReplPage SingleRowPage(JsonObject row) => + new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); + private sealed record Holder(string Name, JsonObject Payload); private sealed record Owner(string Login); diff --git a/src/Repl.Tests/Given_HumanOutputJson.cs b/src/Repl.Tests/Given_HumanOutputJson.cs index f0cc3f88..d8d02d0f 100644 --- a/src/Repl.Tests/Given_HumanOutputJson.cs +++ b/src/Repl.Tests/Given_HumanOutputJson.cs @@ -12,6 +12,7 @@ namespace Repl.Tests; public sealed class Given_HumanOutputJson { private static readonly string[] ClrMembers = ["Options", "Parent", "Root", "Count", "ValueKind"]; + private static readonly string[] PageTwoColumns = ["name", "id"]; [TestMethod] [Description("The issue's own repro: a JsonObject result renders one line per JSON field, values as JSON literals, and none of JsonObject's CLR members.")] @@ -178,20 +179,115 @@ public async Task When_AJsonStringCarriesBidiControls_Then_TheyAreEscaped() output.Should().Contain(@"\u202E"); } - private sealed record Holder(string Name, JsonObject Payload); + [TestMethod] + [Description("A continuation page carries its own JSON header: its rows' keys can come in another order, or be other keys, than the first page's, whose headings would then mislabel its values.")] + public async Task When_AContinuationPageHasOtherKeys_Then_ItCarriesItsOwnHeader() + { + var page = new ReplPage( + [new JsonObject { ["name"] = "b", ["id"] = 2 }], + new ReplPageInfo(Cursor: "1", NextCursor: null, TotalCount: null, PageSize: 1)); - private sealed record Owner(string Login); + var output = await CreateTransformer().TransformPageAsync(page, ResultFlowPageRenderMode.Continuation, CancellationToken.None); + + output.Split(Environment.NewLine)[0].Should().MatchRegex(@"^name\s+id\s*$"); + } + + [TestMethod] + [Description("Rows that are all empty objects have no keys to make columns of: each reads {} rather than being dropped from a table with no columns.")] + public async Task When_EveryRowIsAnEmptyObject_Then_EachReadsAsAnEmptyObject() + { + (await RenderAsync(new JsonArray(new JsonObject(), new JsonObject()))).Split(Environment.NewLine) + .Should().Equal("{}", "{}"); + } + + [TestMethod] + [Description("Cells are looked up ordinally, the way the columns were collected: a case-insensitive row must not also fill the 'name' column from its 'Name' key.")] + public async Task When_ARowIsCaseInsensitive_Then_ItsCellsStillMatchKeysOrdinally() + { + var caseInsensitive = new JsonObject(new JsonNodeOptions { PropertyNameCaseInsensitive = true }) { ["Name"] = "upper" }; + + var output = await RenderAsync(new JsonArray(caseInsensitive, new JsonObject { ["name"] = "lower" })); + + var lines = output.Split(Environment.NewLine); + lines[0].Should().MatchRegex(@"^Name\s+name\s*$"); + var upperRow = lines.Single(line => line.Contains("\"upper\"", StringComparison.Ordinal)); + upperRow.TrimEnd().Should().Be("\"upper\"", "its value sits under Name only, leaving the name cell empty"); + var lowerRow = lines.Single(line => line.Contains("\"lower\"", StringComparison.Ordinal)); + lowerRow.Should().MatchRegex(@"^\s+""lower""\s*$", "its value sits under name only, leaving the Name cell empty"); + } + + [TestMethod] + [Description("A page of JSON nulls is recognized from its declared item type: with no non-null item to go on, it would otherwise render blank rows.")] + public async Task When_APageOfJsonNullsIsRendered_Then_EachItemReadsNull() + { + var page = new ReplPage( + [null, null], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + + (await RenderAsync(page)).Split(Environment.NewLine).Should().Equal("null", "null"); + } - private static async Task RenderAsync(object value) + [TestMethod] + [Description("A page of nullable JsonElement is JSON too: its declared item type is Nullable, not JsonElement itself.")] + public async Task When_APageOfNullableJsonElementNullsIsRendered_Then_EachItemReadsNull() + { + var page = new ReplPage( + [null, null], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + + (await RenderAsync(page)).Split(Environment.NewLine).Should().Equal("null", "null"); + } + + [TestMethod] + [Description("Through the pager, which pins the first page's header and drops a repeated one: a continuation page with other keys must keep its own header, or its rows sit under headings that are not theirs.")] + public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOwnHeader() { - var transformer = new HumanOutputTransformer( - () => new HumanRenderSettings( - Width: 120, - UseAnsi: false, - Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); - return await transformer.TransformAsync(value, CancellationToken.None).ConfigureAwait(false); + var transformer = CreateTransformer(); + var first = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None); + var next = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["name"] = "b", ["id"] = 2 }), ResultFlowPageRenderMode.Continuation, CancellationToken.None); + + var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); + session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + + var lines = session.Lines.ToList(); + var row = lines.FindIndex(line => line.Contains("\"b\"", StringComparison.Ordinal)); + row.Should().BePositive(); + lines.Take(row).Should().Contain(line => line.Split(' ', StringSplitOptions.RemoveEmptyEntries).SequenceEqual(PageTwoColumns)); + } + + [TestMethod] + [Description("A JSON null that arrives as a JsonElement is an empty row, like a CLR null: it must not turn the whole table into bare literals.")] + public async Task When_APageOfJsonElementsHasANullRow_Then_ItStaysATable() + { + using var document = JsonDocument.Parse("""[{"id":1},null,{"id":2}]"""); + var page = new ReplPage( + [.. document.RootElement.EnumerateArray()], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 3)); + + var output = await RenderAsync(page); + + output.Split(Environment.NewLine)[0].Should().MatchRegex(@"^id\s*$"); + output.Should().NotContain("{", "the rows render as table cells, not as JSON literals"); } + private sealed record Holder(string Name, JsonObject Payload); + + private sealed record Owner(string Login); + + private static ReplPage SingleRowPage(JsonObject row) => + new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); + + private static HumanOutputTransformer CreateTransformer() => + new(() => new HumanRenderSettings( + Width: 120, + UseAnsi: false, + Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + + private static async Task RenderAsync(object value) => + await CreateTransformer().TransformAsync(value, CancellationToken.None).ConfigureAwait(false); + private static void AssertNoClrMembers(string output) { foreach (var member in ClrMembers) diff --git a/src/Repl.Tests/Given_ResultFlowPager.cs b/src/Repl.Tests/Given_ResultFlowPager.cs index eae06bc3..73fda908 100644 --- a/src/Repl.Tests/Given_ResultFlowPager.cs +++ b/src/Repl.Tests/Given_ResultFlowPager.cs @@ -1036,6 +1036,36 @@ public void When_HeaderContainsLoneEscape_Then_NormalizationStillDeduplicatesCon second.ContentLines.Should().Equal("two"); } + [TestMethod] + [Description("A continuation header that repeats the pinned one, only padded to other column widths, is still dropped as a duplicate.")] + public void When_AContinuationHeaderRepeatsThePinnedOneAtOtherWidths_Then_ItIsDropped() + { + var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); + var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "--- ----", "100 b"), first.Header); + + second.ContentLines.Should().Equal("100 b"); + } + + [TestMethod] + [Description("A continuation header that names other columns than the pinned one is kept: without it the page's rows would sit under headings that are not theirs.")] + public void When_AContinuationHeaderDiffersFromThePinnedOne_Then_ItIsKept() + { + var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); + var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "name id", "---- --", "b 2"), first.Header); + + second.ContentLines.Should().Equal("name id", "---- --", "b 2"); + } + + [TestMethod] + [Description("A kept continuation header keeps its separator, even when the column widths make that separator identical to the pinned one: without it the header reads as a data row.")] + public void When_AKeptContinuationHeaderHasThePinnedSeparator_Then_TheSeparatorIsKeptToo() + { + var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); + var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id kind", "-- ----", "2 b"), first.Header); + + second.ContentLines.Should().Equal("id kind", "-- ----", "2 b"); + } + private static ValueTask WritePagerAsync( string payload, TextWriter output, From e6b61b4ac0facdf4af4112cc562a6d468c22791c Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Tue, 22 Sep 2026 19:40:25 -0400 Subject: [PATCH 3/7] test(spectre): force ANSI in the JSON pager tests The two Spectre pager tests used the default transformer, which has no OutputOptions and so leaves the ANSI decision to Spectre's own console detection. Locally that console emitted the bold header that the pager detects and pins. On CI it did not: nothing was pinned, both headers stayed inline, and When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded failed on all three OSes. Both tests now force AnsiMode.Always, the ANSI pager case the review raised. Checked both ways: with AnsiMode.Never the same-keys test fails as it did on CI, and with ANSI forced, neutralising the pager's keep-a-differing-header fix turns the other-keys test red. Refs #92 --- .../Given_SpectreHumanOutputJson.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index 7fc92156..34ca9f86 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -1,6 +1,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Text.RegularExpressions; +using Repl.Rendering; namespace Repl.SpectreTests; @@ -186,7 +187,7 @@ public async Task When_APageOfJsonNullsIsRendered_Then_EachItemReadsNull() [Description("Through the pager, which pins the first page's header and drops a repeated one (a bold first line counts as one): a continuation page with other keys must keep its own header, or its rows sit under headings that are not theirs.")] public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOwnHeader() { - var transformer = new SpectreHumanOutputTransformer(); + var transformer = CreateAnsiTransformer(); var first = await transformer.TransformPageAsync( SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) .ConfigureAwait(false); @@ -207,7 +208,7 @@ public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOw [Description("Through the pager, a continuation page with the same keys at other widths repeats the pinned header, so it adds its data row only.")] public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded() { - var transformer = new SpectreHumanOutputTransformer(); + var transformer = CreateAnsiTransformer(); var first = await transformer.TransformPageAsync( SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) .ConfigureAwait(false); @@ -222,6 +223,13 @@ public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdde AnsiStyling().Replace(session.Lines[1], string.Empty).Should().Contain("\"bbbbbb\""); } + // The ANSI pager case, where the bold header line is what the pager detects and pins. Forced, so these + // tests do not depend on whether the console running them supports ANSI. + private static SpectreHumanOutputTransformer CreateAnsiTransformer() => + new( + () => new HumanRenderSettings(Width: 120, UseAnsi: true, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark)), + new OutputOptions { AnsiMode = AnsiMode.Always }); + private static ReplPage SingleRowPage(JsonObject row) => new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); From 2c43687962355d01b6066899a1daed11faaab4b5 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Tue, 22 Sep 2026 20:03:24 -0400 Subject: [PATCH 4/7] fix(output): compare pager headers by column label, and render Spectre JSON details like a JSON result Addresses the second review of #111. The pager compared a continuation header with the pinned one word by word. So JSON keys containing spaces, such as 'first' / 'last name' against 'first last' / 'name', looked like the same header, the continuation header was dropped, and its rows sat under the wrong columns. The comparison is now by column label, read three ways: - In a plain table whose header is aligned on its separator line, each label is the text over one run of dashes. This matters because TextTableFormatter leaves a single space between columns below 100 columns wide. - In a styled header, each label is its own run between escape sequences, which is how Spectre 0.55 writes a bold header cell, spaces included. Each run is also split at double-space gaps, so a header styled as a single run with its padding inside still matches at other widths. - Otherwise, labels are what gaps of two or more spaces leave. This covers a header that is not aligned on its separator, as in the existing hash-header fixture. AnsiTextMetrics gains SplitAtControlSequences for the styled case. A JSON value passed as an IReplResult's details rendered as a compact literal in Spectre, but expanded in the human transformer. Spectre now renders it like a JSON result, as both transformers already do for object details. The JSON renderable is composed into the result's layout directly. Wrapping the already-rendered string in Text, as the page-details path still does, made Spectre count its escape sequences as columns, so wide lines wrapped a second time. The docs line is updated. TDD: the regroup tests (plain, narrow, styled), the whole-line-bold repeat, the Spectre details test and the wide-details test were red first. Each strategy was falsified with a compiling substitution that turned its own test red: - no separator spans: the narrow test; - no styled runs: the styled repeat; - no alignment guard: the existing hash-header test; - styled runs split on single spaces: both regroup tests, one of them driven through real Spectre output. Refs #92 --- docs/output-system.md | 4 +- .../ResultFlow/PagerPayloadParser.cs | 63 +++++++++++++++++-- src/Repl.Core/Terminal/AnsiTextMetrics.cs | 33 ++++++++++ .../SpectreHumanOutputTransformer.cs | 31 +++++++-- .../Given_SpectreHumanOutputJson.cs | 49 +++++++++++++++ src/Repl.Tests/Given_ResultFlowPager.cs | 60 ++++++++++++++++++ 6 files changed, 228 insertions(+), 12 deletions(-) diff --git a/docs/output-system.md b/docs/output-system.md index b7ec31af..e7b6a102 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -39,8 +39,8 @@ data rather than the CLR members of those types: every item on it is a JSON null. - Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and a nested object or array shows as itself. -- A JSON value held by a property of an ordinary result object, or passed as a result's details, shows as - a compact literal too. +- A JSON value held by a property of an ordinary result object shows as a compact literal too. One passed + as a result's details renders like a JSON result. In JSON strings and keys, control characters and Unicode format characters (such as bidirectional overrides) are escaped. So a payload can neither drive the terminal nor make it display something other diff --git a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs index e6570702..3d4215b3 100644 --- a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs +++ b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs @@ -59,15 +59,70 @@ private static PagerHeader DetectHeader(List lines) : PagerHeader.Empty; } - // Word by word: a repeated header is padded to its own page's column widths, and its separator line with it. + // Label by label: a repeated header is padded to its own page's column widths, and its separator line with it. // A label truncated to a different width on each page does not match, so that header is kept, not lost. private static bool RepeatsHeader(PagerHeader pinned, PagerHeader candidate) => pinned.Lines.Count > 0 && candidate.Lines.Count > 0 - && HeaderWords(pinned.Lines[0]).SequenceEqual(HeaderWords(candidate.Lines[0]), StringComparer.Ordinal); + && HeaderLabels(pinned).SequenceEqual(HeaderLabels(candidate), StringComparer.Ordinal); - private static string[] HeaderWords(string line) => - NormalizeLine(line).Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries); + // A label can hold spaces, so words alone would take 'first' / 'last name' for 'first last' / 'name', and a + // narrow table leaves a single space between columns. A plain table's separator spans each column, when the + // header is aligned on it; a styled header brackets each label in its own escape sequences; failing both, + // labels are what gaps of two or more spaces leave. + private static IEnumerable HeaderLabels(PagerHeader header) + { + var line = header.Lines[0]; + if (header.Lines.Count > 1 + && LabelsUnderSeparator(AnsiTextMetrics.StripControlSequences(line), header.Lines[1]) is { } spanned) + { + return spanned; + } + + return line.Contains('\u001b', StringComparison.Ordinal) + // Each run is split at its own gaps too: a header can also be styled as one run, padding included. + ? AnsiTextMetrics.SplitAtControlSequences(line).SelectMany(static run => SplitAtGaps(run)) + : SplitAtGaps(line); + } + + // The label over each run of dashes, or null when some of the header lies outside every run: the separator + // then does not say where its columns are. + private static List? LabelsUnderSeparator(string line, string separator) + { + var labels = new List(); + var checkedUpTo = 0; + var i = 0; + while (i < separator.Length) + { + if (separator[i] != '-') + { + i++; + continue; + } + + var start = i; + while (i < separator.Length && separator[i] == '-') + { + i++; + } + + if (!IsBlank(line, checkedUpTo, start)) + { + return null; + } + + labels.Add(start < line.Length ? line[start..Math.Min(i, line.Length)].Trim() : string.Empty); + checkedUpTo = i; + } + + return IsBlank(line, checkedUpTo, line.Length) ? labels : null; + } + + private static string[] SplitAtGaps(string text) => + text.Split(" ", StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + + private static bool IsBlank(string line, int from, int to) => + from >= line.Length || line.AsSpan(from, Math.Min(to, line.Length) - from).IsWhiteSpace(); private static PagerHeader CreateHeader(string[] lines) => new( diff --git a/src/Repl.Core/Terminal/AnsiTextMetrics.cs b/src/Repl.Core/Terminal/AnsiTextMetrics.cs index 52b72509..178e6278 100644 --- a/src/Repl.Core/Terminal/AnsiTextMetrics.cs +++ b/src/Repl.Core/Terminal/AnsiTextMetrics.cs @@ -69,6 +69,39 @@ public static string StripControlSequences(ReadOnlySpan text) return builder.ToString(); } + /// + /// The runs of text between escape sequences, in order and untrimmed. A renderer that styles each table + /// label on its own brackets every label with its own sequences, so a label reads as one run, spaces included. + /// + public static List SplitAtControlSequences(string text) + { + var span = text.AsSpan(); + var runs = new List(); + var start = 0; + for (var i = 0; i < span.Length; i++) + { + if (span[i] != '\u001b') + { + continue; + } + + if (i > start) + { + runs.Add(text[start..i]); + } + + i = SkipEscapeSequence(span, i); + start = i + 1; + } + + if (start < text.Length) + { + runs.Add(text[start..]); + } + + return runs; + } + private static int SkipEscapeSequence(ReadOnlySpan text, int escapeIndex) { if (escapeIndex + 1 >= text.Length) diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index 1a74e446..90592188 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -192,9 +192,14 @@ private string RenderReplResult(IReplResult result) return RenderToString(new Markup(statusMarkup)); } - var details = result.Details is IReplPage page - ? new Text(RenderPage(page)) - : RenderValueRenderable(result.Details, nested: false); + IRenderable details = result.Details switch + { + IReplPage page => new Text(RenderPage(page)), + // Like a JSON result, as the human transformer renders it, rather than as the compact literal a + // JSON value nested in a result object gets. + _ when JsonHumanShape.TryGetNode(result.Details, out var node) => BuildJson(node), + _ => RenderValueRenderable(result.Details, nested: false), + }; return RenderToString(new Rows(new IRenderable[] { new Markup(statusMarkup), @@ -299,9 +304,7 @@ private static string RenderPageFooter(IReplPage page) private string RenderJson(JsonNode? node) => node switch { JsonObject { Count: 0 } => "{}", - JsonObject jsonObject => RenderToString(BuildLabelValueGrid( - [.. jsonObject.Select(static property => - (JsonHumanShape.Label(property.Key), JsonHumanShape.Literal(property.Value))),])), + JsonObject jsonObject => RenderToString(BuildJsonObjectGrid(jsonObject)), JsonArray { Count: 0 } => "No results.", // Its own path rather than RenderEnumerable, which recognizes JSON by its first non-null item and so // has nothing to go on for an array of nulls. @@ -309,6 +312,22 @@ [.. jsonObject.Select(static property => _ => JsonHumanShape.Literal(node), }; + // The renderable for JSON composed into a larger layout, such as a result's details. Text wrapping an + // already-rendered string would count its escape sequences as columns and wrap its lines again. + private IRenderable BuildJson(JsonNode? node) => node switch + { + JsonObject { Count: > 0 } jsonObject => BuildJsonObjectGrid(jsonObject), + JsonArray jsonArray when JsonHumanShape.TryGetObjectRows([.. jsonArray], out var columns, out var rows) + => BuildJsonTable(columns, rows), + // Literal lines only from here on: plain text, nothing styled. + _ => new Text(RenderJson(node)), + }; + + private static Grid BuildJsonObjectGrid(JsonObject jsonObject) => + BuildLabelValueGrid( + [.. jsonObject.Select(static property => + (JsonHumanShape.Label(property.Key), JsonHumanShape.Literal(property.Value))),]); + // A JSON table always carries its header, continuation pages included: its columns come from its own rows' // keys rather than from a type, so another page's headings could mislabel its cells. private string RenderJsonItems(IReadOnlyList items) => diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index 34ca9f86..6a6320f8 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -223,6 +223,55 @@ public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdde AnsiStyling().Replace(session.Lines[1], string.Empty).Should().Contain("\"bbbbbb\""); } + [TestMethod] + [Description("Through the pager, with Spectre's own styled header: keys 'first' / 'last name' and 'first last' / 'name' share their words but are other columns, so the continuation keeps its header.")] + public async Task When_ThePagerAppendsAPageWhoseKeysRegroupTheWords_Then_ItsRowsKeepTheirOwnHeader() + { + var transformer = CreateAnsiTransformer(); + var first = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["first"] = "a", ["last name"] = "b" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) + .ConfigureAwait(false); + var next = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["first last"] = "c", ["name"] = "d" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) + .ConfigureAwait(false); + + var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); + session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + + var lines = session.Lines.Select(line => AnsiStyling().Replace(line, string.Empty)).ToList(); + var row = lines.FindIndex(line => line.Contains("\"d\"", StringComparison.Ordinal)); + row.Should().BePositive(); + lines.Take(row).Should().Contain(line => line.Contains("first last", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("JSON details render as the JSON result itself would, also when their lines fill the width: composed as a pre-rendered string, their escape sequences counted as columns and the lines wrapped again.")] + public async Task When_AResultCarriesWideJsonDetails_Then_TheyRenderAsTheResultWould() + { + var transformer = new SpectreHumanOutputTransformer( + () => new HumanRenderSettings(Width: 80, UseAnsi: true, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark)), + new OutputOptions { AnsiMode = AnsiMode.Always }); + JsonArray Rows() => new( + new JsonObject { ["id"] = 1, ["description"] = new string('x', 30), ["owner"] = "someone@example.com" }, + new JsonObject { ["id"] = 2, ["description"] = new string('y', 30), ["owner"] = "other@example.com" }); + + var direct = AnsiStyling().Replace(await transformer.TransformAsync(Rows(), CancellationToken.None).ConfigureAwait(false), string.Empty); + var details = AnsiStyling().Replace(await transformer.TransformAsync(Results.Success("x", Rows()), CancellationToken.None).ConfigureAwait(false), string.Empty); + + details.Should().EndWith(direct); + } + + [TestMethod] + [Description("JSON carried as an IReplResult's details renders like a JSON result, as the human transformer renders it, rather than as one compact literal.")] + public async Task When_AResultCarriesJsonDetails_Then_TheFieldsAreShown() + { + var output = await RenderAsync(Results.Success("done", new JsonObject { ["code"] = 42, ["state"] = "ok" })).ConfigureAwait(false); + + output.Should().MatchRegex(@"code:\s+42"); + output.Should().MatchRegex(@"state:\s+""ok"""); + AssertNoClrMembers(output); + } + // The ANSI pager case, where the bold header line is what the pager detects and pins. Forced, so these // tests do not depend on whether the console running them supports ANSI. private static SpectreHumanOutputTransformer CreateAnsiTransformer() => diff --git a/src/Repl.Tests/Given_ResultFlowPager.cs b/src/Repl.Tests/Given_ResultFlowPager.cs index 73fda908..7c238e97 100644 --- a/src/Repl.Tests/Given_ResultFlowPager.cs +++ b/src/Repl.Tests/Given_ResultFlowPager.cs @@ -1056,6 +1056,66 @@ public void When_AContinuationHeaderDiffersFromThePinnedOne_Then_ItIsKept() second.ContentLines.Should().Equal("name id", "---- --", "b 2"); } + [TestMethod] + [Description("Labels are compared column by column, as the separator spans them: 'first' / 'last name' and 'first last' / 'name' have the same words but are other columns, so the continuation keeps its header.")] + public void When_AContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + { + var first = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, "first last name", "----- ---------", "a b"), header: null); + var second = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, "first last name", "---------- ----", "c d"), first.Header); + + second.ContentLines.Should().Equal("first last name", "---------- ----", "c d"); + } + + [TestMethod] + [Description("A narrow table leaves a single space between columns, so both headers read 'first last name': only the separator's column spans tell 'first' / 'last name' from 'first last' / 'name'.")] + public void When_ANarrowContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + { + var first = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, "first last name", "----- ---------", "a b"), header: null); + var second = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, "first last name", "---------- ----", "c d"), first.Header); + + second.ContentLines.Should().Equal("first last name", "---------- ----", "c d"); + } + + [TestMethod] + [Description("A styled header, as Spectre writes it, has no separator: each label is its own styled run, spaces included, so regrouped words are other columns there too.")] + public void When_AStyledContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + { + var first = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, Bold("first") + " " + Bold("last name"), "a b"), header: null); + var next = Bold("first last") + " " + Bold("name"); + var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, next, "c d"), first.Header); + + second.ContentLines.Should().Equal(next, "c d"); + } + + [TestMethod] + [Description("A styled continuation header with the pinned labels, padded to other widths, is still dropped as a duplicate.")] + public void When_AStyledContinuationHeaderRepeatsThePinnedOne_Then_ItIsDropped() + { + var first = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, Bold("id") + " " + Bold("last name"), "1 a"), header: null); + var second = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, Bold("id ") + " " + Bold("last name") + " ", "100 b"), first.Header); + + second.ContentLines.Should().Equal("100 b"); + } + + [TestMethod] + [Description("A header bolded as one run, its padding inside the styling, is still recognized as a repeat at other widths: the labels inside the run are split at their gaps.")] + public void When_AWholeLineBoldHeaderRepeatsAtOtherWidths_Then_ItIsDropped() + { + var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, Bold("id name"), "1 a"), header: null); + var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, Bold("id name"), "1000 b"), first.Header); + + second.ContentLines.Should().Equal("1000 b"); + } + + private static string Bold(string text) => $"{(char)27}[1m{text}{(char)27}[0m"; + [TestMethod] [Description("A kept continuation header keeps its separator, even when the column widths make that separator identical to the pinned one: without it the header reads as a data row.")] public void When_AKeptContinuationHeaderHasThePinnedSeparator_Then_TheSeparatorIsKeptToo() From bf40d08ccc7180441d809fc464e0cbcdc2166138 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Tue, 22 Sep 2026 20:26:05 -0400 Subject: [PATCH 5/7] fix(output): pin the human ANSI table header, and read null JSON-typed properties as null Addresses the third review of #111. The pager recognized a header line only by the exact ESC[1m. The human palette styles its table header as a combined sequence (ESC[1;38;5;221m, or ESC[1;38;5;52m in the light theme), so in the interactive human ANSI pager the first page's header was never pinned. Since JSON continuation pages now carry their own header, that header repeated on every page fetched. A line whose leading escape sequences set bold, before any text, now counts as a header. The SGR parameters are parsed, so a 1 that is an extended colour's argument (38;5;1, 48;2;r;g;b) is not taken for bold. The existing ESC[1m rule is unchanged. Recognizing more headers widens what the pager treats as one, so two guards bound it: - A header whose text is empty after normalization, such as a line holding only a style opener, is no header. Otherwise every blank line after it would be dropped as a repeat. - Lines equal to the header are dropped only from continuation pages. Within one payload, such as a long string result, a line identical to its first line is data and is kept. A property declared as JSON (JsonNode and subtypes, JsonElement, Nullable) that holds null rendered as an empty value, while --json writes null. Both transformers now default such a member's NullDisplayText to "null", decided once per member from the declared type. An explicit DisplayFormat.NullDisplayText still wins. TDD: the combined-bold header, the ANSI pager with the same keys, the null JSON-typed property (human and Spectre), the repeated first line, the escape-only first line and the bold-after-text line were red first. The colour-index and NullDisplayText tests are guards. Each fix was falsified on its own with a compiling substitution that turned its test red. Refs #92 --- docs/output-system.md | 6 +- .../Output/HumanOutputTransformer.cs | 2 +- src/Repl.Core/Output/JsonHumanShape.cs | 11 +++- .../ResultFlow/PagerPayloadParser.cs | 66 ++++++++++++++++++- .../SpectreHumanOutputTransformer.cs | 2 +- .../Given_SpectreHumanOutputJson.cs | 11 ++++ src/Repl.Tests/Given_HumanOutputJson.cs | 43 ++++++++++++ src/Repl.Tests/Given_ResultFlowPager.cs | 53 +++++++++++++++ 8 files changed, 186 insertions(+), 8 deletions(-) diff --git a/docs/output-system.md b/docs/output-system.md index e7b6a102..350407b5 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -39,8 +39,10 @@ data rather than the CLR members of those types: every item on it is a JSON null. - Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and a nested object or array shows as itself. -- A JSON value held by a property of an ordinary result object shows as a compact literal too. One passed - as a result's details renders like a JSON result. +- A JSON value held by a property of an ordinary result object shows as a compact literal too, and a + property declared as JSON (`JsonNode?`, `JsonElement?`) that holds `null` reads `null`, unless its + `DisplayFormat` sets a `NullDisplayText`. A JSON value passed as a result's details renders like a JSON + result. In JSON strings and keys, control characters and Unicode format characters (such as bidirectional overrides) are escaped. So a payload can neither drive the terminal nor make it display something other diff --git a/src/Repl.Core/Output/HumanOutputTransformer.cs b/src/Repl.Core/Output/HumanOutputTransformer.cs index e6003888..4cbd0d95 100644 --- a/src/Repl.Core/Output/HumanOutputTransformer.cs +++ b/src/Repl.Core/Output/HumanOutputTransformer.cs @@ -476,7 +476,7 @@ private static DisplayMember[] GetDisplayMembers( property, string.IsNullOrWhiteSpace(display?.GetName()) ? property.Name : display!.GetName()!, display?.GetOrder(), - displayFormat?.NullDisplayText); + displayFormat?.NullDisplayText ?? JsonHumanShape.NullText(property.PropertyType)); }) .Where(member => member is not null) .Select(member => member!) diff --git a/src/Repl.Core/Output/JsonHumanShape.cs b/src/Repl.Core/Output/JsonHumanShape.cs index 589623d5..53afc874 100644 --- a/src/Repl.Core/Output/JsonHumanShape.cs +++ b/src/Repl.Core/Output/JsonHumanShape.cs @@ -30,6 +30,12 @@ internal static class JsonHumanShape public static bool IsJson([NotNullWhen(true)] object? value) => value is JsonNode or JsonElement; + /// + /// What a declared as reads as: a JSON null, as --json + /// writes it, for a JSON type; , the renderer's own default, for any other. + /// + public static string? NullText(Type type) => IsJsonType(type) ? Literal(node: null) : null; + /// Whether items declared as are JSON data, nullable elements included. public static bool IsJsonType(Type type) => typeof(JsonNode).IsAssignableFrom(type) || (Nullable.GetUnderlyingType(type) ?? type) == typeof(JsonElement); @@ -114,8 +120,9 @@ public static string Label(string key) => /// /// Reads as rows of JSON objects. The columns are the union of their keys in /// first-seen order, so a key missing from one row leaves its cell empty rather than dropping the row. - /// A JSON null, as a value or a null , is an empty row. Fails if any other value is not a JSON object, or if no - /// row has a key: a table with no columns would show nothing of them. + /// A JSON null, as a value or a null , is an empty row. Fails + /// if any other value is not a JSON object, or if no row has a key: a table with no columns would show + /// nothing of them. /// public static bool TryGetObjectRows( IReadOnlyList values, diff --git a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs index 3d4215b3..9e17ef2d 100644 --- a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs +++ b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs @@ -25,7 +25,8 @@ public static ParsedPagerPayload Parse(string payload, PagerHeader? header, bool for (var i = payloadHeader.Lines.Count; i < lines.Count; i++) { var normalized = NormalizeLine(lines[i]); - if (resolvedHeader.NormalizedLines.Contains(normalized) + // Only a continuation repeats a header; within one payload, a line like its first one is data. + if ((header is not null && resolvedHeader.NormalizedLines.Contains(normalized)) || (stripPresentationChrome && IsPageFooterLine(lines[i]))) { continue; @@ -54,11 +55,72 @@ private static PagerHeader DetectHeader(List lines) return CreateHeader([lines[0]]); } - return lines[0].Contains("\u001b[1m", StringComparison.Ordinal) + return (lines[0].Contains("\u001b[1m", StringComparison.Ordinal) || StartsBold(lines[0])) + && NormalizeLine(lines[0]).Length > 0 ? CreateHeader([lines[0]]) : PagerHeader.Empty; } + // Bold set by the sequences the line starts with, before any text, within a combined sequence such as the + // human palette's table header (ESC[1;38;5;221m). A 1 that is an extended colour's argument, as in 38;5;1, + // is a colour. Bold starting later in the line styles text within it, not a header. + private static bool StartsBold(string line) + { + var start = 0; + while (line.AsSpan(start).StartsWith("\u001b[", StringComparison.Ordinal)) + { + var end = start + 2; + while (end < line.Length && (char.IsAsciiDigit(line[end]) || line[end] == ';')) + { + end++; + } + + if (end >= line.Length || line[end] != 'm') + { + return false; + } + + if (SgrSetsBold(line.AsSpan(start + 2, end - start - 2))) + { + return true; + } + + start = end + 1; + } + + return false; + } + + private static bool SgrSetsBold(ReadOnlySpan parameters) + { + var colourArguments = 0; + var colourModeNext = false; + foreach (var range in parameters.Split(';')) + { + var parameter = parameters[range]; + if (colourModeNext) + { + // 5 takes a palette index, 2 an RGB triple. + colourArguments = parameter is "5" ? 1 : parameter is "2" ? 3 : 0; + colourModeNext = false; + } + else if (colourArguments > 0) + { + colourArguments--; + } + else if (parameter is "38" or "48" or "58") + { + colourModeNext = true; + } + else if (parameter is "1") + { + return true; + } + } + + return false; + } + // Label by label: a repeated header is padded to its own page's column widths, and its separator line with it. // A label truncated to a different width on each page does not match, so that header is kept, not lost. private static bool RepeatsHeader(PagerHeader pinned, PagerHeader candidate) => diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index 90592188..9d640fee 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -699,7 +699,7 @@ private static DisplayMember[] GetDisplayMembers(Type type) => property, string.IsNullOrWhiteSpace(display?.GetName()) ? property.Name : display!.GetName()!, display?.GetOrder(), - displayFormat?.NullDisplayText); + displayFormat?.NullDisplayText ?? JsonHumanShape.NullText(property.PropertyType)); }) .Where(member => member is not null) .Select(member => member!) diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index 6a6320f8..82860259 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -282,6 +282,17 @@ private static SpectreHumanOutputTransformer CreateAnsiTransformer() => private static ReplPage SingleRowPage(JsonObject row) => new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); + [TestMethod] + [Description("A property declared as JSON holding null is a JSON null, as --json writes it: it reads null, not an empty value.")] + public async Task When_AJsonTypedPropertyIsNull_Then_ItReadsNull() + { + var output = await RenderAsync(new NullableHolder("h1", Payload: null)).ConfigureAwait(false); + + output.Should().MatchRegex(@"Payload\W+null"); + } + + private sealed record NullableHolder(string Name, JsonNode? Payload); + private sealed record Holder(string Name, JsonObject Payload); private sealed record Owner(string Login); diff --git a/src/Repl.Tests/Given_HumanOutputJson.cs b/src/Repl.Tests/Given_HumanOutputJson.cs index d8d02d0f..ccf74eab 100644 --- a/src/Repl.Tests/Given_HumanOutputJson.cs +++ b/src/Repl.Tests/Given_HumanOutputJson.cs @@ -272,6 +272,49 @@ [.. document.RootElement.EnumerateArray()], output.Should().NotContain("{", "the rows render as table cells, not as JSON literals"); } + [TestMethod] + [Description("A property declared as JSON holding null is a JSON null, as --json writes it: it reads null, not an empty value, both as a field and as a table cell.")] + public async Task When_AJsonTypedPropertyIsNull_Then_ItReadsNull() + { + (await RenderAsync(new NullableHolder("h1", Payload: null))).Should().MatchRegex(@"Payload\s*:\s*null"); + + var page = new ReplPage( + [new NullableHolder("a", Payload: null), new NullableHolder("b", new JsonObject { ["x"] = 1 })], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + var rowA = (await RenderAsync(page)).Split(Environment.NewLine).Single(line => line.StartsWith('a')); + rowA.TrimEnd().Should().EndWith("null"); + } + + [TestMethod] + [Description("An explicit NullDisplayText still wins over the JSON null of a property declared as JSON.")] + public async Task When_AJsonTypedPropertyHasANullDisplayText_Then_ItIsUsed() + { + (await RenderAsync(new DisplayedHolder(Payload: null))).Should().MatchRegex(@"Payload\s*:\s*\(none\)"); + } + + [TestMethod] + [Description("With ANSI on, the table header is styled bold within a combined sequence; the pager must still recognize it, or every JSON page, which carries its own header, would repeat it.")] + public async Task When_AnAnsiPagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded() + { + var transformer = new HumanOutputTransformer( + () => new HumanRenderSettings(Width: 120, UseAnsi: true, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + var first = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None); + var next = await transformer.TransformPageAsync( + SingleRowPage(new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None); + + var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); + session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + + session.HeaderLines.Should().ContainSingle("the styled header line is the pinned header"); + session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); + } + + private sealed record NullableHolder(string Name, JsonNode? Payload); + + private sealed record DisplayedHolder( + [property: System.ComponentModel.DataAnnotations.DisplayFormat(NullDisplayText = "(none)")] JsonNode? Payload); + private sealed record Holder(string Name, JsonObject Payload); private sealed record Owner(string Login); diff --git a/src/Repl.Tests/Given_ResultFlowPager.cs b/src/Repl.Tests/Given_ResultFlowPager.cs index 7c238e97..f866e513 100644 --- a/src/Repl.Tests/Given_ResultFlowPager.cs +++ b/src/Repl.Tests/Given_ResultFlowPager.cs @@ -1114,6 +1114,59 @@ public void When_AWholeLineBoldHeaderRepeatsAtOtherWidths_Then_ItIsDropped() second.ContentLines.Should().Equal("1000 b"); } + [TestMethod] + [Description("Bold within a combined SGR sequence, as the human palette styles its table header, marks a header line too.")] + public void When_TheFirstLineIsBoldWithinACombinedSequence_Then_ItIsTheHeader() + { + var parsed = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, $"{(char)27}[1;38;5;221mid name{(char)27}[0m", "1 a"), header: null); + + parsed.Header.Lines.Should().ContainSingle(); + parsed.ContentLines.Should().Equal("1 a"); + } + + [TestMethod] + [Description("A 1 that is an extended colour's argument, as in 38;5;1, is a colour, not bold: that line is content.")] + public void When_TheFirstLineUsesColourIndexOne_Then_ItIsNotAHeader() + { + var colored = $"{(char)27}[38;5;1mred{(char)27}[0m"; + var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, colored, "next"), header: null); + + parsed.Header.Lines.Should().BeEmpty(); + parsed.ContentLines.Should().Equal(colored, "next"); + } + + [TestMethod] + [Description("A single payload, such as a long string result, keeps a line identical to its first one: only a continuation page repeats a header.")] + public void When_APayloadRepeatsItsBoldFirstLine_Then_BothAreKept() + { + var title = $"{(char)27}[1;31mSection{(char)27}[0m"; + var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, title, "one", title, "two"), header: null); + + parsed.ContentLines.Should().Equal(["one", title, "two"], "the first line may be pinned, but its repeat inside the payload is data"); + } + + [TestMethod] + [Description("A first line holding only a style opener has no header text; taken as a header, it would drop every blank line after it.")] + public void When_TheFirstLineIsOnlyAStyleOpener_Then_BlankLinesAreKept() + { + var parsed = PagerPayloadParser.Parse( + string.Join(Environment.NewLine, $"{(char)27}[1;37m", "one", string.Empty, "two"), header: null); + + parsed.Header.Lines.Should().BeEmpty(); + parsed.ContentLines.Should().Contain(string.Empty); + } + + [TestMethod] + [Description("Bold that only starts inside the line, after other text, does not style a header: that line is content.")] + public void When_ALineIsBoldOnlyAfterOtherText_Then_ItIsNotAHeader() + { + var line = $"note: {(char)27}[1;31mX{(char)27}[0m"; + var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, line, "next"), header: null); + + parsed.Header.Lines.Should().BeEmpty(); + } + private static string Bold(string text) => $"{(char)27}[1m{text}{(char)27}[0m"; [TestMethod] From 1297d28e88b9929d3ec49f6b95d8e1738062d8f7 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 24 Sep 2026 14:33:11 -0400 Subject: [PATCH 6/7] refactor(output): declare the pager layout instead of inferring headers from styling Four review waves on #111 kept finding cases where the pager's style-based header inference failed on JSON tables, whose columns come from each page's own keys: - plain Spectre headers could not be detected; - keys containing spaces or runs of spaces compared as equal; - data rows that read like the header were dropped; - a trailing null row was trimmed away. Rather than add heuristics, human output transformers now declare what they render. IResultFlowOutputTransformer (internal) gains RenderAsync(value) and RenderPageAsync(page). Both return a RenderedPayload(Text, Layout), where RenderedLayout declares: - how many of the payload's first lines are its header; - the columns that header names; - how many of its last lines are the footer asking to rerun for more. TransformAsync is RenderAsync(...).Text. The layout is validated: no negative counts, and a header exactly when there are columns. CoreReplApp passes the layout to the pager on both paths, a long single result and an IReplPageSource. PagerSession picks declared or detected once, for the whole session. PagerPayloadParser.ParseDeclared takes exactly the declared lines: no styling is read, and no line is dropped for reading like a header or a footer. A fetched page's header is dropped when it names the previous page's columns, and kept when it names others, so the sequence A, B, A shows each header. The first page's header stays pinned. Tables now render their header on every page, typed CLR tables included, and the pager owns dropping repeats; ResultFlowPageRenderMode is gone. Some consequences: - A continuation page of rows of another type now gets its own header instead of sitting under the first type's columns. - Custom IReplPagerRenderers still receive text only; a declared header repeating the previous page's columns is stripped for them. - A long single result keeps its rerun footer in view, since that pager cannot fetch the next page. The previous inference stripped it, losing the cursor. Headers are declared one line tall, so they must render on one line: - Human: TextTableFormatter never wraps. Labels go through ToSingleLine, which turns control characters and U+2028/U+2029 into spaces, since the pager splits at every line break. - Spectre: table headers use SingleLineLabel, which truncates with "..." rather than wraps. This applies to every Spectre data table. - Spectre composes an IReplResult's page details as text rather than as a Text renderable. As a Text renderable, the lines were wrapped a second time and the footer could split in two. JSON tables require every row to be an object with at least one key. A null or {} row would render blank, reading as no row and, as the last one, trimmed away; such pages render literals. Spectre uses the shared ResultFlowPageFooterBuilder instead of its own copy, since the declared footer height depends on it. The earlier heuristics in PagerPayloadParser and AnsiTextMetrics are reverted to main; the detected path is unchanged for third-party transformers. Tests: - Parser: previous-page comparison, declared footer, a header taller than the payload, layout validation. - Transformers: A, B, A; a first page with no header; rows of another type; a line feed and U+2028 in a display name; {} rows; a narrow Spectre table; a result with page details, including a long cursor with ANSI in Spectre. - Custom pager renderer. - Three integration tests through CoreReplApp: a JSON page source whose keys change, a JSON array whose rows read like its header, and a long page returned whole. Falsified in rounds, each with compiling substitutions that turned their targeted tests red: - dropping the layout from the initial or the fetched pages in CoreReplApp; - comparing with the first page rather than the previous one; - the {} rule, the footer declarations, ToSingleLine including U+2028; - the Spectre result-details composition, the custom-renderer stripping, the kept footer. Refs #92 --- docs/output-system.md | 13 +- docs/result-flow.md | 22 ++ src/Repl.Core/CoreReplApp.Execution.cs | 63 +++-- src/Repl.Core/IResultFlowOutputTransformer.cs | 18 +- .../Output/HumanOutputTransformer.cs | 218 ++++++++---------- src/Repl.Core/Output/JsonHumanShape.cs | 43 ++-- src/Repl.Core/Output/RenderedLayout.cs | 50 ++++ src/Repl.Core/Output/RenderedPayload.cs | 16 ++ src/Repl.Core/Output/TextTableFormatter.cs | 39 ++++ .../ResultFlow/PagerPayloadParser.cs | 181 ++++----------- src/Repl.Core/ResultFlow/PagerSession.cs | 57 ++++- src/Repl.Core/ResultFlow/ResultFlowPager.cs | 35 ++- .../ResultFlow/ResultFlowPagerOptions.cs | 6 + .../ResultFlow/ResultFlowPagerPage.cs | 10 +- src/Repl.Core/ResultFlowPageRenderMode.cs | 7 - src/Repl.Core/Terminal/AnsiTextMetrics.cs | 33 --- .../Given_OutputFormatting.cs | 82 +++++++ src/Repl.Spectre/SingleLineLabel.cs | 51 ++++ .../SpectreHumanOutputTransformer.cs | 203 +++++++--------- .../Given_SpectreHumanOutputJson.cs | 131 +++++++---- src/Repl.Tests/Given_HumanOutputJson.cs | 198 +++++++++++++--- .../Given_ResultFlowOutputTransformer.cs | 56 ++--- src/Repl.Tests/Given_ResultFlowPager.cs | 195 ++++++++-------- 23 files changed, 1030 insertions(+), 697 deletions(-) create mode 100644 src/Repl.Core/Output/RenderedLayout.cs create mode 100644 src/Repl.Core/Output/RenderedPayload.cs delete mode 100644 src/Repl.Core/ResultFlowPageRenderMode.cs create mode 100644 src/Repl.Spectre/SingleLineLabel.cs diff --git a/docs/output-system.md b/docs/output-system.md index 350407b5..862f629e 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -30,11 +30,14 @@ A handler can return `System.Text.Json.Nodes.JsonNode` (`JsonObject`, `JsonArray data rather than the CLR members of those types: - An object becomes one `key: value` line per field. -- Rows of objects become a table. Its columns are the union of the rows' keys in first-seen order, matched - ordinally, and a key missing from a row leaves that cell empty. The rows' keys, not a type, define the - columns. So when a page the pager fetches has other columns than the first page, it keeps its own header - row, while a header that repeats the first page's is still dropped. -- An array of scalars becomes one value per line. So do rows that have no keys at all: each reads `{}`. +- In `spectre` as in `human`, a table's header labels stay on one line: a label wider than its column is + truncated rather than wrapped, and a line break in a label reads as a space. +- Rows that are all objects with at least one key become a table. Its columns are the union of the rows' + keys in first-seen order, matched ordinally, and a key missing from a row leaves that cell empty. The rows' + keys, not a type, define the columns, so each page the pager fetches can name other columns than the page + before it. See [Result Flow And Paging](result-flow.md) for how the pager shows their headers. +- An array of scalars becomes one value per line. So does a page of rows among which one is a JSON null or + an empty object: each row reads as its literal, since a blank table row would read as no row at all. - A page declared with a JSON item type, such as `IReplPageSource`, renders as JSON even when every item on it is a JSON null. - Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and diff --git a/docs/result-flow.md b/docs/result-flow.md index 7a3c4ab8..d183ec24 100644 --- a/docs/result-flow.md +++ b/docs/result-flow.md @@ -523,6 +523,28 @@ The full viewport is inspired by `less`: it does not depend on terminal scrollback. It renders from an internal buffer and fetches additional `IReplPageSource` payloads as the user pages past the buffered end. +The pager pins the first page's table header. `human` and `spectre` render a +table's header on every page and tell the pager exactly which lines hold the +header and which columns it names, whether the pager shows a result that is +simply long or pages an `IReplPageSource`. A long result keeps its footer +asking to rerun for the next page, since that pager cannot fetch it. A fetched page whose header names +the columns the page before it showed adds only its rows. A page naming other +columns keeps its own header, since its rows would otherwise sit under headings +that are not theirs. That can happen with JSON rows, whose columns come from +their keys, or with pages of rows of different types. In `inline` and `full`, +the header at the top of the viewport stays the first page's, and a later +header scrolls with its rows. + +A custom `IReplPagerRenderer` receives each fetched page as text: a header +naming the previous page's columns is stripped from it first, and one naming +other columns is kept. + +For any other transformer, the pager recognizes a header from the text: a +separator line under the first line, a first line starting with `#` and a +space, or bold styling. It drops that header, and any line that reads like it, +from every fetched page, and drops lines that read like the rerun footer from +the first payload. + Applications that need a different terminal experience can register a custom `IReplPagerRenderer` with `options.Output.ResultFlow.UsePagerRenderer(renderer)`. A custom renderer is diff --git a/src/Repl.Core/CoreReplApp.Execution.cs b/src/Repl.Core/CoreReplApp.Execution.cs index dfcd7bf8..1dd097d0 100644 --- a/src/Repl.Core/CoreReplApp.Execution.cs +++ b/src/Repl.Core/CoreReplApp.Execution.cs @@ -1286,11 +1286,11 @@ internal async ValueTask RenderOutputAsync( .ConfigureAwait(false); } - var payload = await transformer.TransformAsync(result, cancellationToken).ConfigureAwait(false); - payload = TryColorizeStructuredPayload(payload, format, isInteractive); + var (rendered, layout) = await RenderPayloadAsync(transformer, result, cancellationToken).ConfigureAwait(false); + var payload = TryColorizeStructuredPayload(rendered, format, isInteractive); if (!string.IsNullOrEmpty(payload)) { - await WritePayloadAsync(payload, transformer, resultFlow, cancellationToken).ConfigureAwait(false); + await WritePayloadAsync(payload, layout, transformer, resultFlow, cancellationToken).ConfigureAwait(false); } return true; @@ -1305,11 +1305,12 @@ private async ValueTask RenderPageSourceAsync( { var request = CreatePageSourceRequest(resultFlow); var page = await FetchPageSourceAsync(source, request, cancellationToken).ConfigureAwait(false); - var payload = await transformer.TransformAsync(page, cancellationToken).ConfigureAwait(false); - payload = TryColorizeStructuredPayload(payload, transformer.Name, isInteractive); + var (rendered, layout) = await RenderPayloadAsync(transformer, page, cancellationToken).ConfigureAwait(false); + var payload = TryColorizeStructuredPayload(rendered, transformer.Name, isInteractive); if (!TryCreatePager( payload, + layout, transformer, resultFlow, page.PageInfo.HasMore, @@ -1358,9 +1359,9 @@ private async ValueTask RenderPageSourcePagerAsync( CancellationToken cancellationToken) { var nextCursor = page.PageInfo.NextCursor; - var pagerPayload = await TransformPagerPageAsync(transformer, page, ResultFlowPageRenderMode.Initial, cancellationToken) + var (initialPayload, initialLayout) = await RenderPagerPageAsync(transformer, page, cancellationToken) .ConfigureAwait(false); - pagerPayload = TryColorizeStructuredPayload(pagerPayload, transformer.Name, isInteractive); + var pagerPayload = TryColorizeStructuredPayload(initialPayload, transformer.Name, isInteractive); await ResultFlowPager.WriteAsync( pagerPayload, ReplSessionIO.Output, @@ -1372,6 +1373,7 @@ await ResultFlowPager.WriteAsync( PagerMode = pagerMode, AnsiEnabled = ansiEnabled, HasMorePayload = page.PageInfo.HasMore, + PayloadLayout = initialLayout, FetchNextPayload = FetchNextPayloadAsync, PagerRenderers = _options.Output.ResultFlow.PagerRenderers, MaxBufferedLines = _options.Output.ResultFlow.MaxBufferedLines, @@ -1390,13 +1392,13 @@ await ResultFlowPager.WriteAsync( var nextRequest = request with { Cursor = nextCursor }; var nextPage = await FetchPageSourceAsync(source, nextRequest, token).ConfigureAwait(false); nextCursor = nextPage.PageInfo.NextCursor; - var nextPayload = await TransformPagerPageAsync(transformer, nextPage, ResultFlowPageRenderMode.Continuation, token) + var (nextPayload, nextLayout) = await RenderPagerPageAsync(transformer, nextPage, token) .ConfigureAwait(false); - nextPayload = TryColorizeStructuredPayload(nextPayload, transformer.Name, isInteractive); return new ResultFlowPagerPage( - nextPayload, + TryColorizeStructuredPayload(nextPayload, transformer.Name, isInteractive), nextPage.PageInfo.HasMore, - ContainsPresentationChrome: false); + ContainsPresentationChrome: false, + nextLayout); } } @@ -1412,26 +1414,47 @@ await ResultFlowPager.WriteAsync( return await RefuseGlobalOptionErrorsAsync(globalOptions, cancellationToken).ConfigureAwait(false); } - private static ValueTask TransformPagerPageAsync( + // A result-flow transformer declares the layout it renders; for any other, the pager detects it from the text. + private static async ValueTask<(string Payload, RenderedLayout? Layout)> RenderPayloadAsync( + IOutputTransformer transformer, + object? value, + CancellationToken cancellationToken) + { + if (transformer is IResultFlowOutputTransformer resultFlowTransformer) + { + var rendered = await resultFlowTransformer.RenderAsync(value, cancellationToken).ConfigureAwait(false); + return (rendered.Text, rendered.Layout); + } + + return (await transformer.TransformAsync(value, cancellationToken).ConfigureAwait(false), null); + } + + private static async ValueTask<(string Payload, RenderedLayout? Layout)> RenderPagerPageAsync( IOutputTransformer transformer, IReplPage page, - ResultFlowPageRenderMode mode, CancellationToken cancellationToken) { var displayPage = CreatePagerDisplayPage(page); - return transformer is IResultFlowOutputTransformer resultFlowTransformer - ? resultFlowTransformer.TransformPageAsync(displayPage, mode, cancellationToken) - : transformer.TransformAsync(displayPage, cancellationToken); + if (transformer is IResultFlowOutputTransformer resultFlowTransformer) + { + var rendered = await resultFlowTransformer.RenderPageAsync(displayPage, cancellationToken) + .ConfigureAwait(false); + return (rendered.Text, rendered.Layout); + } + + return (await transformer.TransformAsync(displayPage, cancellationToken).ConfigureAwait(false), null); } private async ValueTask WritePayloadAsync( string payload, + RenderedLayout? layout, IOutputTransformer transformer, ResultFlowInvocationOptions? resultFlow, CancellationToken cancellationToken) { if (TryCreatePager( payload, + layout?.WithFooter(0), transformer, resultFlow, out var keyReader, @@ -1448,6 +1471,9 @@ await ResultFlowPager.WriteAsync( VisibleRows = visibleRows, PagerMode = pagerMode, AnsiEnabled = ansiEnabled, + // This pager cannot fetch more, so a footer asking to rerun for the next page is the only way + // to continue, and stays in view instead of being stripped. + PayloadLayout = layout?.WithFooter(0), PagerRenderers = _options.Output.ResultFlow.PagerRenderers, MaxBufferedLines = _options.Output.ResultFlow.MaxBufferedLines, }, @@ -1461,6 +1487,7 @@ await ResultFlowPager.WriteAsync( private bool TryCreatePager( string payload, + RenderedLayout? layout, IOutputTransformer transformer, ResultFlowInvocationOptions? resultFlow, [NotNullWhen(true)] out IReplKeyReader? keyReader, @@ -1469,6 +1496,7 @@ private bool TryCreatePager( out bool ansiEnabled) => TryCreatePager( payload, + layout, transformer, resultFlow, hasMorePayload: false, @@ -1479,6 +1507,7 @@ private bool TryCreatePager( private bool TryCreatePager( string payload, + RenderedLayout? layout, IOutputTransformer transformer, ResultFlowInvocationOptions? resultFlow, bool hasMorePayload, @@ -1501,7 +1530,7 @@ private bool TryCreatePager( } if (!TryResolvePagerVisibleRows(out visibleRows) - || (!hasMorePayload && ResultFlowPager.CountLines(payload) <= visibleRows) + || (!hasMorePayload && ResultFlowPager.CountLines(payload, layout) <= visibleRows) || !TryResolvePagerKeyReader(out keyReader)) { return false; diff --git a/src/Repl.Core/IResultFlowOutputTransformer.cs b/src/Repl.Core/IResultFlowOutputTransformer.cs index 093d790d..084ff593 100644 --- a/src/Repl.Core/IResultFlowOutputTransformer.cs +++ b/src/Repl.Core/IResultFlowOutputTransformer.cs @@ -1,9 +1,19 @@ namespace Repl; +/// +/// A human output transformer that declares the layout of what it renders, so the pager can pin its header, drop +/// that header's repeats and strip its footer without inferring any of them from the text. +/// +/// +/// A table always renders its header, on every page: a page of JSON rows takes its columns from its own keys, so it +/// can name other columns than the page before it. The pager drops a header that repeats the previous page's +/// columns and keeps one that names others. +/// internal interface IResultFlowOutputTransformer : IOutputTransformer { - ValueTask TransformPageAsync( - IReplPage page, - ResultFlowPageRenderMode mode, - CancellationToken cancellationToken = default); + /// Renders as does, with its layout. + ValueTask RenderAsync(object? value, CancellationToken cancellationToken = default); + + /// Renders a page fetched for the pager: its items, without the footer that asks to rerun for more. + ValueTask RenderPageAsync(IReplPage page, CancellationToken cancellationToken = default); } diff --git a/src/Repl.Core/Output/HumanOutputTransformer.cs b/src/Repl.Core/Output/HumanOutputTransformer.cs index 4cbd0d95..cbc281e0 100644 --- a/src/Repl.Core/Output/HumanOutputTransformer.cs +++ b/src/Repl.Core/Output/HumanOutputTransformer.cs @@ -31,83 +31,44 @@ public HumanOutputTransformer(Func resolveRenderSettings) public ValueTask TransformAsync(object? value, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); - var settings = _resolveRenderSettings(); - - if (value is null) - { - return ValueTask.FromResult(string.Empty); - } - - if (value is IReplPage page) - { - return ValueTask.FromResult(RenderPage(page, settings)); - } - - if (value is IReplResult replResult) - { - return ValueTask.FromResult(RenderReplResult(replResult, settings)); - } - - if (value is string text) - { - return ValueTask.FromResult(text); - } - - // Before the enumerable branch: a JsonObject enumerates as key/value pairs, and reflecting over - // those would show JsonNode's CLR members instead of the data. - if (JsonHumanShape.TryGetNode(value, out var node)) - { - return ValueTask.FromResult(RenderJson(node, settings)); - } - - if (value is System.Collections.IEnumerable enumerable) - { - return ValueTask.FromResult(RenderTopLevelEnumerable(enumerable, settings)); - } - - if (TryRenderObject(value, settings, out var objectText)) - { - return ValueTask.FromResult(objectText); - } + return ValueTask.FromResult(Render(value, _resolveRenderSettings()).Text); + } - return ValueTask.FromResult( - Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty); + public ValueTask RenderAsync(object? value, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + return ValueTask.FromResult(Render(value, _resolveRenderSettings())); } - public ValueTask TransformPageAsync( - IReplPage page, - ResultFlowPageRenderMode mode, - CancellationToken cancellationToken = default) + public ValueTask RenderPageAsync(IReplPage page, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(page); cancellationToken.ThrowIfCancellationRequested(); - return ValueTask.FromResult(RenderPage(page, _resolveRenderSettings(), mode)); + return ValueTask.FromResult(RenderPageBody(page, _resolveRenderSettings())); } - private static string RenderPage(IReplPage page, HumanRenderSettings settings) => - RenderPage(page, settings, ResultFlowPageRenderMode.Initial, includeFooter: true); - - private static string RenderPage(IReplPage page, HumanRenderSettings settings, ResultFlowPageRenderMode mode) => - RenderPage(page, settings, mode, includeFooter: false); - - private static string RenderPage( - IReplPage page, - HumanRenderSettings settings, - ResultFlowPageRenderMode mode, - bool includeFooter) + private static RenderedPayload Render(object? value, HumanRenderSettings settings) => value switch { - var body = RenderPageBody(page, settings, mode); - var footer = includeFooter ? ResultFlowPageFooterBuilder.RenderHuman(page) : string.Empty; - return string.IsNullOrWhiteSpace(footer) - ? body - : string.Concat(body, Environment.NewLine, footer); - } + null => RenderedPayload.Plain(string.Empty), + IReplPage page => RenderPageWithFooter(page, settings), + IReplResult replResult => RenderReplResult(replResult, settings), + string text => RenderedPayload.Plain(text), + // Before the enumerable arm: a JsonObject enumerates as key/value pairs, and reflecting over those would + // show JsonNode's CLR members instead of the data. + _ when JsonHumanShape.TryGetNode(value, out var node) => RenderJson(node, settings), + System.Collections.IEnumerable enumerable => RenderTopLevelEnumerable(enumerable, settings), + _ when TryRenderObject(value, settings, out var objectText) => RenderedPayload.Plain(objectText), + _ => RenderedPayload.Plain(Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty), + }; + + private static RenderedPayload RenderPageWithFooter(IReplPage page, HumanRenderSettings settings) => + RenderPageBody(page, settings).WithFooterLine(ResultFlowPageFooterBuilder.RenderHuman(page)); - private static string RenderPageBody(IReplPage page, HumanRenderSettings settings, ResultFlowPageRenderMode mode) + private static RenderedPayload RenderPageBody(IReplPage page, HumanRenderSettings settings) { if (page.UntypedItems.Count == 0) { - return "No results."; + return RenderedPayload.Plain("No results."); } // By its declared item type: a page of JSON nulls has no item to be recognized by. @@ -116,26 +77,24 @@ private static string RenderPageBody(IReplPage page, HumanRenderSettings setting return RenderJsonItems(page.UntypedItems, settings); } - return RenderCollection( - page.UntypedItems, - depth: 0, - settings, - includeTableHeader: mode == ResultFlowPageRenderMode.Initial); + return RenderItems([.. page.UntypedItems], depth: 0, settings); } - private static string RenderTopLevelEnumerable(System.Collections.IEnumerable enumerable, HumanRenderSettings settings) + private static RenderedPayload RenderTopLevelEnumerable( + System.Collections.IEnumerable enumerable, + HumanRenderSettings settings) { var lines = enumerable .Cast() .ToArray(); if (lines.Length == 0) { - return "No results."; + return RenderedPayload.Plain("No results."); } - if (TryRenderTable(lines, settings, includeHeader: true, out var tableText)) + if (TryRenderTable(lines, settings, out var table)) { - return tableText; + return table; } var scalarLines = lines @@ -143,35 +102,36 @@ private static string RenderTopLevelEnumerable(System.Collections.IEnumerable en .Where(item => !string.IsNullOrWhiteSpace(item)) .ToArray(); - return scalarLines.Length == 0 ? "No results." : string.Join(Environment.NewLine, scalarLines); + return RenderedPayload.Plain(scalarLines.Length == 0 ? "No results." : string.Join(Environment.NewLine, scalarLines)); } - private static string RenderJson(JsonNode? node, HumanRenderSettings settings) => node switch + private static RenderedPayload RenderJson(JsonNode? node, HumanRenderSettings settings) => node switch { - JsonObject jsonObject => RenderJsonObject(jsonObject, settings), - JsonArray { Count: 0 } => "No results.", + JsonObject jsonObject => RenderedPayload.Plain(RenderJsonObject(jsonObject, settings)), + JsonArray { Count: 0 } => RenderedPayload.Plain("No results."), // Its own path rather than the generic collection one, which recognizes JSON by its first non-null // item and so has nothing to go on for an array of nulls. JsonArray jsonArray => RenderJsonItems([.. jsonArray], settings), - _ => JsonHumanShape.Literal(node), + _ => RenderedPayload.Plain(JsonHumanShape.Literal(node)), }; - // A JSON table always carries its header, continuation pages included: its columns come from its own rows' - // keys rather than from a type, so another page's headings could mislabel its cells. - private static string RenderJsonItems(IReadOnlyList items, HumanRenderSettings settings) + // The table's columns come from its own rows' keys rather than from a type, so a later page can name others; + // declaring those keys lets the pager tell a repeated header from one it must keep. + private static RenderedPayload RenderJsonItems(IReadOnlyList items, HumanRenderSettings settings) { if (!JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows)) { - return string.Join( + var literals = string.Join( Environment.NewLine, items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) ? literal : RenderScalar(item, member: null, depth: 0, compactCollection: true, settings.Width, settings))); + return RenderedPayload.Plain(literals); } var tableRows = new List(rows.Length + 1) { columns.Select(JsonHumanShape.Label).ToArray() }; tableRows.AddRange(rows.Select(row => columns.Select(column => JsonHumanShape.Cell(row, column)).ToArray())); - return FormatTable(tableRows, settings, includeHeader: true); + return FormatTable(tableRows, settings, columns); } private static string RenderJsonObject(JsonObject jsonObject, HumanRenderSettings settings) @@ -248,87 +208,89 @@ private static bool TryRenderObject(object value, HumanRenderSettings settings, private static string RenderCollection( System.Collections.IEnumerable collection, int depth, - HumanRenderSettings settings, - bool includeTableHeader = true) + HumanRenderSettings settings) => + RenderItems([.. collection.Cast()], depth, settings).Text; + + private static RenderedPayload RenderItems(object?[] values, int depth, HumanRenderSettings settings) { - var values = collection.Cast().ToArray(); if (values.Length == 0) { - return string.Empty; + return RenderedPayload.Plain(string.Empty); } - if (TryRenderTable(values, settings, includeTableHeader, out var tableText)) + if (TryRenderTable(values, settings, out var table)) { - return tableText; + return table; } - return string.Join( + var list = string.Join( Environment.NewLine, values.Select(value => $"- {RenderScalar(value, member: null, depth, compactCollection: false, settings.Width, settings)}")); + return RenderedPayload.Plain(list); } private static bool TryRenderTable( object?[] values, HumanRenderSettings settings, - bool includeHeader, - out string text) + [NotNullWhen(true)] out RenderedPayload? rendered) { var firstNonNull = values.FirstOrDefault(value => value is not null); if (firstNonNull is null) { - text = string.Empty; + rendered = null; return false; } // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. if (JsonHumanShape.IsJson(firstNonNull)) { - text = RenderJsonItems(values, settings); + rendered = RenderJsonItems(values, settings); return true; } if (IsSimpleValue(firstNonNull.GetType())) { - text = string.Join( + var lines = string.Join( Environment.NewLine, values.Select(value => RenderScalar(value, member: null, depth: 0, compactCollection: true, settings.Width, settings))); + rendered = RenderedPayload.Plain(lines); return true; } var members = GetDisplayMembers(firstNonNull.GetType()); if (members.Length == 0) { - text = string.Empty; + rendered = null; return false; } - text = FormatTable(BuildTableRows(values, members, settings, includeHeader), settings, includeHeader); + var rows = BuildTableRows(values, members, settings); + rendered = FormatTable(rows, settings, columns: rows[0]); return true; } - private static string FormatTable(List rows, HumanRenderSettings settings, bool includeHeader) + // The first row is the header. TextTableFormatter writes it on one line (it truncates a cell, never wraps it) + // and, without ANSI styling to set it apart, a separator line under it: the layout declares exactly that. + private static RenderedPayload FormatTable( + List rows, + HumanRenderSettings settings, + IReadOnlyList columns) { - var style = includeHeader && settings.UseAnsi - ? TextTableStyle.ForHeader(settings.Palette.TableHeaderStyle) - : TextTableStyle.None; - return TextTableFormatter.FormatRows( + var separated = !settings.UseAnsi; + var text = TextTableFormatter.FormatRows( rows, settings.Width, - includeHeaderSeparator: includeHeader && !settings.UseAnsi, - style); + includeHeaderSeparator: separated, + separated ? TextTableStyle.None : TextTableStyle.ForHeader(settings.Palette.TableHeaderStyle)); + return new RenderedPayload(text, new RenderedLayout(separated ? 2 : 1, columns)); } private static List BuildTableRows( object?[] values, DisplayMember[] members, - HumanRenderSettings settings, - bool includeHeader) + HumanRenderSettings settings) { - var rows = new List(values.Length + (includeHeader ? 1 : 0)); - if (includeHeader) - { - rows.Add(members.Select(member => member.Label).ToArray()); - } + var rows = new List(values.Length + 1) { members.Select(member => member.Label).ToArray() }; foreach (var item in values) { @@ -474,7 +436,8 @@ private static DisplayMember[] GetDisplayMembers( var displayFormat = property.GetCustomAttribute(); return new DisplayMember( property, - string.IsNullOrWhiteSpace(display?.GetName()) ? property.Name : display!.GetName()!, + TextTableFormatter.ToSingleLine( + string.IsNullOrWhiteSpace(display?.GetName()) ? property.Name : display!.GetName()!), display?.GetOrder(), displayFormat?.NullDisplayText ?? JsonHumanShape.NullText(property.PropertyType)); }) @@ -484,13 +447,27 @@ private static DisplayMember[] GetDisplayMembers( .ThenBy(member => member.Property.MetadataToken) .ToArray(); - private sealed record DisplayMember( + private sealed record DisplayMember( PropertyInfo Property, string Label, int? Order, string? NullDisplayText); - private static string RenderReplResult(IReplResult result, HumanRenderSettings settings) + private static RenderedPayload RenderReplResult(IReplResult result, HumanRenderSettings settings) + { + // The message comes first, so no header starts the payload; a page's footer still ends it. + if (result.Details is IReplPage page) + { + var rendered = RenderPageWithFooter(page, settings); + return new RenderedPayload( + $"{DescribeResult(result)}{Environment.NewLine}{rendered.Text}", + RenderedLayout.None.WithFooter(rendered.Layout.FooterLineCount)); + } + + return RenderedPayload.Plain(RenderReplResultText(result, settings)); + } + + private static string DescribeResult(IReplResult result) { var prefix = result.Kind.ToLowerInvariant() switch { @@ -503,23 +480,22 @@ private static string RenderReplResult(IReplResult result, HumanRenderSettings s _ => "Result", }; - var message = string.IsNullOrWhiteSpace(prefix) + return string.IsNullOrWhiteSpace(prefix) ? result.Message : $"{prefix}: {result.Message}"; + } + private static string RenderReplResultText(IReplResult result, HumanRenderSettings settings) + { + var message = DescribeResult(result); if (result.Details is null) { return message; } - if (result.Details is IReplPage page) - { - return $"{message}{Environment.NewLine}{RenderPage(page, settings)}"; - } - if (JsonHumanShape.TryGetNode(result.Details, out var jsonDetails)) { - return $"{message}{Environment.NewLine}{RenderJson(jsonDetails, settings)}"; + return $"{message}{Environment.NewLine}{RenderJson(jsonDetails, settings).Text}"; } if (TryRenderDictionary(result.Details, settings, out var dictionaryText)) diff --git a/src/Repl.Core/Output/JsonHumanShape.cs b/src/Repl.Core/Output/JsonHumanShape.cs index 53afc874..c1a45273 100644 --- a/src/Repl.Core/Output/JsonHumanShape.cs +++ b/src/Repl.Core/Output/JsonHumanShape.cs @@ -120,40 +120,28 @@ public static string Label(string key) => /// /// Reads as rows of JSON objects. The columns are the union of their keys in /// first-seen order, so a key missing from one row leaves its cell empty rather than dropping the row. - /// A JSON null, as a value or a null , is an empty row. Fails - /// if any other value is not a JSON object, or if no row has a key: a table with no columns would show - /// nothing of them. + /// Fails unless every value is a JSON object with at least one key. A JSON null or an empty object would + /// render as a blank row, which reads as no row at all and, as the last one, is trimmed away with the payload's + /// trailing whitespace; those rows then read as their literals instead. /// public static bool TryGetObjectRows( IReadOnlyList values, [NotNullWhen(true)] out string[]? columns, - [NotNullWhen(true)] out JsonObject?[]? rows) + [NotNullWhen(true)] out JsonObject[]? rows) { columns = null; rows = null; - var converted = new JsonObject?[values.Count]; + if (values.Count == 0) + { + return false; + } + + var converted = new JsonObject[values.Count]; var seen = new HashSet(StringComparer.Ordinal); var ordered = new List(); for (var i = 0; i < values.Count; i++) { - if (values[i] is null) - { - continue; - } - - if (!TryGetNode(values[i], out var node)) - { - return false; - } - - // A JsonElement of kind Null (or Undefined, a default element) is no data either: an empty row, like - // a CLR null. - if (node is null) - { - continue; - } - - if (node is not JsonObject row) + if (!TryGetNode(values[i], out var node) || node is not JsonObject { Count: > 0 } row) { return false; } @@ -168,11 +156,6 @@ public static bool TryGetObjectRows( } } - if (ordered.Count == 0) - { - return false; - } - columns = [.. ordered]; rows = converted; return true; @@ -182,9 +165,9 @@ public static bool TryGetObjectRows( /// The cell for : empty when the row does not have the key. Matched ordinally, the /// way the columns were collected, even in a row built with case-insensitive property names. /// - public static string Cell(JsonObject? row, string column) + public static string Cell(JsonObject row, string column) { - if (row is null || row.IndexOf(column) is not (>= 0 and var index)) + if (row.IndexOf(column) is not (>= 0 and var index)) { return string.Empty; } diff --git a/src/Repl.Core/Output/RenderedLayout.cs b/src/Repl.Core/Output/RenderedLayout.cs new file mode 100644 index 00000000..b2261e9c --- /dev/null +++ b/src/Repl.Core/Output/RenderedLayout.cs @@ -0,0 +1,50 @@ +namespace Repl; + +/// +/// What a result-flow transformer rendered around a payload's data: the header its first +/// lines hold, the columns that header names, and the footer its last +/// lines hold. The pager takes these lines as declared rather than inferring them +/// from the text's styling. +/// +internal sealed class RenderedLayout +{ + /// How many of the payload's first lines are its header, separator included. + /// The columns the header names, in order, as the transformer keys them. + /// How many of the payload's last lines are its footer. + public RenderedLayout(int headerLineCount, IReadOnlyList columns, int footerLineCount = 0) + { + ArgumentOutOfRangeException.ThrowIfNegative(headerLineCount); + ArgumentNullException.ThrowIfNull(columns); + ArgumentOutOfRangeException.ThrowIfNegative(footerLineCount); + if (headerLineCount == 0 != (columns.Count == 0)) + { + throw new ArgumentException("A header names at least one column, and only a header names columns.", nameof(columns)); + } + + HeaderLineCount = headerLineCount; + Columns = columns; + FooterLineCount = footerLineCount; + } + + /// A payload rendered with neither a header nor a footer, such as a list or an object's fields. + public static RenderedLayout None { get; } = new(headerLineCount: 0, columns: []); + + public int HeaderLineCount { get; } + + public IReadOnlyList Columns { get; } + + public int FooterLineCount { get; } + + /// + /// Whether this layout's header names the same columns as 's, compared ordinally: the + /// pager drops a fetched page's header only when it repeats the columns the previous page showed. + /// + public bool NamesSameColumns(RenderedLayout other) + { + ArgumentNullException.ThrowIfNull(other); + return HeaderLineCount > 0 && Columns.SequenceEqual(other.Columns, StringComparer.Ordinal); + } + + /// This layout with a footer of lines appended below the data. + public RenderedLayout WithFooter(int footerLineCount) => new(HeaderLineCount, Columns, footerLineCount); +} diff --git a/src/Repl.Core/Output/RenderedPayload.cs b/src/Repl.Core/Output/RenderedPayload.cs new file mode 100644 index 00000000..65dad45c --- /dev/null +++ b/src/Repl.Core/Output/RenderedPayload.cs @@ -0,0 +1,16 @@ +namespace Repl; + +/// A result-flow transformer's rendering, with the layout it declares for the pager. +/// The rendered text. +/// The header and footer starts and ends with. +internal sealed record RenderedPayload(string Text, RenderedLayout Layout) +{ + /// Text with neither a header nor a footer. + public static RenderedPayload Plain(string text) => new(text, RenderedLayout.None); + + /// This payload with , one line, declared below it; unchanged when it is blank. + public RenderedPayload WithFooterLine(string footer) => + string.IsNullOrWhiteSpace(footer) + ? this + : new RenderedPayload(string.Concat(Text, Environment.NewLine, footer), Layout.WithFooter(1)); +} diff --git a/src/Repl.Core/Output/TextTableFormatter.cs b/src/Repl.Core/Output/TextTableFormatter.cs index 6832f7e5..9a267102 100644 --- a/src/Repl.Core/Output/TextTableFormatter.cs +++ b/src/Repl.Core/Output/TextTableFormatter.cs @@ -4,6 +4,45 @@ namespace Repl; internal static class TextTableFormatter { + /// + /// with each control character and line or paragraph separator replaced by a space: a + /// table header is declared one line tall, and a line break in a label would push part of it onto a second + /// line for the pager, which splits at every one of them. Returns the input itself, allocating nothing, when + /// it has none. + /// + public static string ToSingleLine(string text) + { + ArgumentNullException.ThrowIfNull(text); + if (!ContainsLineBreaking(text)) + { + return text; + } + + return string.Create(text.Length, text, static (buffer, source) => + { + for (var i = 0; i < source.Length; i++) + { + buffer[i] = BreaksLine(source[i]) ? ' ' : source[i]; + } + }); + } + + private static bool ContainsLineBreaking(string text) + { + foreach (var character in text) + { + if (BreaksLine(character)) + { + return true; + } + } + + return false; + } + + private static bool BreaksLine(char character) => + char.IsControl(character) || character is '\u2028' or '\u2029'; + public static string FormatRows( IReadOnlyList rows, int renderWidth, diff --git a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs index 9e17ef2d..4f325c19 100644 --- a/src/Repl.Core/ResultFlow/PagerPayloadParser.cs +++ b/src/Repl.Core/ResultFlow/PagerPayloadParser.cs @@ -12,21 +12,12 @@ public static ParsedPagerPayload Parse(string payload, PagerHeader? header, bool var lines = SplitLines(payload); var payloadHeader = DetectHeader(lines); var resolvedHeader = header ?? payloadHeader; + var headerLineCount = payloadHeader.Lines.Count; var content = new List(); - // A continuation's own header is dropped only when it repeats the pinned one. One that names other - // columns stays in the content, separator included even when it matches the pinned one: JSON rows take - // their columns from their own keys, so a later page can have others, and its rows would otherwise sit - // under headings that are not theirs. - if (header is not null && !RepeatsHeader(header, payloadHeader)) - { - content.AddRange(payloadHeader.Lines); - } - - for (var i = payloadHeader.Lines.Count; i < lines.Count; i++) + for (var i = headerLineCount; i < lines.Count; i++) { var normalized = NormalizeLine(lines[i]); - // Only a continuation repeats a header; within one payload, a line like its first one is data. - if ((header is not null && resolvedHeader.NormalizedLines.Contains(normalized)) + if (resolvedHeader.NormalizedLines.Contains(normalized) || (stripPresentationChrome && IsPageFooterLine(lines[i]))) { continue; @@ -38,6 +29,44 @@ public static ParsedPagerPayload Parse(string payload, PagerHeader? header, bool return new ParsedPagerPayload(resolvedHeader, content); } + /// + /// Parses a payload whose transformer declared its layout: its header and footer are exactly the lines the + /// layout names, so nothing is inferred from styling and no line is dropped for reading like a header or a + /// footer. The first payload's header is pinned. A later payload's header is dropped when it names the columns + /// the previous payload showed, and kept when it names others, or its rows would sit under headings that are + /// not theirs. + /// + /// The rendered payload. + /// The pinned header, or for the first payload. + /// The previous payload's layout, or for the first payload. + /// The layout the payload's transformer declared. + public static ParsedPagerPayload ParseDeclared( + string payload, + PagerHeader? header, + RenderedLayout? previousLayout, + RenderedLayout layout) + { + var lines = SplitLines(payload); + var headerLineCount = Math.Min(layout.HeaderLineCount, lines.Count); + var contentEnd = Math.Max(headerLineCount, lines.Count - layout.FooterLineCount); + var content = new List(contentEnd); + if (header is null) + { + content.AddRange(lines.Take(headerLineCount..contentEnd)); + return new ParsedPagerPayload(DeclaredHeader(lines, headerLineCount), content); + } + + var repeatsPreviousColumns = previousLayout is not null && layout.NamesSameColumns(previousLayout); + content.AddRange(lines.Take((repeatsPreviousColumns ? headerLineCount : 0)..contentEnd)); + return new ParsedPagerPayload(header, content); + } + + // Only the detected path compares lines with the header, so a declared one needs no normalized forms. + private static PagerHeader DeclaredHeader(List lines, int headerLineCount) => + headerLineCount == 0 + ? PagerHeader.Empty + : new PagerHeader([.. lines.Take(headerLineCount)], PagerHeader.Empty.NormalizedLines); + private static PagerHeader DetectHeader(List lines) { if (lines.Count == 0) @@ -55,137 +84,11 @@ private static PagerHeader DetectHeader(List lines) return CreateHeader([lines[0]]); } - return (lines[0].Contains("\u001b[1m", StringComparison.Ordinal) || StartsBold(lines[0])) - && NormalizeLine(lines[0]).Length > 0 + return lines[0].Contains("\u001b[1m", StringComparison.Ordinal) ? CreateHeader([lines[0]]) : PagerHeader.Empty; } - // Bold set by the sequences the line starts with, before any text, within a combined sequence such as the - // human palette's table header (ESC[1;38;5;221m). A 1 that is an extended colour's argument, as in 38;5;1, - // is a colour. Bold starting later in the line styles text within it, not a header. - private static bool StartsBold(string line) - { - var start = 0; - while (line.AsSpan(start).StartsWith("\u001b[", StringComparison.Ordinal)) - { - var end = start + 2; - while (end < line.Length && (char.IsAsciiDigit(line[end]) || line[end] == ';')) - { - end++; - } - - if (end >= line.Length || line[end] != 'm') - { - return false; - } - - if (SgrSetsBold(line.AsSpan(start + 2, end - start - 2))) - { - return true; - } - - start = end + 1; - } - - return false; - } - - private static bool SgrSetsBold(ReadOnlySpan parameters) - { - var colourArguments = 0; - var colourModeNext = false; - foreach (var range in parameters.Split(';')) - { - var parameter = parameters[range]; - if (colourModeNext) - { - // 5 takes a palette index, 2 an RGB triple. - colourArguments = parameter is "5" ? 1 : parameter is "2" ? 3 : 0; - colourModeNext = false; - } - else if (colourArguments > 0) - { - colourArguments--; - } - else if (parameter is "38" or "48" or "58") - { - colourModeNext = true; - } - else if (parameter is "1") - { - return true; - } - } - - return false; - } - - // Label by label: a repeated header is padded to its own page's column widths, and its separator line with it. - // A label truncated to a different width on each page does not match, so that header is kept, not lost. - private static bool RepeatsHeader(PagerHeader pinned, PagerHeader candidate) => - pinned.Lines.Count > 0 - && candidate.Lines.Count > 0 - && HeaderLabels(pinned).SequenceEqual(HeaderLabels(candidate), StringComparer.Ordinal); - - // A label can hold spaces, so words alone would take 'first' / 'last name' for 'first last' / 'name', and a - // narrow table leaves a single space between columns. A plain table's separator spans each column, when the - // header is aligned on it; a styled header brackets each label in its own escape sequences; failing both, - // labels are what gaps of two or more spaces leave. - private static IEnumerable HeaderLabels(PagerHeader header) - { - var line = header.Lines[0]; - if (header.Lines.Count > 1 - && LabelsUnderSeparator(AnsiTextMetrics.StripControlSequences(line), header.Lines[1]) is { } spanned) - { - return spanned; - } - - return line.Contains('\u001b', StringComparison.Ordinal) - // Each run is split at its own gaps too: a header can also be styled as one run, padding included. - ? AnsiTextMetrics.SplitAtControlSequences(line).SelectMany(static run => SplitAtGaps(run)) - : SplitAtGaps(line); - } - - // The label over each run of dashes, or null when some of the header lies outside every run: the separator - // then does not say where its columns are. - private static List? LabelsUnderSeparator(string line, string separator) - { - var labels = new List(); - var checkedUpTo = 0; - var i = 0; - while (i < separator.Length) - { - if (separator[i] != '-') - { - i++; - continue; - } - - var start = i; - while (i < separator.Length && separator[i] == '-') - { - i++; - } - - if (!IsBlank(line, checkedUpTo, start)) - { - return null; - } - - labels.Add(start < line.Length ? line[start..Math.Min(i, line.Length)].Trim() : string.Empty); - checkedUpTo = i; - } - - return IsBlank(line, checkedUpTo, line.Length) ? labels : null; - } - - private static string[] SplitAtGaps(string text) => - text.Split(" ", StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - - private static bool IsBlank(string line, int from, int to) => - from >= line.Length || line.AsSpan(from, Math.Min(to, line.Length) - from).IsWhiteSpace(); - private static PagerHeader CreateHeader(string[] lines) => new( lines, diff --git a/src/Repl.Core/ResultFlow/PagerSession.cs b/src/Repl.Core/ResultFlow/PagerSession.cs index 22d4a7dc..ea78e041 100644 --- a/src/Repl.Core/ResultFlow/PagerSession.cs +++ b/src/Repl.Core/ResultFlow/PagerSession.cs @@ -3,19 +3,37 @@ namespace Repl; internal sealed class PagerSession { private readonly PagerHeader _header; + private readonly bool _declaresLayout; + private RenderedLayout? _previousLayout; private readonly int _maxBufferedLines; private readonly List _lines = []; private readonly IReadOnlyList _readOnlyLines; - public PagerSession(string initialPayload, bool hasMorePayload, int maxBufferedLines) + /// The first payload. + /// Whether another payload can be fetched. + /// How many content lines the session buffers at most. + /// + /// The layout the payload's transformer declared, or for a transformer that declares + /// none, whose headers and footers are then detected from the text. Decided once, for every payload of the + /// session: one transformer renders them all. + /// + public PagerSession( + string initialPayload, + bool hasMorePayload, + int maxBufferedLines, + RenderedLayout? layout = null) { _maxBufferedLines = Math.Max(1, maxBufferedLines); _readOnlyLines = _lines.AsReadOnly(); - var parsed = PagerPayloadParser.Parse(initialPayload, header: null); - _header = parsed.Header; - AppendContent(parsed.ContentLines, hasMorePayload); - PageSize = 1; - NextWindow = 1; + _declaresLayout = layout is not null; + _previousLayout = layout; + var parsed = layout is null + ? PagerPayloadParser.Parse(initialPayload, header: null) + : PagerPayloadParser.ParseDeclared(initialPayload, header: null, previousLayout: null, layout); + _header = parsed.Header; + AppendContent(parsed.ContentLines, hasMorePayload); + PageSize = 1; + NextWindow = 1; } public IReadOnlyList HeaderLines => _header.Lines; @@ -34,9 +52,32 @@ public PagerSession(string initialPayload, bool hasMorePayload, int maxBufferedL public bool SourceReturnedNoData { get; set; } - public void Append(string payload, bool hasMorePayload, bool containsPresentationChrome = true) + /// The fetched payload. + /// Whether another payload can be fetched. + /// + /// Whether an undeclared payload can carry footer lines to strip; a declared layout names its footer itself. + /// + /// + /// The layout the payload's transformer declared; ignored, like a missing one, when the session detects. + /// + public void Append( + string payload, + bool hasMorePayload, + bool containsPresentationChrome = true, + RenderedLayout? layout = null) { - var parsed = PagerPayloadParser.Parse(payload, _header, containsPresentationChrome); + ParsedPagerPayload parsed; + if (_declaresLayout) + { + var declared = layout ?? RenderedLayout.None; + parsed = PagerPayloadParser.ParseDeclared(payload, _header, _previousLayout, declared); + _previousLayout = declared; + } + else + { + parsed = PagerPayloadParser.Parse(payload, _header, containsPresentationChrome); + } + AppendContent(parsed.ContentLines, hasMorePayload); } diff --git a/src/Repl.Core/ResultFlow/ResultFlowPager.cs b/src/Repl.Core/ResultFlow/ResultFlowPager.cs index 4ca68dc6..05837855 100644 --- a/src/Repl.Core/ResultFlow/ResultFlowPager.cs +++ b/src/Repl.Core/ResultFlow/ResultFlowPager.cs @@ -14,7 +14,11 @@ internal static class ResultFlowPager private static readonly System.Text.CompositeFormat FullStatusBufferLimitFormat = System.Text.CompositeFormat.Parse(FullStatusBufferLimit); - internal static int CountLines(string payload) => PagerPayloadParser.Parse(payload, header: null).TotalLineCount; + internal static int CountLines(string payload, RenderedLayout? layout = null) => + (layout is null + ? PagerPayloadParser.Parse(payload, header: null) + : PagerPayloadParser.ParseDeclared(payload, header: null, previousLayout: null, layout)) + .TotalLineCount; internal static ValueTask WriteAsync( string payload, @@ -53,6 +57,7 @@ internal static async ValueTask WriteAsync( options.VisibleRowsProvider, options.AnsiEnabled, options.HasMorePayload, + options.PayloadLayout, options.FetchNextPayload, cancellationToken) .ConfigureAwait(false)) @@ -60,7 +65,7 @@ internal static async ValueTask WriteAsync( return; } - var session = new PagerSession(payload, options.HasMorePayload, maxBufferedLines); + var session = new PagerSession(payload, options.HasMorePayload, maxBufferedLines, options.PayloadLayout); await RenderBuiltInAsync( mode, session, @@ -140,6 +145,7 @@ private static async ValueTask TryRenderCustomAsync( Func? visibleRowsProvider, bool ansiEnabled, bool hasMorePayload, + RenderedLayout? payloadLayout, Func>? fetchNextPayload, CancellationToken cancellationToken) { @@ -148,6 +154,8 @@ private static async ValueTask TryRenderCustomAsync( return false; } + var previousLayout = payloadLayout; + foreach (var renderer in pagerRenderers) { if (renderer.Mode != mode) @@ -175,7 +183,26 @@ await renderer.RenderAsync( async ValueTask FetchPublicPayloadAsync(CancellationToken token) { var next = await fetchNextPayload!(token).ConfigureAwait(false); - return next is null ? null : new ReplPagerPayload(next.Payload, next.HasMore); + return next is null ? null : new ReplPagerPayload(WithoutRepeatedHeader(next), next.HasMore); + } + + // A custom renderer gets text only, so a declared header naming the previous page's columns is stripped + // for it, as the built-in pager drops it; a header naming other columns stays with its rows. + string WithoutRepeatedHeader(ResultFlowPagerPage page) + { + if (previousLayout is not { } previous || page.Layout is not { } layout) + { + return page.Payload; + } + + previousLayout = layout; + if (!layout.NamesSameColumns(previous)) + { + return page.Payload; + } + + var parsed = PagerPayloadParser.ParseDeclared(page.Payload, PagerHeader.Empty, previous, layout); + return string.Join(Environment.NewLine, parsed.ContentLines); } } @@ -282,7 +309,7 @@ private static async ValueTask TryFetchIntoSessionAsync( return false; } - session.Append(nextPayload.Payload, nextPayload.HasMore, nextPayload.ContainsPresentationChrome); + session.Append(nextPayload.Payload, nextPayload.HasMore, nextPayload.ContainsPresentationChrome, nextPayload.Layout); return true; } diff --git a/src/Repl.Core/ResultFlow/ResultFlowPagerOptions.cs b/src/Repl.Core/ResultFlow/ResultFlowPagerOptions.cs index 76c8758a..dc3af34c 100644 --- a/src/Repl.Core/ResultFlow/ResultFlowPagerOptions.cs +++ b/src/Repl.Core/ResultFlow/ResultFlowPagerOptions.cs @@ -12,6 +12,12 @@ internal sealed record ResultFlowPagerOptions public bool HasMorePayload { get; init; } + /// + /// The layout the first payload's transformer declared, or to detect its header and + /// footer from the text. It decides for the whole session: fetched payloads come from the same transformer. + /// + public RenderedLayout? PayloadLayout { get; init; } + public Func>? FetchNextPayload { get; init; } public IReadOnlyList? PagerRenderers { get; init; } diff --git a/src/Repl.Core/ResultFlow/ResultFlowPagerPage.cs b/src/Repl.Core/ResultFlow/ResultFlowPagerPage.cs index 4b95bd55..abf388e8 100644 --- a/src/Repl.Core/ResultFlow/ResultFlowPagerPage.cs +++ b/src/Repl.Core/ResultFlow/ResultFlowPagerPage.cs @@ -1,6 +1,14 @@ namespace Repl; +/// The rendered page. +/// Whether another page can be fetched. +/// Whether the payload can carry footer lines for the pager to strip. +/// +/// The layout the page's transformer declared, or when it declares none and the pager +/// detects its header and footer from the text. +/// internal sealed record ResultFlowPagerPage( string Payload, bool HasMore, - bool ContainsPresentationChrome = true); + bool ContainsPresentationChrome = true, + RenderedLayout? Layout = null); diff --git a/src/Repl.Core/ResultFlowPageRenderMode.cs b/src/Repl.Core/ResultFlowPageRenderMode.cs deleted file mode 100644 index cfa4289c..00000000 --- a/src/Repl.Core/ResultFlowPageRenderMode.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace Repl; - -internal enum ResultFlowPageRenderMode -{ - Initial, - Continuation, -} diff --git a/src/Repl.Core/Terminal/AnsiTextMetrics.cs b/src/Repl.Core/Terminal/AnsiTextMetrics.cs index 178e6278..52b72509 100644 --- a/src/Repl.Core/Terminal/AnsiTextMetrics.cs +++ b/src/Repl.Core/Terminal/AnsiTextMetrics.cs @@ -69,39 +69,6 @@ public static string StripControlSequences(ReadOnlySpan text) return builder.ToString(); } - /// - /// The runs of text between escape sequences, in order and untrimmed. A renderer that styles each table - /// label on its own brackets every label with its own sequences, so a label reads as one run, spaces included. - /// - public static List SplitAtControlSequences(string text) - { - var span = text.AsSpan(); - var runs = new List(); - var start = 0; - for (var i = 0; i < span.Length; i++) - { - if (span[i] != '\u001b') - { - continue; - } - - if (i > start) - { - runs.Add(text[start..i]); - } - - i = SkipEscapeSequence(span, i); - start = i + 1; - } - - if (start < text.Length) - { - runs.Add(text[start..]); - } - - return runs; - } - private static int SkipEscapeSequence(ReadOnlySpan text, int escapeIndex) { if (escapeIndex + 1 >= text.Length) diff --git a/src/Repl.IntegrationTests/Given_OutputFormatting.cs b/src/Repl.IntegrationTests/Given_OutputFormatting.cs index 68dc3a0f..188e5d4a 100644 --- a/src/Repl.IntegrationTests/Given_OutputFormatting.cs +++ b/src/Repl.IntegrationTests/Given_OutputFormatting.cs @@ -1,4 +1,6 @@ using System.ComponentModel.DataAnnotations; +using System.Globalization; +using System.Text.Json.Nodes; using System.Text.RegularExpressions; using Repl.Spectre; @@ -269,6 +271,86 @@ public void When_RenderingPageSourceInHumanPager_Then_SpaceFetchesNextPageWithou text.Should().NotContain("Showing "); } + [TestMethod] + [Description("A JSON page source in the human pager: a page with the same keys at other widths adds its row only, and a page with other keys shows its own header, so no row sits under headings that are not its own.")] + public void When_PagingJsonRowsWhoseKeysChange_Then_EachHeaderShowsOnceForItsRows() + { + var sut = ReplApp.Create(); + JsonObject[] rows = + [ + new() { ["id"] = 1, ["name"] = "a" }, + new() { ["id"] = 22, ["name"] = "bbbb" }, + new() { ["name"] = "c", ["id"] = 3 }, + ]; + sut.Map("rows", (IReplPagingContext paging) => + paging.CreateSource((request, _) => + { + var index = request.Cursor is null ? 0 : int.Parse(request.Cursor, CultureInfo.InvariantCulture); + var next = index + 1 < rows.Length ? (index + 1).ToString(CultureInfo.InvariantCulture) : null; + return ValueTask.FromResult(new ReplPage( + [rows[index]], + new ReplPageInfo(request.Cursor, next, rows.Length, request.PageSize))); + })); + + using var output = new StringWriter(); + using var session = ReplSessionIO.SetSession(output, TextReader.Null); + ReplSessionIO.KeyReader = new QueueKeyReader([.. Enumerable.Repeat(Key(ConsoleKey.Spacebar, ' '), 4)]); + ReplSessionIO.WindowSize = (100, 20); + + var exitCode = sut.Run(["rows", "--result:page-size=1", "--no-logo"]); + + exitCode.Should().Be(0); + var lines = StripAnsi(output.ToString()).Split(Environment.NewLine); + lines.Count(line => IsHeader(line, "id", "name")).Should().Be(1, "the second page repeats the first one's columns"); + lines.Count(line => IsHeader(line, "name", "id")).Should().Be(1, "the third page names other columns"); + var third = Array.FindIndex(lines, line => line.Contains("\"c\"", StringComparison.Ordinal)); + third.Should().BePositive(); + lines.Take(third).Last(line => IsHeader(line, "id", "name") || IsHeader(line, "name", "id")) + .Should().Match(line => IsHeader(line, "name", "id"), "the third page's rows sit under its own header"); + } + + [TestMethod] + [Description("A JSON array returned whole and paged: a row that reads like the header is data, so every row reaches the screen rather than being dropped as a repeated header.")] + public void When_PagingAJsonArrayWhoseRowsReadLikeItsHeader_Then_EveryRowIsShown() + { + var sut = ReplApp.Create(); + sut.Map("ones", () => new JsonArray([.. Enumerable.Range(0, 10).Select(_ => (JsonNode)new JsonObject { ["1"] = 1 })])); + + using var output = new StringWriter(); + using var session = ReplSessionIO.SetSession(output, TextReader.Null); + ReplSessionIO.KeyReader = new QueueKeyReader([.. Enumerable.Repeat(Key(ConsoleKey.Spacebar, ' '), 10)]); + ReplSessionIO.WindowSize = (100, 8); + + var exitCode = sut.Run(["ones", "--no-logo"]); + + exitCode.Should().Be(0); + StripAnsi(output.ToString()).Split(Environment.NewLine).Count(line => string.Equals(line.Trim(), "1", StringComparison.Ordinal)) + .Should().Be(11, "the header, then each of the ten rows"); + } + + [TestMethod] + [Description("A long page returned whole is paged without a way to fetch the next one, so its footer, which says how to rerun for it, stays in view.")] + public void When_PagingALongPageReturnedWhole_Then_ItsRerunFooterIsShown() + { + var sut = ReplApp.Create(); + sut.Map("ones", () => new ReplPage( + [.. Enumerable.Range(1, 10).Select(i => new JsonObject { ["id"] = i })], + new ReplPageInfo(Cursor: null, NextCursor: "next", TotalCount: 20, PageSize: 10))); + + using var output = new StringWriter(); + using var session = ReplSessionIO.SetSession(output, TextReader.Null); + ReplSessionIO.KeyReader = new QueueKeyReader([.. Enumerable.Repeat(Key(ConsoleKey.Spacebar, ' '), 10)]); + ReplSessionIO.WindowSize = (100, 8); + + var exitCode = sut.Run(["ones", "--no-logo"]); + + exitCode.Should().Be(0); + StripAnsi(output.ToString()).Should().Contain("Showing 10 of 20. Next data page: rerun with"); + } + + private static bool IsHeader(string line, params string[] columns) => + line.Split(' ', StringSplitOptions.RemoveEmptyEntries).SequenceEqual(columns, StringComparer.Ordinal); + [TestMethod] [Description("Regression guard: verifies paging.CreateSource receives the caller cursor and suggested page size through the first source request.")] public void When_PageSourceIsCreatedFromPagingContext_Then_FirstRequestUsesCurrentPagingIntent() diff --git a/src/Repl.Spectre/SingleLineLabel.cs b/src/Repl.Spectre/SingleLineLabel.cs new file mode 100644 index 00000000..9a46a41b --- /dev/null +++ b/src/Repl.Spectre/SingleLineLabel.cs @@ -0,0 +1,51 @@ +using Spectre.Console.Rendering; + +namespace Repl.Spectre; + +/// +/// A bold table header label that always renders on one line, truncated when its column cannot fit it, the way the +/// default human transformer truncates. The pager is told a table's header is one line tall, so a header cell must +/// never wrap onto a second one. +/// +internal sealed class SingleLineLabel : Renderable +{ + private const string Ellipsis = "..."; + private static readonly Style Bold = new(decoration: Decoration.Bold); + + private readonly Segment _label; + private readonly int _width; + + public SingleLineLabel(string text) + { + ArgumentNullException.ThrowIfNull(text); + _label = new Segment(TextTableFormatter.ToSingleLine(text), Bold); + _width = _label.CellCount(); + } + + // Unable to wrap, it has no narrower form than its whole width. + protected override Measurement Measure(RenderOptions options, int maxWidth) + { + var width = Math.Min(_width, maxWidth); + return new Measurement(width, width); + } + + // Always one segment, even for a column with no room at all, so the header row keeps its one line. + protected override IEnumerable Render(RenderOptions options, int maxWidth) + { + if (_width <= maxWidth) + { + return [_label]; + } + + if (maxWidth <= Ellipsis.Length) + { + return [Segment.Truncate(_label, Math.Max(0, maxWidth)) ?? new Segment(string.Empty)]; + } + + return + [ + Segment.Truncate(_label, maxWidth - Ellipsis.Length) ?? new Segment(string.Empty), + new Segment(Ellipsis, Bold), + ]; + } +} diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index 9d640fee..b6fb7762 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -47,37 +47,37 @@ public SpectreHumanOutputTransformer( public ValueTask TransformAsync(object? value, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); + return ValueTask.FromResult(Render(value).Text); + } - if (value is null) - { - return ValueTask.FromResult(string.Empty); - } - - return ValueTask.FromResult(value switch - { - HelpRenderDocument help => RenderHelp(help), - IReplPage page => RenderPage(page), - IReplResult replResult => RenderReplResult(replResult), - string text => text, - // Before the enumerable arm: a JsonObject enumerates as key/value pairs, and reflecting over - // those walks JsonNode's Root and Parent, which point back at each other. - _ when JsonHumanShape.TryGetNode(value, out var node) => RenderJson(node), - System.Collections.IEnumerable enumerable => RenderEnumerable(enumerable), - _ when TryRenderObject(value, out var objectText) => objectText, - _ => Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty, - }); + public ValueTask RenderAsync(object? value, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + return ValueTask.FromResult(Render(value)); } - public ValueTask TransformPageAsync( - IReplPage page, - ResultFlowPageRenderMode mode, - CancellationToken cancellationToken = default) + public ValueTask RenderPageAsync(IReplPage page, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(page); cancellationToken.ThrowIfCancellationRequested(); - return ValueTask.FromResult(RenderPage(page, mode, includeFooter: false)); + return ValueTask.FromResult(RenderPageBody(page)); } + private RenderedPayload Render(object? value) => value switch + { + null => RenderedPayload.Plain(string.Empty), + HelpRenderDocument help => RenderedPayload.Plain(RenderHelp(help)), + IReplPage page => RenderPageWithFooter(page), + IReplResult replResult => RenderReplResult(replResult), + string text => RenderedPayload.Plain(text), + // Before the enumerable arm: a JsonObject enumerates as key/value pairs, and reflecting over + // those walks JsonNode's Root and Parent, which point back at each other. + _ when JsonHumanShape.TryGetNode(value, out var node) => RenderJson(node), + System.Collections.IEnumerable enumerable => RenderItems([.. enumerable.Cast()]), + _ when TryRenderObject(value, out var objectText) => RenderedPayload.Plain(objectText), + _ => RenderedPayload.Plain(Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty), + }; + private string RenderHelp(HelpRenderDocument help) { if (help.IsCommandHelp) @@ -174,7 +174,7 @@ private string RenderCommandList(IReadOnlyList commands, stri return RenderToString(new Rows(sections)); } - private string RenderReplResult(IReplResult result) + private RenderedPayload RenderReplResult(IReplResult result) { var statusMarkup = result.Kind.ToLowerInvariant() switch { @@ -189,39 +189,39 @@ private string RenderReplResult(IReplResult result) if (result.Details is null) { - return RenderToString(new Markup(statusMarkup)); + return RenderedPayload.Plain(RenderToString(new Markup(statusMarkup))); } - IRenderable details = result.Details switch + if (result.Details is IReplPage page) { - IReplPage page => new Text(RenderPage(page)), + // The page is already rendered: appended as text rather than laid out again, which would count its + // escape sequences as columns and could wrap its lines, the footer among them. The status comes + // first, so no header starts the payload; the page's footer still ends it. + var renderedPage = RenderPageWithFooter(page); + return new RenderedPayload( + string.Concat(RenderToString(new Markup(statusMarkup)), Environment.NewLine, Environment.NewLine, renderedPage.Text), + RenderedLayout.None.WithFooter(renderedPage.Layout.FooterLineCount)); + } + + var details = JsonHumanShape.TryGetNode(result.Details, out var node) // Like a JSON result, as the human transformer renders it, rather than as the compact literal a // JSON value nested in a result object gets. - _ when JsonHumanShape.TryGetNode(result.Details, out var node) => BuildJson(node), - _ => RenderValueRenderable(result.Details, nested: false), - }; - return RenderToString(new Rows(new IRenderable[] + ? BuildJson(node) + : RenderValueRenderable(result.Details, nested: false); + return RenderedPayload.Plain(RenderToString(new Rows(new IRenderable[] { new Markup(statusMarkup), new Text(string.Empty), details, - })); + }))); } - private string RenderEnumerable( - System.Collections.IEnumerable enumerable, - bool includeTableHeader = true) + private RenderedPayload RenderItems(object?[] items) { - var items = enumerable.Cast().ToArray(); - if (items.Length == 0) - { - return "No results."; - } - var firstNonNull = items.FirstOrDefault(item => item is not null); if (firstNonNull is null) { - return "No results."; + return RenderedPayload.Plain("No results."); } // Only once the first item is JSON: ordinary collections must not pay for the JSON row scan. @@ -230,44 +230,28 @@ private string RenderEnumerable( return RenderJsonItems(items); } - if (IsSimpleValue(firstNonNull.GetType())) - { - return string.Join( - Environment.NewLine, - items.Select(item => Convert.ToString(item, CultureInfo.InvariantCulture) ?? string.Empty)); - } - - var members = GetDisplayMembers(firstNonNull.GetType()); + var members = IsSimpleValue(firstNonNull.GetType()) ? [] : GetDisplayMembers(firstNonNull.GetType()); if (members.Length == 0) { - return string.Join( + var lines = string.Join( Environment.NewLine, items.Select(item => Convert.ToString(item, CultureInfo.InvariantCulture) ?? string.Empty)); + return RenderedPayload.Plain(lines); } - return RenderToString(BuildObjectTable(items, members, includeTableHeader)); + return new RenderedPayload( + RenderToString(BuildObjectTable(items, members)), + new RenderedLayout(TableHeaderLineCount, [.. members.Select(member => member.Label)])); } - private string RenderPage(IReplPage page) => - RenderPage(page, ResultFlowPageRenderMode.Initial, includeFooter: true); - - private string RenderPage( - IReplPage page, - ResultFlowPageRenderMode mode, - bool includeFooter) - { - var body = RenderPageBody(page, mode); - var footer = includeFooter ? RenderPageFooter(page) : string.Empty; - return string.IsNullOrWhiteSpace(footer) - ? body - : string.Concat(body, Environment.NewLine, footer); - } + private RenderedPayload RenderPageWithFooter(IReplPage page) => + RenderPageBody(page).WithFooterLine(ResultFlowPageFooterBuilder.RenderHuman(page)); - private string RenderPageBody(IReplPage page, ResultFlowPageRenderMode mode) + private RenderedPayload RenderPageBody(IReplPage page) { if (page.UntypedItems.Count == 0) { - return "No results."; + return RenderedPayload.Plain("No results."); } // By its declared item type: a page of JSON nulls has no item to be recognized by. @@ -276,40 +260,18 @@ private string RenderPageBody(IReplPage page, ResultFlowPageRenderMode mode) return RenderJsonItems(page.UntypedItems); } - return RenderEnumerable( - page.UntypedItems, - includeTableHeader: mode == ResultFlowPageRenderMode.Initial); - } - - private static string RenderPageFooter(IReplPage page) - { - var info = page.PageInfo; - var count = page.UntypedItems.Count; - if (info.TotalCount is { } total) - { - var prefix = $"Showing {count.ToString(CultureInfo.InvariantCulture)} of {total.ToString(CultureInfo.InvariantCulture)}."; - return info.HasMore - ? $"{prefix} Next data page: rerun with {ResultFlowCursorPolicy.FormatCliContinuation(info.NextCursor)}." - : prefix; - } - - if (!info.HasMore) - { - return string.Empty; - } - - return $"Showing {count.ToString(CultureInfo.InvariantCulture)} result(s). Next data page: rerun with {ResultFlowCursorPolicy.FormatCliContinuation(info.NextCursor)}."; + return RenderItems([.. page.UntypedItems]); } - private string RenderJson(JsonNode? node) => node switch + private RenderedPayload RenderJson(JsonNode? node) => node switch { - JsonObject { Count: 0 } => "{}", - JsonObject jsonObject => RenderToString(BuildJsonObjectGrid(jsonObject)), - JsonArray { Count: 0 } => "No results.", - // Its own path rather than RenderEnumerable, which recognizes JSON by its first non-null item and so - // has nothing to go on for an array of nulls. + JsonObject { Count: 0 } => RenderedPayload.Plain("{}"), + JsonObject jsonObject => RenderedPayload.Plain(RenderToString(BuildJsonObjectGrid(jsonObject))), + JsonArray { Count: 0 } => RenderedPayload.Plain("No results."), + // Its own path rather than RenderItems, which recognizes JSON by its first non-null item and so has + // nothing to go on for an array of nulls. JsonArray jsonArray => RenderJsonItems([.. jsonArray]), - _ => JsonHumanShape.Literal(node), + _ => RenderedPayload.Plain(JsonHumanShape.Literal(node)), }; // The renderable for JSON composed into a larger layout, such as a result's details. Text wrapping an @@ -320,7 +282,7 @@ private static string RenderPageFooter(IReplPage page) JsonArray jsonArray when JsonHumanShape.TryGetObjectRows([.. jsonArray], out var columns, out var rows) => BuildJsonTable(columns, rows), // Literal lines only from here on: plain text, nothing styled. - _ => new Text(RenderJson(node)), + _ => new Text(RenderJson(node).Text), }; private static Grid BuildJsonObjectGrid(JsonObject jsonObject) => @@ -328,23 +290,35 @@ private static Grid BuildJsonObjectGrid(JsonObject jsonObject) => [.. jsonObject.Select(static property => (JsonHumanShape.Label(property.Key), JsonHumanShape.Literal(property.Value))),]); - // A JSON table always carries its header, continuation pages included: its columns come from its own rows' - // keys rather than from a type, so another page's headings could mislabel its cells. - private string RenderJsonItems(IReadOnlyList items) => - JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows) - ? RenderToString(BuildJsonTable(columns, rows)) - : string.Join( - Environment.NewLine, - items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) ? literal : RenderInlineValue(item))); + // The table's columns come from its own rows' keys rather than from a type, so a later page can name others; + // declaring those keys lets the pager tell a repeated header from one it must keep. + private RenderedPayload RenderJsonItems(IReadOnlyList items) + { + if (JsonHumanShape.TryGetObjectRows(items, out var columns, out var rows)) + { + return new RenderedPayload( + RenderToString(BuildJsonTable(columns, rows)), + new RenderedLayout(TableHeaderLineCount, columns)); + } - private static Table BuildJsonTable(string[] columns, JsonObject?[] rows) + var literals = string.Join( + Environment.NewLine, + items.Select(item => JsonHumanShape.TryLiteral(item, out var literal) ? literal : RenderInlineValue(item))); + return RenderedPayload.Plain(literals); + } + + // Both table builders label their columns with SingleLineLabel, which never wraps, and a borderless table puts + // no rule under its header: the header is exactly one line. + private const int TableHeaderLineCount = 1; + + private static Table BuildJsonTable(string[] columns, JsonObject[] rows) { var table = new Table() .Border(TableBorder.None) .Collapse(); foreach (var column in columns) { - table.AddColumn(new TableColumn($"[bold]{Markup.Escape(JsonHumanShape.Label(column))}[/]")); + table.AddColumn(new TableColumn(new SingleLineLabel(JsonHumanShape.Label(column)))); } foreach (var row in rows) @@ -391,22 +365,15 @@ private Grid BuildObjectGrid(object value, IReadOnlyList members) return grid; } - private static Table BuildObjectTable( - object?[] items, - IReadOnlyList members, - bool includeHeaders = true) + private static Table BuildObjectTable(object?[] items, IReadOnlyList members) { var table = new Table() .Border(TableBorder.None) .Collapse(); - if (!includeHeaders) - { - table.HideHeaders(); - } foreach (var member in members) { - table.AddColumn(new TableColumn($"[bold]{Markup.Escape(member.Label)}[/]")); + table.AddColumn(new TableColumn(new SingleLineLabel(member.Label))); } foreach (var item in items) diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index 82860259..ae043829 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -143,11 +143,13 @@ public async Task When_AContinuationPageHasOtherKeys_Then_ItCarriesItsOwnHeader( [new JsonObject { ["name"] = "b", ["id"] = 2 }], new ReplPageInfo(Cursor: "1", NextCursor: null, TotalCount: null, PageSize: 1)); - var raw = await new SpectreHumanOutputTransformer() - .TransformPageAsync(page, ResultFlowPageRenderMode.Continuation, CancellationToken.None) + var rendered = await new SpectreHumanOutputTransformer() + .RenderPageAsync(page, CancellationToken.None) .ConfigureAwait(false); - AnsiStyling().Replace(raw, string.Empty).Should().MatchRegex(@"name\s+id"); + AnsiStyling().Replace(rendered.Text, string.Empty).Should().MatchRegex(@"name\s+id"); + rendered.Layout.HeaderLineCount.Should().Be(1); + rendered.Layout.Columns.Should().Equal("name", "id"); } [TestMethod] @@ -184,59 +186,73 @@ public async Task When_APageOfJsonNullsIsRendered_Then_EachItemReadsNull() } [TestMethod] - [Description("Through the pager, which pins the first page's header and drops a repeated one (a bold first line counts as one): a continuation page with other keys must keep its own header, or its rows sit under headings that are not theirs.")] - public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOwnHeader() + [Description("Through the pager, a continuation page with the same keys at other widths repeats the pinned header, so it adds its data row only, with or without ANSI: a plain Spectre header has neither a separator nor styling to be recognized by.")] + [DataRow(AnsiMode.Always)] + [DataRow(AnsiMode.Never)] + public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded(AnsiMode ansiMode) { - var transformer = CreateAnsiTransformer(); - var first = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) - .ConfigureAwait(false); - var next = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["name"] = "b", ["id"] = 2 }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) - .ConfigureAwait(false); + var session = await PageThroughAsync( + CreateTransformer(ansiMode), + new JsonObject { ["id"] = 1, ["name"] = "a" }, + new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" }).ConfigureAwait(false); - var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); - session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + session.HeaderLines.Should().ContainSingle("the first page's header is pinned"); + session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); + AnsiStyling().Replace(session.Lines[1], string.Empty).Should().Contain("\"bbbbbb\""); + } - var lines = session.Lines.Select(line => AnsiStyling().Replace(line, string.Empty)).ToList(); - var row = lines.FindIndex(line => line.Contains("\"b\"", StringComparison.Ordinal)); - row.Should().BePositive(); - lines.Take(row).Should().Contain(line => line.Contains("name", StringComparison.Ordinal)); + [TestMethod] + [Description("A header label too long for a narrow table is truncated on its one line rather than wrapped onto a second: the pager is told the header is one line tall, and a wrapped half would show up as a row.")] + public async Task When_AHeaderLabelDoesNotFit_Then_ItStaysOnOneLine() + { + var label = "a rather long column label"; + var session = await PageThroughAsync( + CreateTransformer(AnsiMode.Never, width: 20), + new JsonObject { ["id"] = 1, [label] = "x" }, + new JsonObject { ["id"] = 2, [label] = "y" }).ConfigureAwait(false); + + session.HeaderLines.Should().ContainSingle(); + session.Lines.Should().HaveCount(2, "the two rows; no part of the header is left among them"); } [TestMethod] - [Description("Through the pager, a continuation page with the same keys at other widths repeats the pinned header, so it adds its data row only.")] - public async Task When_ThePagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded() + [Description("A result whose details are a page ends with that page's footer, whole on its last line even when it outruns the width: laid out again as Spectre text, it would wrap, and the declared one-line footer would leave half of it behind.")] + public async Task When_AResultCarriesAPageWithALongCursor_Then_ItsFooterIsWholeAndLast() { - var transformer = CreateAnsiTransformer(); - var first = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) - .ConfigureAwait(false); - var next = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) + var page = new ReplPage( + [new JsonObject { ["id"] = 1, ["note"] = new string('x', 30) }], + new ReplPageInfo(Cursor: null, NextCursor: new string('c', 120), TotalCount: 5, PageSize: 1)); + + var rendered = await CreateTransformer(AnsiMode.Always, width: 40) + .RenderAsync(Results.Success("Found", page), CancellationToken.None) .ConfigureAwait(false); - var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); - session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + var lines = rendered.Text.ReplaceLineEndings("\n").Split('\n'); + AnsiStyling().Replace(lines[^1], string.Empty).Should().StartWith("Showing 1 of 5.").And.EndWith(new string('c', 120) + "."); + rendered.Layout.HeaderLineCount.Should().Be(0); + rendered.Layout.FooterLineCount.Should().Be(1); + } - session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); - AnsiStyling().Replace(session.Lines[1], string.Empty).Should().Contain("\"bbbbbb\""); + [TestMethod] + [Description("A type's table whose header fits keeps its labels whole and wraps its data: labels are one line each, but are truncated only when the header itself does not fit.")] + public async Task When_ATypeTableIsNarrow_Then_ItsLabelsStayWhole() + { + var output = await CreateTransformer(AnsiMode.Never, width: 40) + .TransformAsync(new[] { new Described("Ada", "2026-09-24", new string('x', 60)) }, CancellationToken.None) + .ConfigureAwait(false); + + var header = output.Split(Environment.NewLine)[0]; + header.Should().Contain("Display Name").And.Contain("Created At"); } [TestMethod] [Description("Through the pager, with Spectre's own styled header: keys 'first' / 'last name' and 'first last' / 'name' share their words but are other columns, so the continuation keeps its header.")] public async Task When_ThePagerAppendsAPageWhoseKeysRegroupTheWords_Then_ItsRowsKeepTheirOwnHeader() { - var transformer = CreateAnsiTransformer(); - var first = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["first"] = "a", ["last name"] = "b" }), ResultFlowPageRenderMode.Initial, CancellationToken.None) - .ConfigureAwait(false); - var next = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["first last"] = "c", ["name"] = "d" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None) - .ConfigureAwait(false); - - var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); - session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + var session = await PageThroughAsync( + CreateTransformer(AnsiMode.Always), + new JsonObject { ["first"] = "a", ["last name"] = "b" }, + new JsonObject { ["first last"] = "c", ["name"] = "d" }).ConfigureAwait(false); var lines = session.Lines.Select(line => AnsiStyling().Replace(line, string.Empty)).ToList(); var row = lines.FindIndex(line => line.Contains("\"d\"", StringComparison.Ordinal)); @@ -272,12 +288,37 @@ public async Task When_AResultCarriesJsonDetails_Then_TheFieldsAreShown() AssertNoClrMembers(output); } - // The ANSI pager case, where the bold header line is what the pager detects and pins. Forced, so these - // tests do not depend on whether the console running them supports ANSI. - private static SpectreHumanOutputTransformer CreateAnsiTransformer() => + // ANSI is forced one way or the other, so these tests do not depend on whether the console running them + // supports it. + private static SpectreHumanOutputTransformer CreateTransformer(AnsiMode ansiMode, int width = 120) => new( - () => new HumanRenderSettings(Width: 120, UseAnsi: true, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark)), - new OutputOptions { AnsiMode = AnsiMode.Always }); + () => new HumanRenderSettings( + Width: width, + UseAnsi: ansiMode == AnsiMode.Always, + Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark)), + new OutputOptions { AnsiMode = ansiMode }); + + // Drives single-row pages through a PagerSession the way the pager does, each with the layout its transformer + // declared. + private static async Task PageThroughAsync( + SpectreHumanOutputTransformer transformer, + params JsonObject[] rows) + { + var initial = await transformer.RenderPageAsync(SingleRowPage(rows[0]), CancellationToken.None).ConfigureAwait(false); + var session = new PagerSession(initial.Text, hasMorePayload: rows.Length > 1, maxBufferedLines: 100, initial.Layout); + for (var i = 1; i < rows.Length; i++) + { + var next = await transformer.RenderPageAsync(SingleRowPage(rows[i]), CancellationToken.None).ConfigureAwait(false); + session.Append(next.Text, hasMorePayload: i < rows.Length - 1, containsPresentationChrome: false, next.Layout); + } + + return session; + } + + private sealed record Described( + [property: System.ComponentModel.DataAnnotations.Display(Name = "Display Name")] string Name, + [property: System.ComponentModel.DataAnnotations.Display(Name = "Created At")] string CreatedAt, + string Description); private static ReplPage SingleRowPage(JsonObject row) => new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); diff --git a/src/Repl.Tests/Given_HumanOutputJson.cs b/src/Repl.Tests/Given_HumanOutputJson.cs index ccf74eab..56ec4b41 100644 --- a/src/Repl.Tests/Given_HumanOutputJson.cs +++ b/src/Repl.Tests/Given_HumanOutputJson.cs @@ -12,7 +12,6 @@ namespace Repl.Tests; public sealed class Given_HumanOutputJson { private static readonly string[] ClrMembers = ["Options", "Parent", "Root", "Count", "ValueKind"]; - private static readonly string[] PageTwoColumns = ["name", "id"]; [TestMethod] [Description("The issue's own repro: a JsonObject result renders one line per JSON field, values as JSON literals, and none of JsonObject's CLR members.")] @@ -187,9 +186,11 @@ public async Task When_AContinuationPageHasOtherKeys_Then_ItCarriesItsOwnHeader( [new JsonObject { ["name"] = "b", ["id"] = 2 }], new ReplPageInfo(Cursor: "1", NextCursor: null, TotalCount: null, PageSize: 1)); - var output = await CreateTransformer().TransformPageAsync(page, ResultFlowPageRenderMode.Continuation, CancellationToken.None); + var rendered = await CreateTransformer().RenderPageAsync(page, CancellationToken.None); - output.Split(Environment.NewLine)[0].Should().MatchRegex(@"^name\s+id\s*$"); + rendered.Text.Split(Environment.NewLine)[0].Should().MatchRegex(@"^name\s+id\s*$"); + rendered.Layout.HeaderLineCount.Should().Be(2, "the header line and its separator"); + rendered.Layout.Columns.Should().Equal("name", "id"); } [TestMethod] @@ -239,37 +240,147 @@ public async Task When_APageOfNullableJsonElementNullsIsRendered_Then_EachItemRe } [TestMethod] - [Description("Through the pager, which pins the first page's header and drops a repeated one: a continuation page with other keys must keep its own header, or its rows sit under headings that are not theirs.")] - public async Task When_ThePagerAppendsAPageWithOtherKeys_Then_ItsRowsKeepTheirOwnHeader() + [Description("A JSON null among object rows, as a JsonElement or a CLR null, makes every row a literal: as an empty table row it would read as no row, and as the last one it would vanish with the payload's trailing blank line.")] + public async Task When_APageOfJsonObjectsHasANullRow_Then_EveryRowReadsAsALiteral() { - var transformer = CreateTransformer(); - var first = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None); - var next = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["name"] = "b", ["id"] = 2 }), ResultFlowPageRenderMode.Continuation, CancellationToken.None); + using var document = JsonDocument.Parse("""[{"id":1},null]"""); + var elements = new ReplPage( + [.. document.RootElement.EnumerateArray()], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + var nodes = new ReplPage( + [new JsonObject { ["id"] = 1 }, null], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + + (await RenderAsync(elements)).Split(Environment.NewLine).Should().Equal("""{"id":1}""", "null"); + (await RenderAsync(nodes)).Split(Environment.NewLine).Should().Equal("""{"id":1}""", "null"); + } - var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); - session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + [TestMethod] + [Description("An empty object among keyed rows renders as blank as a null would, so it makes every row a literal too.")] + public async Task When_APageOfJsonObjectsHasAnEmptyObject_Then_EveryRowReadsAsALiteral() + { + var output = await RenderAsync(new JsonArray(new JsonObject { ["id"] = 1 }, new JsonObject())); - var lines = session.Lines.ToList(); - var row = lines.FindIndex(line => line.Contains("\"b\"", StringComparison.Ordinal)); - row.Should().BePositive(); - lines.Take(row).Should().Contain(line => line.Split(' ', StringSplitOptions.RemoveEmptyEntries).SequenceEqual(PageTwoColumns)); + output.Split(Environment.NewLine).Should().Equal("""{"id":1}""", "{}"); } [TestMethod] - [Description("A JSON null that arrives as a JsonElement is an empty row, like a CLR null: it must not turn the whole table into bare literals.")] - public async Task When_APageOfJsonElementsHasANullRow_Then_ItStaysATable() + [Description("Through the pager, keys A, then B, then A again: each page shows its header, since the one in view before it names other columns.")] + public async Task When_ThePagerAppendsKeysThatSwitchBack_Then_EachPageShowsItsHeader() { - using var document = JsonDocument.Parse("""[{"id":1},null,{"id":2}]"""); - var page = new ReplPage( - [.. document.RootElement.EnumerateArray()], - new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 3)); + var session = await PageThroughAsync( + CreateTransformer(), + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), + SingleRowPage(new JsonObject { ["name"] = "b", ["id"] = 2 }), + SingleRowPage(new JsonObject { ["id"] = 3, ["name"] = "c" })); + + session.Lines.Should().HaveCount(7, "the first row, then a header, its separator and a row for each later page"); + session.Lines[4].Should().MatchRegex(@"^id\s+name\s*$"); + } - var output = await RenderAsync(page); + [TestMethod] + [Description("Through the pager, a first page with no header pins none; the next table page shows its header, and the one after, with the same columns, adds its row only.")] + public async Task When_TheFirstPageHasNoHeader_Then_TheFirstTablePageShowsItOnce() + { + var session = await PageThroughAsync( + CreateTransformer(), + new ReplPage([null], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)), + SingleRowPage(new JsonObject { ["id"] = 1 }), + SingleRowPage(new JsonObject { ["id"] = 2 })); + + session.HeaderLines.Should().BeEmpty(); + session.Lines.Should().Equal("null", "id", "--", "1", "2"); + } + + [TestMethod] + [Description("Through the pager, a page of rows of another type shows its own header: a type's table renders its header on every page, and the pager drops only a repeat.")] + public async Task When_ThePagerAppendsRowsOfAnotherType_Then_TheirHeaderIsShown() + { + var session = await PageThroughAsync( + CreateTransformer(), + new ReplPage([new Holder("h1", new JsonObject())], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)), + new ReplPage([new Holder("h2", new JsonObject())], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)), + new ReplPage([new Owner("octo")], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1))); + + session.Lines.Should().HaveCount(5, "the two holders' rows, then the owner's header, separator and row"); + session.Lines[2].Should().MatchRegex(@"^Login\s*$"); + } + + [TestMethod] + [Description("A line separator (U+2028) in a display name breaks the line for the pager as a line feed does, so it reads as a space too.")] + public async Task When_ADisplayNameHasALineSeparator_Then_TheHeaderStaysOnOneLine() + { + var rendered = await CreateTransformer().RenderAsync(new[] { new Separated("now") }, CancellationToken.None); + + var session = new PagerSession(rendered.Text, hasMorePayload: false, maxBufferedLines: 100, rendered.Layout); + session.HeaderLines.Should().HaveCount(2, "the header line and its separator"); + session.HeaderLines[0].Should().MatchRegex("^Created At *$"); + session.Lines.Should().ContainSingle(); + } + + [TestMethod] + [Description("A result whose details are a page declares that page's footer, its last line, and no header: the message comes first.")] + public async Task When_AResultCarriesAPage_Then_ItsFooterIsDeclaredAndLast() + { + var page = new ReplPage( + [new JsonObject { ["id"] = 1 }], + new ReplPageInfo(Cursor: null, NextCursor: "2", TotalCount: 5, PageSize: 1)); + + var rendered = await CreateTransformer().RenderAsync(Results.Success("Found", page), CancellationToken.None); + + rendered.Text.Split(Environment.NewLine)[^1].Should().StartWith("Showing 1 of 5."); + rendered.Layout.HeaderLineCount.Should().Be(0); + rendered.Layout.FooterLineCount.Should().Be(1); + } - output.Split(Environment.NewLine)[0].Should().MatchRegex(@"^id\s*$"); - output.Should().NotContain("{", "the rows render as table cells, not as JSON literals"); + [TestMethod] + [Description("A display name with a line break still makes a one-line header: the declared height must match what is rendered.")] + public async Task When_ADisplayNameHasALineBreak_Then_TheHeaderStaysOnOneLine() + { + var rendered = await CreateTransformer().RenderAsync(new[] { new Stamped("now") }, CancellationToken.None); + + // Split the way the pager splits, at any line break, lone ones included. + var lines = rendered.Text.ReplaceLineEndings("\n").Split('\n'); + lines.Should().HaveCount(3, "the header, its separator and the row"); + lines[0].Should().MatchRegex("^Created At *$"); + rendered.Layout.HeaderLineCount.Should().Be(2); + } + + [TestMethod] + [Description("Through the pager, a page whose last row is a JSON null keeps that row: it reads null rather than being trimmed as a trailing blank line.")] + public async Task When_ThePagerReceivesAPageEndingInANullRow_Then_TheRowIsKept() + { + var page = new ReplPage( + [new JsonObject { ["id"] = 1 }, null], + new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 2)); + + var session = await PageThroughAsync(CreateTransformer(), page, SingleRowPage(new JsonObject { ["id"] = 2 })); + + session.Lines.Should().Contain("null"); + } + + [TestMethod] + [Description("Through the pager, a column named 1 holding 1 renders its header and its row alike; the row is data, and must not be dropped as a repeated header.")] + public async Task When_ARowReadsLikeTheHeader_Then_ThePagerKeepsIt() + { + var session = await PageThroughAsync( + CreateTransformer(), + SingleRowPage(new JsonObject { ["1"] = 1 }), + SingleRowPage(new JsonObject { ["1"] = 1 })); + + session.Lines.Should().Equal("1", "1"); + } + + [TestMethod] + [Description("Through the ANSI pager, keys 'a b' / 'c' and 'a' / 'b c' render headers that read alike but name other columns: the continuation keeps its own.")] + public async Task When_TheAnsiPagerAppendsKeysThatRegroupTheirSpaces_Then_TheContinuationKeepsItsHeader() + { + var session = await PageThroughAsync( + CreateTransformer(useAnsi: true), + SingleRowPage(new JsonObject { ["a b"] = 1, ["c"] = 2 }), + SingleRowPage(new JsonObject { ["a"] = 3, ["b c"] = 4 })); + + session.Lines.Should().HaveCount(3, "the first row, then the continuation's own header and its row"); } [TestMethod] @@ -293,18 +404,13 @@ public async Task When_AJsonTypedPropertyHasANullDisplayText_Then_ItIsUsed() } [TestMethod] - [Description("With ANSI on, the table header is styled bold within a combined sequence; the pager must still recognize it, or every JSON page, which carries its own header, would repeat it.")] + [Description("With ANSI on, the table header has no separator line and a palette style; the pager must still pin it, or every JSON page, which carries its own header, would repeat it.")] public async Task When_AnAnsiPagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsAdded() { - var transformer = new HumanOutputTransformer( - () => new HumanRenderSettings(Width: 120, UseAnsi: true, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); - var first = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), ResultFlowPageRenderMode.Initial, CancellationToken.None); - var next = await transformer.TransformPageAsync( - SingleRowPage(new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" }), ResultFlowPageRenderMode.Continuation, CancellationToken.None); - - var session = new PagerSession(first, hasMorePayload: true, maxBufferedLines: 100); - session.Append(next, hasMorePayload: false, containsPresentationChrome: false); + var session = await PageThroughAsync( + CreateTransformer(useAnsi: true), + SingleRowPage(new JsonObject { ["id"] = 1, ["name"] = "a" }), + SingleRowPage(new JsonObject { ["id"] = 22, ["name"] = "bbbbbb" })); session.HeaderLines.Should().ContainSingle("the styled header line is the pinned header"); session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); @@ -319,15 +425,33 @@ private sealed record Holder(string Name, JsonObject Payload); private sealed record Owner(string Login); + private sealed record Separated([property: System.ComponentModel.DataAnnotations.Display(Name = "Created\u2028At")] string When); + + private sealed record Stamped([property: System.ComponentModel.DataAnnotations.Display(Name = "Created\nAt")] string When); + private static ReplPage SingleRowPage(JsonObject row) => new([row], new ReplPageInfo(Cursor: null, NextCursor: null, TotalCount: null, PageSize: 1)); - private static HumanOutputTransformer CreateTransformer() => + private static HumanOutputTransformer CreateTransformer(bool useAnsi = false) => new(() => new HumanRenderSettings( Width: 120, - UseAnsi: false, + UseAnsi: useAnsi, Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + // Drives pages through a PagerSession the way the pager does, each with the layout its transformer declared. + private static async Task PageThroughAsync(HumanOutputTransformer transformer, params IReplPage[] pages) + { + var initial = await transformer.RenderPageAsync(pages[0], CancellationToken.None).ConfigureAwait(false); + var session = new PagerSession(initial.Text, hasMorePayload: pages.Length > 1, maxBufferedLines: 100, initial.Layout); + for (var i = 1; i < pages.Length; i++) + { + var next = await transformer.RenderPageAsync(pages[i], CancellationToken.None).ConfigureAwait(false); + session.Append(next.Text, hasMorePayload: i < pages.Length - 1, containsPresentationChrome: false, next.Layout); + } + + return session; + } + private static async Task RenderAsync(object value) => await CreateTransformer().TransformAsync(value, CancellationToken.None).ConfigureAwait(false); diff --git a/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs b/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs index c1c99a7b..0ba32f18 100644 --- a/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs +++ b/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs @@ -6,14 +6,10 @@ namespace Repl.Tests; public sealed class Given_ResultFlowOutputTransformer { [TestMethod] - [Description("Result-flow continuation payloads render table rows only so pagers do not receive repeated headers or page footers.")] - public async Task When_RenderingHumanContinuationPage_Then_HeaderAndFooterAreOmitted() + [Description("A page rendered for the pager carries its table header on every page, declared with the columns it names, and no footer: the pager, not the transformer, drops a header that repeats the previous page's columns.")] + public async Task When_RenderingAHumanPageForThePager_Then_ItsHeaderIsDeclaredAndItsFooterOmitted() { - var transformer = new HumanOutputTransformer( - () => new HumanRenderSettings( - Width: 120, - UseAnsi: false, - Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + var transformer = CreateTransformer(); var page = new ReplPage( [ new ActivityRow( @@ -35,29 +31,22 @@ public async Task When_RenderingHumanContinuationPage_Then_HeaderAndFooterAreOmi TotalCount: 250, PageSize: 2)); - var output = await ((IResultFlowOutputTransformer)transformer).TransformPageAsync( - page, - ResultFlowPageRenderMode.Continuation, - CancellationToken.None); + var rendered = await ((IResultFlowOutputTransformer)transformer).RenderPageAsync(page, CancellationToken.None); - output.Should().NotContain("#"); - output.Should().NotContain("---"); - output.Should().NotContain("Showing "); - output.Should().Contain("49"); - output.Should().Contain("identity batch 10 validated successfully"); - output.Should().Contain("50"); - output.Should().Contain("billing batch 10 queued successfully"); + var lines = rendered.Text.Split(Environment.NewLine); + lines.Should().HaveCount(4, "the header, its separator and the two rows"); + lines[0].Should().MatchRegex(@"^#\s+At\s+Area\s+Event\s+Summary"); + rendered.Text.Should().NotContain("Showing "); + rendered.Layout.HeaderLineCount.Should().Be(2); + rendered.Layout.Columns.Should().Equal("#", "At", "Area", "Event", "Summary"); + rendered.Layout.FooterLineCount.Should().Be(0); } [TestMethod] - [Description("Result-flow initial payloads keep the table header so the first page remains readable.")] - public async Task When_RenderingHumanInitialPage_Then_HeaderIsIncluded() + [Description("A page rendered on its own keeps the footer asking to rerun for more, and declares it, so the pager strips exactly that line rather than any line that reads like one.")] + public async Task When_RenderingAHumanPageOnItsOwn_Then_ItsFooterIsDeclared() { - var transformer = new HumanOutputTransformer( - () => new HumanRenderSettings( - Width: 120, - UseAnsi: false, - Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + var transformer = CreateTransformer(); var page = new ReplPage( [ new ActivityRow( @@ -73,16 +62,19 @@ public async Task When_RenderingHumanInitialPage_Then_HeaderIsIncluded() TotalCount: 250, PageSize: 1)); - var output = await ((IResultFlowOutputTransformer)transformer).TransformPageAsync( - page, - ResultFlowPageRenderMode.Initial, - CancellationToken.None); + var rendered = await ((IResultFlowOutputTransformer)transformer).RenderAsync(page, CancellationToken.None); - output.Should().Contain("#"); - output.Should().Contain("At"); - output.Should().NotContain("Showing "); + rendered.Text.Split(Environment.NewLine)[^1].Should().StartWith("Showing 1 of 250."); + rendered.Layout.HeaderLineCount.Should().Be(2); + rendered.Layout.FooterLineCount.Should().Be(1); } + private static HumanOutputTransformer CreateTransformer() => + new(() => new HumanRenderSettings( + Width: 120, + UseAnsi: false, + Palette: new DefaultAnsiPaletteProvider().Create(ThemeMode.Dark))); + [TestMethod] [Description("Human page footers never render unsafe cursor text directly.")] public async Task When_HumanPageFooterHasUnsafeCursor_Then_CursorIsNotRenderedVerbatim() diff --git a/src/Repl.Tests/Given_ResultFlowPager.cs b/src/Repl.Tests/Given_ResultFlowPager.cs index f866e513..3dddd4a5 100644 --- a/src/Repl.Tests/Given_ResultFlowPager.cs +++ b/src/Repl.Tests/Given_ResultFlowPager.cs @@ -1037,147 +1037,130 @@ public void When_HeaderContainsLoneEscape_Then_NormalizationStillDeduplicatesCon } [TestMethod] - [Description("A continuation header that repeats the pinned one, only padded to other column widths, is still dropped as a duplicate.")] - public void When_AContinuationHeaderRepeatsThePinnedOneAtOtherWidths_Then_ItIsDropped() + [Description("A declared header is dropped from a page that names the previous page's columns, whatever its text: a plain Spectre header has neither a separator nor styling to be recognized by.")] + public void When_ADeclaredHeaderNamesThePreviousColumns_Then_ItIsDropped() { - var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); - var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "--- ----", "100 b"), first.Header); + var layout = Declared(1, "id", "name"); + var first = PagerPayloadParser.ParseDeclared(Lines("id name", "1 a"), header: null, previousLayout: null, layout); + var second = PagerPayloadParser.ParseDeclared(Lines("id name", "22 bb"), first.Header, layout, Declared(1, "id", "name")); - second.ContentLines.Should().Equal("100 b"); + first.Header.Lines.Should().Equal("id name"); + first.ContentLines.Should().Equal("1 a"); + second.ContentLines.Should().Equal("22 bb"); } [TestMethod] - [Description("A continuation header that names other columns than the pinned one is kept: without it the page's rows would sit under headings that are not theirs.")] - public void When_AContinuationHeaderDiffersFromThePinnedOne_Then_ItIsKept() + [Description("A declared header naming other columns is kept, even when its text reads the same: keys 'a b' / 'c' and 'a' / 'b c' render alike.")] + public void When_ADeclaredHeaderNamesOtherColumns_Then_ItIsKept() { - var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); - var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "name id", "---- --", "b 2"), first.Header); + var layout = Declared(1, "a b", "c"); + var first = PagerPayloadParser.ParseDeclared(Lines("a b c", "1 2"), header: null, previousLayout: null, layout); + var second = PagerPayloadParser.ParseDeclared(Lines("a b c", "3 4"), first.Header, layout, Declared(1, "a", "b c")); - second.ContentLines.Should().Equal("name id", "---- --", "b 2"); + second.ContentLines.Should().Equal("a b c", "3 4"); } [TestMethod] - [Description("Labels are compared column by column, as the separator spans them: 'first' / 'last name' and 'first last' / 'name' have the same words but are other columns, so the continuation keeps its header.")] - public void When_AContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + [Description("A page is compared with the page before it, not with the pinned one: after columns A then B, a page back on A must show its header again, since the last header in view names B.")] + public void When_ColumnsGoBackToThePinnedOnes_Then_TheHeaderIsShownAgain() { - var first = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, "first last name", "----- ---------", "a b"), header: null); - var second = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, "first last name", "---------- ----", "c d"), first.Header); + var a = Declared(1, "id", "name"); + var b = Declared(1, "name", "id"); + var first = PagerPayloadParser.ParseDeclared(Lines("id name", "1 a"), header: null, previousLayout: null, a); + var third = PagerPayloadParser.ParseDeclared(Lines("id name", "3 c"), first.Header, b, a); - second.ContentLines.Should().Equal("first last name", "---------- ----", "c d"); + third.ContentLines.Should().Equal("id name", "3 c"); } [TestMethod] - [Description("A narrow table leaves a single space between columns, so both headers read 'first last name': only the separator's column spans tell 'first' / 'last name' from 'first last' / 'name'.")] - public void When_ANarrowContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + [Description("Under a declared header, a data line that reads like the header is data: a column named 1 holding 1 renders its header and its row alike.")] + public void When_ADeclaredPageHasARowLikeItsHeader_Then_TheRowIsKept() { - var first = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, "first last name", "----- ---------", "a b"), header: null); - var second = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, "first last name", "---------- ----", "c d"), first.Header); + var layout = Declared(2, "1"); + var first = PagerPayloadParser.ParseDeclared(Lines("1", "-", "1"), header: null, previousLayout: null, layout); + var second = PagerPayloadParser.ParseDeclared(Lines("1", "-", "1"), first.Header, layout, layout); - second.ContentLines.Should().Equal("first last name", "---------- ----", "c d"); + first.ContentLines.Should().Equal("1"); + second.ContentLines.Should().Equal("1"); } [TestMethod] - [Description("A styled header, as Spectre writes it, has no separator: each label is its own styled run, spaces included, so regrouped words are other columns there too.")] - public void When_AStyledContinuationHeaderRegroupsThePinnedWords_Then_ItIsKept() + [Description("A declared footer is exactly the lines it names: a data line that reads like a footer stays.")] + public void When_APayloadDeclaresItsFooter_Then_OnlyThoseLinesAreStripped() { - var first = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, Bold("first") + " " + Bold("last name"), "a b"), header: null); - var next = Bold("first last") + " " + Bold("name"); - var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, next, "c d"), first.Header); + var parsed = PagerPayloadParser.ParseDeclared( + Lines("Showing 1 of 2.", "Showing 2 of 9. Next data page: rerun with --result:cursor 2."), + header: null, + previousLayout: null, + RenderedLayout.None.WithFooter(1)); - second.ContentLines.Should().Equal(next, "c d"); + parsed.ContentLines.Should().Equal("Showing 1 of 2."); } [TestMethod] - [Description("A styled continuation header with the pinned labels, padded to other widths, is still dropped as a duplicate.")] - public void When_AStyledContinuationHeaderRepeatsThePinnedOne_Then_ItIsDropped() + [Description("A page with no declared header, a list between two tables for instance, adds its lines only.")] + public void When_APageDeclaresNoHeader_Then_AllItsLinesAreContent() { - var first = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, Bold("id") + " " + Bold("last name"), "1 a"), header: null); - var second = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, Bold("id ") + " " + Bold("last name") + " ", "100 b"), first.Header); + var layout = Declared(2, "id", "name"); + var first = PagerPayloadParser.ParseDeclared(Lines("id name", "-- ----", "1 a"), header: null, previousLayout: null, layout); + var second = PagerPayloadParser.ParseDeclared(Lines("2 b"), first.Header, layout, RenderedLayout.None); - second.ContentLines.Should().Equal("100 b"); + first.Header.Lines.Should().Equal("id name", "-- ----"); + second.ContentLines.Should().Equal("2 b"); } [TestMethod] - [Description("A header bolded as one run, its padding inside the styling, is still recognized as a repeat at other widths: the labels inside the run are split at their gaps.")] - public void When_AWholeLineBoldHeaderRepeatsAtOtherWidths_Then_ItIsDropped() + [Description("A header declared taller than the payload takes the lines there are, rather than reading past them.")] + public void When_ADeclaredHeaderIsTallerThanThePayload_Then_ItTakesTheLinesThereAre() { - var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, Bold("id name"), "1 a"), header: null); - var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, Bold("id name"), "1000 b"), first.Header); + var parsed = PagerPayloadParser.ParseDeclared(Lines("id"), header: null, previousLayout: null, Declared(2, "id")); - second.ContentLines.Should().Equal("1000 b"); + parsed.Header.Lines.Should().Equal("id"); + parsed.ContentLines.Should().BeEmpty(); } [TestMethod] - [Description("Bold within a combined SGR sequence, as the human palette styles its table header, marks a header line too.")] - public void When_TheFirstLineIsBoldWithinACombinedSequence_Then_ItIsTheHeader() + [Description("A layout holds only what a rendering can have: no negative line counts, and a header exactly when there are columns.")] + public void When_ALayoutIsInconsistent_Then_ItCannotBeCreated() { - var parsed = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, $"{(char)27}[1;38;5;221mid name{(char)27}[0m", "1 a"), header: null); - - parsed.Header.Lines.Should().ContainSingle(); - parsed.ContentLines.Should().Equal("1 a"); - } - - [TestMethod] - [Description("A 1 that is an extended colour's argument, as in 38;5;1, is a colour, not bold: that line is content.")] - public void When_TheFirstLineUsesColourIndexOne_Then_ItIsNotAHeader() - { - var colored = $"{(char)27}[38;5;1mred{(char)27}[0m"; - var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, colored, "next"), header: null); - - parsed.Header.Lines.Should().BeEmpty(); - parsed.ContentLines.Should().Equal(colored, "next"); - } - - [TestMethod] - [Description("A single payload, such as a long string result, keeps a line identical to its first one: only a continuation page repeats a header.")] - public void When_APayloadRepeatsItsBoldFirstLine_Then_BothAreKept() - { - var title = $"{(char)27}[1;31mSection{(char)27}[0m"; - var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, title, "one", title, "two"), header: null); - - parsed.ContentLines.Should().Equal(["one", title, "two"], "the first line may be pinned, but its repeat inside the payload is data"); + FluentActions.Invoking(() => new RenderedLayout(-1, [])).Should().Throw(); + FluentActions.Invoking(() => new RenderedLayout(0, [], footerLineCount: -1)).Should().Throw(); + FluentActions.Invoking(() => new RenderedLayout(1, [])).Should().Throw(); + FluentActions.Invoking(() => new RenderedLayout(0, ["id"])).Should().Throw(); } [TestMethod] - [Description("A first line holding only a style opener has no header text; taken as a header, it would drop every blank line after it.")] - public void When_TheFirstLineIsOnlyAStyleOpener_Then_BlankLinesAreKept() + [Description("A custom pager renderer gets fetched pages as text only: a declared header naming the previous page's columns is stripped for it, as the built-in pager drops it, and one naming other columns is kept.")] + public async Task When_ACustomRendererFetchesDeclaredPages_Then_RepeatedHeadersAreStripped() { - var parsed = PagerPayloadParser.Parse( - string.Join(Environment.NewLine, $"{(char)27}[1;37m", "one", string.Empty, "two"), header: null); - - parsed.Header.Lines.Should().BeEmpty(); - parsed.ContentLines.Should().Contain(string.Empty); - } + var renderer = new FetchingPagerRenderer(ReplPagerMode.More); + var pages = new Queue( + [ + new(Lines("id name", "-- ----", "22 bb"), HasMore: true, ContainsPresentationChrome: false, Declared(2, "id", "name")), + new(Lines("name id", "---- --", "c 3"), HasMore: false, ContainsPresentationChrome: false, Declared(2, "name", "id")), + ]); - [TestMethod] - [Description("Bold that only starts inside the line, after other text, does not style a header: that line is content.")] - public void When_ALineIsBoldOnlyAfterOtherText_Then_ItIsNotAHeader() - { - var line = $"note: {(char)27}[1;31mX{(char)27}[0m"; - var parsed = PagerPayloadParser.Parse(string.Join(Environment.NewLine, line, "next"), header: null); + await ResultFlowPager.WriteAsync( + Lines("id name", "-- ----", "1 a"), + new StringWriter(), + new FakeKeyReader([]), + new ResultFlowPagerOptions + { + VisibleRows = 5, + PagerMode = ReplPagerMode.More, + HasMorePayload = true, + PayloadLayout = Declared(2, "id", "name"), + FetchNextPayload = _ => ValueTask.FromResult(pages.TryDequeue(out var page) ? page : null), + PagerRenderers = [renderer], + }, + CancellationToken.None); - parsed.Header.Lines.Should().BeEmpty(); + renderer.Fetched.Should().Equal("22 bb", Lines("name id", "---- --", "c 3")); } - private static string Bold(string text) => $"{(char)27}[1m{text}{(char)27}[0m"; + private static string Lines(params string[] lines) => string.Join(Environment.NewLine, lines); - [TestMethod] - [Description("A kept continuation header keeps its separator, even when the column widths make that separator identical to the pinned one: without it the header reads as a data row.")] - public void When_AKeptContinuationHeaderHasThePinnedSeparator_Then_TheSeparatorIsKeptToo() - { - var first = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id name", "-- ----", "1 a"), header: null); - var second = PagerPayloadParser.Parse(string.Join(Environment.NewLine, "id kind", "-- ----", "2 b"), first.Header); - - second.ContentLines.Should().Equal("id kind", "-- ----", "2 b"); - } + private static RenderedLayout Declared(int headerLineCount, params string[] columns) => new(headerLineCount, columns); private static ValueTask WritePagerAsync( string payload, @@ -1343,6 +1326,26 @@ private static ValueTask WritePagerAsync( private static ConsoleKeyInfo MakeKey(ConsoleKey key, char keyChar) => new(keyChar, key, shift: false, alt: false, control: false); + private sealed class FetchingPagerRenderer(ReplPagerMode mode) : IReplPagerRenderer + { + public List Fetched { get; } = []; + + public ReplPagerMode Mode { get; } = mode; + + public async ValueTask RenderAsync(ReplPagerRenderContext context, CancellationToken cancellationToken = default) + { + while (context.CanFetchNextPayload + && await context.FetchNextPayloadAsync(cancellationToken).ConfigureAwait(false) is { } next) + { + Fetched.Add(next.Payload); + if (!next.HasMore) + { + break; + } + } + } + } + private sealed class RecordingPagerRenderer(ReplPagerMode mode) : IReplPagerRenderer { public List Payloads { get; } = []; From 8502ed93d974e7555daa9c7535073af9960d944c Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Thu, 24 Sep 2026 14:51:16 -0400 Subject: [PATCH 7/7] fix(output): keep the 0.11 page-rendering ABI, and render JsonElements that repeat a property name Addresses the fifth review of #111. IResultFlowOutputTransformer.TransformPageAsync(IReplPage, ResultFlowPageRenderMode, ...) shipped in 0.11.0 and is implemented by Repl.Spectre through InternalsVisibleTo. Commit 1297d28 replaced it with RenderAsync/RenderPageAsync and removed the enum, so a 0.11 Repl.Spectre loaded next to this Repl.Core failed with a TypeLoadException ("Method 'RenderAsync' ... does not have an implementation"). The repository already keeps friend-assembly ABI for this case (see HelpTextBuilder.BuildRenderModel). The interface and the enum are restored exactly as they shipped. The layout contract moves to a separate internal interface, ILayoutDeclaringOutputTransformer, which both human transformers now implement. CoreReplApp tries it first, then falls back to TransformPageAsync with the render mode, then to TransformAsync. For the last two, the pager detects headers from the text as before. Checked with a probe outside the repository: Repl.Spectre built at v0.11.0, loaded against this Repl.Core, implements the interface and renders Initial and Continuation pages. The same probe against 1297d28 throws the TypeLoadException above. An integration test pages through a transformer implementing only the 0.11 interface and asserts it is asked for Initial, then Continuation. Falsified: skipping the fallback leaves it asked for nothing. A JsonElement may repeat a property name, which JsonDocument accepts and a JsonObject cannot hold. Enumerating a JsonObject created over such an element threw ArgumentException, so human rendering failed. JsonElements are now copied into nodes property by property, and the last value given for a name wins, as JavaScript reads it. Scalars still refer to the element. The test was red with that exception first. Line and paragraph separators (U+2028/U+2029) in JSON keys and strings turned out to be escaped already by the relaxed encoder. Tests in both transformers now pin that. Refs #92 --- docs/output-system.md | 3 +- src/Repl.Core/CoreReplApp.Execution.cs | 32 +++++++--- .../ILayoutDeclaringOutputTransformer.cs | 21 +++++++ src/Repl.Core/IResultFlowOutputTransformer.cs | 21 +++---- .../Output/HumanOutputTransformer.cs | 2 +- src/Repl.Core/Output/JsonHumanShape.cs | 42 ++++++++++--- src/Repl.Core/ResultFlowPageRenderMode.cs | 7 +++ .../Given_OutputFormatting.cs | 59 +++++++++++++++++++ .../SpectreHumanOutputTransformer.cs | 2 +- .../Given_SpectreHumanOutputJson.cs | 10 ++++ src/Repl.Tests/Given_HumanOutputJson.cs | 25 ++++++++ .../Given_ResultFlowOutputTransformer.cs | 4 +- 12 files changed, 191 insertions(+), 37 deletions(-) create mode 100644 src/Repl.Core/ILayoutDeclaringOutputTransformer.cs create mode 100644 src/Repl.Core/ResultFlowPageRenderMode.cs diff --git a/docs/output-system.md b/docs/output-system.md index 862f629e..07a9adb1 100644 --- a/docs/output-system.md +++ b/docs/output-system.md @@ -41,7 +41,8 @@ data rather than the CLR members of those types: - A page declared with a JSON item type, such as `IReplPageSource`, renders as JSON even when every item on it is a JSON null. - Values are compact JSON literals. So a string shows as `"x"`, an explicit JSON null shows as `null`, and - a nested object or array shows as itself. + a nested object or array shows as itself. A `JsonElement` object that repeats a property name shows the + last value given for it, as JavaScript reads it. - A JSON value held by a property of an ordinary result object shows as a compact literal too, and a property declared as JSON (`JsonNode?`, `JsonElement?`) that holds `null` reads `null`, unless its `DisplayFormat` sets a `NullDisplayText`. A JSON value passed as a result's details renders like a JSON diff --git a/src/Repl.Core/CoreReplApp.Execution.cs b/src/Repl.Core/CoreReplApp.Execution.cs index 1dd097d0..ef55b3cf 100644 --- a/src/Repl.Core/CoreReplApp.Execution.cs +++ b/src/Repl.Core/CoreReplApp.Execution.cs @@ -1359,7 +1359,11 @@ private async ValueTask RenderPageSourcePagerAsync( CancellationToken cancellationToken) { var nextCursor = page.PageInfo.NextCursor; - var (initialPayload, initialLayout) = await RenderPagerPageAsync(transformer, page, cancellationToken) + var (initialPayload, initialLayout) = await RenderPagerPageAsync( + transformer, + page, + ResultFlowPageRenderMode.Initial, + cancellationToken) .ConfigureAwait(false); var pagerPayload = TryColorizeStructuredPayload(initialPayload, transformer.Name, isInteractive); await ResultFlowPager.WriteAsync( @@ -1392,7 +1396,11 @@ await ResultFlowPager.WriteAsync( var nextRequest = request with { Cursor = nextCursor }; var nextPage = await FetchPageSourceAsync(source, nextRequest, token).ConfigureAwait(false); nextCursor = nextPage.PageInfo.NextCursor; - var (nextPayload, nextLayout) = await RenderPagerPageAsync(transformer, nextPage, token) + var (nextPayload, nextLayout) = await RenderPagerPageAsync( + transformer, + nextPage, + ResultFlowPageRenderMode.Continuation, + token) .ConfigureAwait(false); return new ResultFlowPagerPage( TryColorizeStructuredPayload(nextPayload, transformer.Name, isInteractive), @@ -1414,35 +1422,41 @@ await ResultFlowPager.WriteAsync( return await RefuseGlobalOptionErrorsAsync(globalOptions, cancellationToken).ConfigureAwait(false); } - // A result-flow transformer declares the layout it renders; for any other, the pager detects it from the text. + // A transformer that declares its layout says where its header and footer are; for any other, the pager + // detects them from the text. private static async ValueTask<(string Payload, RenderedLayout? Layout)> RenderPayloadAsync( IOutputTransformer transformer, object? value, CancellationToken cancellationToken) { - if (transformer is IResultFlowOutputTransformer resultFlowTransformer) + if (transformer is ILayoutDeclaringOutputTransformer declaring) { - var rendered = await resultFlowTransformer.RenderAsync(value, cancellationToken).ConfigureAwait(false); + var rendered = await declaring.RenderAsync(value, cancellationToken).ConfigureAwait(false); return (rendered.Text, rendered.Layout); } return (await transformer.TransformAsync(value, cancellationToken).ConfigureAwait(false), null); } + // A transformer built against 0.11 still gets the render mode that asks it to leave a continuation's header + // out, which the pager then detects on the first page only. private static async ValueTask<(string Payload, RenderedLayout? Layout)> RenderPagerPageAsync( IOutputTransformer transformer, IReplPage page, + ResultFlowPageRenderMode mode, CancellationToken cancellationToken) { var displayPage = CreatePagerDisplayPage(page); - if (transformer is IResultFlowOutputTransformer resultFlowTransformer) + if (transformer is ILayoutDeclaringOutputTransformer declaring) { - var rendered = await resultFlowTransformer.RenderPageAsync(displayPage, cancellationToken) - .ConfigureAwait(false); + var rendered = await declaring.RenderPageAsync(displayPage, cancellationToken).ConfigureAwait(false); return (rendered.Text, rendered.Layout); } - return (await transformer.TransformAsync(displayPage, cancellationToken).ConfigureAwait(false), null); + var text = transformer is IResultFlowOutputTransformer resultFlowTransformer + ? await resultFlowTransformer.TransformPageAsync(displayPage, mode, cancellationToken).ConfigureAwait(false) + : await transformer.TransformAsync(displayPage, cancellationToken).ConfigureAwait(false); + return (text, null); } private async ValueTask WritePayloadAsync( diff --git a/src/Repl.Core/ILayoutDeclaringOutputTransformer.cs b/src/Repl.Core/ILayoutDeclaringOutputTransformer.cs new file mode 100644 index 00000000..26385837 --- /dev/null +++ b/src/Repl.Core/ILayoutDeclaringOutputTransformer.cs @@ -0,0 +1,21 @@ +namespace Repl; + +/// +/// A human output transformer that declares the layout of what it renders, so the pager can pin its header, drop +/// that header's repeats and strip its footer without inferring any of them from the text. +/// +/// +/// A table always renders its header, on every page: a page of JSON rows takes its columns from its own keys, so it +/// can name other columns than the page before it. The pager drops a header that repeats the previous page's +/// columns and keeps one that names others. This is a separate interface rather than new members of +/// : a friend assembly built against an earlier Repl.Core implements that +/// one as it shipped, and still has to load when only Repl.Core is upgraded. +/// +internal interface ILayoutDeclaringOutputTransformer : IOutputTransformer +{ + /// Renders as does, with its layout. + ValueTask RenderAsync(object? value, CancellationToken cancellationToken = default); + + /// Renders a page fetched for the pager: its items, without the footer that asks to rerun for more. + ValueTask RenderPageAsync(IReplPage page, CancellationToken cancellationToken = default); +} diff --git a/src/Repl.Core/IResultFlowOutputTransformer.cs b/src/Repl.Core/IResultFlowOutputTransformer.cs index 084ff593..855cb17c 100644 --- a/src/Repl.Core/IResultFlowOutputTransformer.cs +++ b/src/Repl.Core/IResultFlowOutputTransformer.cs @@ -1,19 +1,12 @@ namespace Repl; -/// -/// A human output transformer that declares the layout of what it renders, so the pager can pin its header, drop -/// that header's repeats and strip its footer without inferring any of them from the text. -/// -/// -/// A table always renders its header, on every page: a page of JSON rows takes its columns from its own keys, so it -/// can name other columns than the page before it. The pager drops a header that repeats the previous page's -/// columns and keeps one that names others. -/// +// Kept as it shipped in 0.11, with ResultFlowPageRenderMode: a friend assembly built against it, such as an older +// Repl.Spectre, implements this exact signature and must still load when only Repl.Core is upgraded. Transformers +// built with this version declare their layout through ILayoutDeclaringOutputTransformer instead. internal interface IResultFlowOutputTransformer : IOutputTransformer { - /// Renders as does, with its layout. - ValueTask RenderAsync(object? value, CancellationToken cancellationToken = default); - - /// Renders a page fetched for the pager: its items, without the footer that asks to rerun for more. - ValueTask RenderPageAsync(IReplPage page, CancellationToken cancellationToken = default); + ValueTask TransformPageAsync( + IReplPage page, + ResultFlowPageRenderMode mode, + CancellationToken cancellationToken = default); } diff --git a/src/Repl.Core/Output/HumanOutputTransformer.cs b/src/Repl.Core/Output/HumanOutputTransformer.cs index cbc281e0..54a667d3 100644 --- a/src/Repl.Core/Output/HumanOutputTransformer.cs +++ b/src/Repl.Core/Output/HumanOutputTransformer.cs @@ -7,7 +7,7 @@ namespace Repl; -internal sealed class HumanOutputTransformer : IResultFlowOutputTransformer +internal sealed class HumanOutputTransformer : ILayoutDeclaringOutputTransformer { private readonly Func _resolveRenderSettings; diff --git a/src/Repl.Core/Output/JsonHumanShape.cs b/src/Repl.Core/Output/JsonHumanShape.cs index c1a45273..cedabcae 100644 --- a/src/Repl.Core/Output/JsonHumanShape.cs +++ b/src/Repl.Core/Output/JsonHumanShape.cs @@ -41,8 +41,8 @@ public static bool IsJsonType(Type type) => typeof(JsonNode).IsAssignableFrom(type) || (Nullable.GetUnderlyingType(type) ?? type) == typeof(JsonElement); /// - /// Reads as a JSON node. A is wrapped, and the element - /// itself is never modified. A JSON null comes back as a node with + /// Reads as a JSON node. A is copied into nodes, and the + /// element itself is never modified. A JSON null comes back as a node with /// . /// public static bool TryGetNode(object? value, out JsonNode? node) @@ -53,13 +53,7 @@ public static bool TryGetNode(object? value, out JsonNode? node) node = jsonNode; return true; case JsonElement element: - node = element.ValueKind switch - { - JsonValueKind.Object => JsonObject.Create(element), - JsonValueKind.Array => JsonArray.Create(element), - JsonValueKind.Null or JsonValueKind.Undefined => null, - _ => JsonValue.Create(element), - }; + node = FromElement(element); return true; default: node = null; @@ -67,6 +61,36 @@ public static bool TryGetNode(object? value, out JsonNode? node) } } + // Built property by property rather than through JsonObject.Create: an element may repeat a property name, + // which a JsonObject cannot hold, and enumerating one created over such an element throws. The last value + // wins, as JavaScript reads it. Scalars keep referring to the element rather than copying its text. + private static JsonNode? FromElement(JsonElement element) + { + switch (element.ValueKind) + { + case JsonValueKind.Object: + var jsonObject = new JsonObject(); + foreach (var property in element.EnumerateObject()) + { + jsonObject[property.Name] = FromElement(property.Value); + } + + return jsonObject; + case JsonValueKind.Array: + var jsonArray = new JsonArray(); + foreach (var item in element.EnumerateArray()) + { + jsonArray.Add(FromElement(item)); + } + + return jsonArray; + case JsonValueKind.Null or JsonValueKind.Undefined: + return null; + default: + return JsonValue.Create(element); + } + } + /// /// The literal for an item of a JSON collection: — a JSON null — reads /// null. for a value that is not JSON at all. diff --git a/src/Repl.Core/ResultFlowPageRenderMode.cs b/src/Repl.Core/ResultFlowPageRenderMode.cs new file mode 100644 index 00000000..cfa4289c --- /dev/null +++ b/src/Repl.Core/ResultFlowPageRenderMode.cs @@ -0,0 +1,7 @@ +namespace Repl; + +internal enum ResultFlowPageRenderMode +{ + Initial, + Continuation, +} diff --git a/src/Repl.IntegrationTests/Given_OutputFormatting.cs b/src/Repl.IntegrationTests/Given_OutputFormatting.cs index 188e5d4a..77ddc710 100644 --- a/src/Repl.IntegrationTests/Given_OutputFormatting.cs +++ b/src/Repl.IntegrationTests/Given_OutputFormatting.cs @@ -348,6 +348,65 @@ public void When_PagingALongPageReturnedWhole_Then_ItsRerunFooterIsShown() StripAnsi(output.ToString()).Should().Contain("Showing 10 of 20. Next data page: rerun with"); } + [TestMethod] + [Description("A transformer built against 0.11 implements the page-rendering interface as it shipped: it still pages, asked for the first page and then for continuations, whose header it leaves out.")] + public void When_PagingThroughATransformerBuiltAgainstTheEarlierInterface_Then_ItGetsEachRenderMode() + { + var legacy = new LegacyPageTransformer(); + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Output.AddTransformer("legacy", legacy); + options.Output.AddAlias("legacy", "legacy"); + }); + sut.Map("rows", (IReplPagingContext paging) => + paging.CreateSource((request, _) => + { + var index = request.Cursor is null ? 0 : 1; + return ValueTask.FromResult(new ReplPage( + [new ContactRow($"row {index}", "x@example.com")], + new ReplPageInfo(request.Cursor, index == 0 ? "1" : null, 2, request.PageSize))); + })); + + using var output = new StringWriter(); + using var session = ReplSessionIO.SetSession(output, TextReader.Null); + ReplSessionIO.KeyReader = new QueueKeyReader([.. Enumerable.Repeat(Key(ConsoleKey.Spacebar, ' '), 3)]); + ReplSessionIO.WindowSize = (100, 20); + + var exitCode = sut.Run(["rows", "--legacy", "--result:page-size=1", "--no-logo"]); + + exitCode.Should().Be(0); + legacy.Modes.Should().Equal(ResultFlowPageRenderMode.Initial, ResultFlowPageRenderMode.Continuation); + output.ToString().Should().Contain("row 0").And.Contain("row 1"); + } + + private sealed class LegacyPageTransformer : IResultFlowOutputTransformer + { + public List Modes { get; } = []; + + public string Name => "legacy"; + + public bool SupportsInteractivePaging => true; + + public ValueTask TransformAsync(object? value, CancellationToken cancellationToken = default) => + ValueTask.FromResult(value is IReplPage page ? Render(page, withHeader: true) : string.Empty); + + public ValueTask TransformPageAsync( + IReplPage page, + ResultFlowPageRenderMode mode, + CancellationToken cancellationToken = default) + { + Modes.Add(mode); + return ValueTask.FromResult(Render(page, withHeader: mode == ResultFlowPageRenderMode.Initial)); + } + + private static string Render(IReplPage page, bool withHeader) => + string.Join( + Environment.NewLine, + (withHeader ? ["Name", "----"] : Array.Empty()) + .Concat(page.UntypedItems.OfType().Select(row => row.Name))); + } + private static bool IsHeader(string line, params string[] columns) => line.Split(' ', StringSplitOptions.RemoveEmptyEntries).SequenceEqual(columns, StringComparer.Ordinal); diff --git a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs index b6fb7762..3bc99522 100644 --- a/src/Repl.Spectre/SpectreHumanOutputTransformer.cs +++ b/src/Repl.Spectre/SpectreHumanOutputTransformer.cs @@ -12,7 +12,7 @@ namespace Repl.Spectre; /// /// Output transformer that renders values using light Spectre.Console layouts. /// -internal sealed class SpectreHumanOutputTransformer : IResultFlowOutputTransformer +internal sealed class SpectreHumanOutputTransformer : ILayoutDeclaringOutputTransformer { private readonly Func _resolveRenderSettings; private readonly OutputOptions? _outputOptions; diff --git a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs index ae043829..f8761968 100644 --- a/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs +++ b/src/Repl.SpectreTests/Given_SpectreHumanOutputJson.cs @@ -332,6 +332,16 @@ public async Task When_AJsonTypedPropertyIsNull_Then_ItReadsNull() output.Should().MatchRegex(@"Payload\W+null"); } + [TestMethod] + [Description("Line and paragraph separators (U+2028, U+2029) in a JSON key or string are escaped too: the pager breaks lines at them, so left raw they would split a one-line header or row.")] + public async Task When_AJsonStringCarriesLineSeparators_Then_TheyAreEscaped() + { + var output = await RenderRawAsync(new JsonArray(new JsonObject { ["a\u2028b"] = "c\u2029d" })).ConfigureAwait(false); + + output.Should().NotContain("\u2028").And.NotContain("\u2029"); + output.Should().Contain(@"a\u2028b").And.Contain(@"c\u2029d"); + } + private sealed record NullableHolder(string Name, JsonNode? Payload); private sealed record Holder(string Name, JsonObject Payload); diff --git a/src/Repl.Tests/Given_HumanOutputJson.cs b/src/Repl.Tests/Given_HumanOutputJson.cs index 56ec4b41..144568fc 100644 --- a/src/Repl.Tests/Given_HumanOutputJson.cs +++ b/src/Repl.Tests/Given_HumanOutputJson.cs @@ -416,6 +416,31 @@ public async Task When_AnAnsiPagerAppendsAPageWithTheSameKeys_Then_OnlyItsRowIsA session.Lines.Should().HaveCount(2, "the first page's row and the continuation's row, with no repeated header"); } + [TestMethod] + [Description("Line and paragraph separators (U+2028, U+2029) in a JSON key or string are escaped too: the pager breaks lines at them, so left raw they would split a one-line header or row.")] + public async Task When_AJsonStringCarriesLineSeparators_Then_TheyAreEscaped() + { + var output = await RenderAsync(new JsonArray(new JsonObject { ["a\u2028b"] = "c\u2029d" })); + + output.Should().NotContain("\u2028").And.NotContain("\u2029"); + output.Should().Contain(@"a\u2028b").And.Contain(@"c\u2029d"); + } + + [TestMethod] + [Description("A JsonElement may repeat a property name, which a JsonObject cannot hold: the last value is the one shown, as JavaScript reads it, rather than rendering failing.")] + public async Task When_AJsonElementRepeatsAPropertyName_Then_TheLastValueIsShown() + { + using var document = JsonDocument.Parse("""{"id":1,"id":2,"nested":{"x":1,"x":3}}"""); + using var rows = JsonDocument.Parse("""[{"id":1,"id":2}]"""); + + var output = await RenderAsync(document.RootElement); + var table = await RenderAsync(rows.RootElement); + + output.Should().MatchRegex(@"id\s*:\s*2"); + output.Should().Contain("""{"x":3}"""); + table.Split(Environment.NewLine)[^1].Trim().Should().Be("2"); + } + private sealed record NullableHolder(string Name, JsonNode? Payload); private sealed record DisplayedHolder( diff --git a/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs b/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs index 0ba32f18..8bc828ae 100644 --- a/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs +++ b/src/Repl.Tests/Given_ResultFlowOutputTransformer.cs @@ -31,7 +31,7 @@ public async Task When_RenderingAHumanPageForThePager_Then_ItsHeaderIsDeclaredAn TotalCount: 250, PageSize: 2)); - var rendered = await ((IResultFlowOutputTransformer)transformer).RenderPageAsync(page, CancellationToken.None); + var rendered = await ((ILayoutDeclaringOutputTransformer)transformer).RenderPageAsync(page, CancellationToken.None); var lines = rendered.Text.Split(Environment.NewLine); lines.Should().HaveCount(4, "the header, its separator and the two rows"); @@ -62,7 +62,7 @@ public async Task When_RenderingAHumanPageOnItsOwn_Then_ItsFooterIsDeclared() TotalCount: 250, PageSize: 1)); - var rendered = await ((IResultFlowOutputTransformer)transformer).RenderAsync(page, CancellationToken.None); + var rendered = await ((ILayoutDeclaringOutputTransformer)transformer).RenderAsync(page, CancellationToken.None); rendered.Text.Split(Environment.NewLine)[^1].Should().StartWith("Showing 1 of 250."); rendered.Layout.HeaderLineCount.Should().Be(2);