From fe2bafd90d9e09eb61d3cbbedab78c6c7202b83b Mon Sep 17 00:00:00 2001 From: "a-team-app[bot]" <334837322+a-team-app[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:06:48 +1300 Subject: [PATCH] Show the measured cell size and its source in Diagnostics (#335) MeasureCell now reports a CellMeasurement (pixels, the reply that answered first, and iTerm2's scale) instead of a bare size, and takes a queue delegate so first-answer-wins is testable without a driver. F12 runs it on every open, like the cursor colour probe, and a legacy console reports the assumed default straight away. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 1 + src/TuiCode.Workbench/About/SixelProbe.cs | 43 +++++++++++---- .../Diagnostics/DiagnosticsView.cs | 31 ++++++++++- src/TuiCode.Workbench/WorkbenchHost.cs | 13 +++-- tests/TuiCode.Tests/AboutViewTests.cs | 54 ++++++++++++++++++- tests/TuiCode.Tests/DiagnosticsViewTests.cs | 43 +++++++++++++++ 6 files changed, 169 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5388b5b..1d4db42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,6 +190,7 @@ What follows is how TuiCode implements it today. - `tui` opens `AboutView`. If the terminal supports sixel (WezTerm, iTerm2, tmux 3.6, Windows Terminal, foot…) it shows the artwork as an image. Otherwise (kitty, Ghostty, Terminal.app — TG 2.1.0 has no kitty graphics) it shows ASCII art. - `SixelProbe.Detect` sends DA1 once at startup (`WorkbenchHost` ctor), so the dialog knows up front and shows a spinner, not ASCII art that gets replaced. `SixelProbe.MeasureCell` runs each time About opens, because the cell's pixel size changes when the window moves between a Retina and a non-Retina screen. We don't use TG's `SixelSupportDetector`: it asks `CSI 16 t` first and iTerm2 never answers, which costs TG's 1 s abandon timeout, and its fallback (window pixels ÷ cells) counts the title bar and margins. `MeasureCell` sends iTerm2's `OSC 1337 ; ReportCellSize` and `CSI 16 t` together and takes the first answer. `Detect` also parses tmux's DA1 reply (`…;4c`), which TG's check misses. - iTerm2 reports sizes in points everywhere, its `CSI 14 t` reply included. Only `ReportCellSize` carries the scale (`2.0` on Retina), so `ParseIterm2CellSize` multiplies by it (clamped to 1–4; absent in older iTerm2s, meaning 1). Sixels are drawn in device pixels, so ignoring it drew the About image at a quarter of its area (#333). +- F12 Diagnostics runs `MeasureCell` too each time it opens and shows the size with the reply it came from (`CellMeasurement.Source`), or that none came and 10 × 20 was assumed (#335). - The cell size has to be exact: iTerm2 blanks every row an image touches, so an image that ends mid-row leaves a dark band (`AboutImage.Fit` rounds down to whole rows and `Cover` trims the top/bottom to match). - Encoding runs in the background and is cached per pixel size, so reopening shows the image immediately. - TG re-emits queued sixels on every output write and only rewrites cells whose contents changed, so `AboutView.Dispose` dequeues its sixel and sets `ClearScreenNextIteration`. Otherwise the image stays on screen after close. diff --git a/src/TuiCode.Workbench/About/SixelProbe.cs b/src/TuiCode.Workbench/About/SixelProbe.cs index 4c9d85d..26b97d8 100644 --- a/src/TuiCode.Workbench/About/SixelProbe.cs +++ b/src/TuiCode.Workbench/About/SixelProbe.cs @@ -10,6 +10,10 @@ internal sealed record SixelSupport(bool IsSupported, SizeF CellPixels) public static readonly SixelSupport Unsupported = new(false, SizeF.Empty); } +internal enum CellSizeSource { Iterm2Report, CellResolutionReply, Assumed } + +internal sealed record CellMeasurement(SizeF Pixels, CellSizeSource Source, float Scale = 1); + internal static class SixelProbe { private static readonly SizeF DefaultCellPixels = new(10, 20); @@ -29,12 +33,23 @@ public static void Detect(IDriver driver, Action found) // iTerm2 answers only its own query and most other terminals only CSI 16 t. An unanswered query takes TG a // second to abandon, so ask both at once and take the first answer. /// Asks how many pixels a cell is. - public static void MeasureCell(IDriver driver, Action found) + public static void MeasureCell(IDriver driver, Action found) + { + if (driver.IsLegacyConsole) + { + found(new CellMeasurement(DefaultCellPixels, CellSizeSource.Assumed)); + return; + } + + MeasureCell(driver.QueueAnsiRequest, found); + } + + internal static void MeasureCell(Action queue, Action found) { var settled = false; var misses = 0; - driver.QueueAnsiRequest(new AnsiEscapeSequenceRequest + queue(new AnsiEscapeSequenceRequest { Request = $"{EscSeqUtils.OSC}1337;ReportCellSize{EscSeqUtils.ST}", Value = "1337", @@ -42,28 +57,33 @@ public static void MeasureCell(IDriver driver, Action found) ResponseReceived = response => Answer(ParseIterm2CellSize(response)), Abandoned = () => Answer(null), }); - Queue(driver, EscSeqUtils.CSI_RequestSixelResolution, - response => Answer(ParseCellResolution(response)), + Queue(queue, EscSeqUtils.CSI_RequestSixelResolution, + response => Answer(ParseCellResolution(response) is { } size + ? new CellMeasurement(size, CellSizeSource.CellResolutionReply) + : null), () => Answer(null)); - void Answer(SizeF? cellPixels) + void Answer(CellMeasurement? measurement) { if (settled) return; - if (cellPixels is { } size) + if (measurement is not null) { settled = true; - found(size); + found(measurement); } else if (++misses == 2) { settled = true; - found(DefaultCellPixels); + found(new CellMeasurement(DefaultCellPixels, CellSizeSource.Assumed)); } } } private static void Queue(IDriver driver, AnsiEscapeSequence sequence, Action received, Action abandoned) => - driver.QueueAnsiRequest(new AnsiEscapeSequenceRequest + Queue(driver.QueueAnsiRequest, sequence, received, abandoned); + + private static void Queue(Action queue, AnsiEscapeSequence sequence, Action received, Action abandoned) => + queue(new AnsiEscapeSequenceRequest { Request = sequence.Request, Value = sequence.Value, @@ -84,12 +104,13 @@ internal static bool IndicatesSixel(string? response) => } /// Reads iTerm2's OSC 1337 ; ReportCellSize=height;width;scale ST, in points times the scale. - internal static SizeF? ParseIterm2CellSize(string? response) + internal static CellMeasurement? ParseIterm2CellSize(string? response) { var match = Regex.Match(response ?? "", @"ReportCellSize=([\d.]+);([\d.]+)(?:;([\d.]+))?"); if (!match.Success || Positive(match.Groups[2].Value, match.Groups[1].Value) is not { } points) return null; var scale = float.TryParse(match.Groups[3].Value, CultureInfo.InvariantCulture, out var s) ? Math.Clamp(s, 1, 4) : 1; - return scale == 1 ? points : new SizeF(MathF.Round(points.Width * scale), MathF.Round(points.Height * scale)); + var pixels = scale == 1 ? points : new SizeF(MathF.Round(points.Width * scale), MathF.Round(points.Height * scale)); + return new CellMeasurement(pixels, CellSizeSource.Iterm2Report, scale); } private static SizeF? Positive(string width, string height) => diff --git a/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs b/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs index ece96d7..e3da573 100644 --- a/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs +++ b/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs @@ -1,5 +1,7 @@ +using System.Globalization; using Terminal.Gui.Drivers; using TuiCode.Abstractions; +using TuiCode.Workbench.About; using TuiCode.Workbench.Services; namespace TuiCode.Workbench.Diagnostics; @@ -16,6 +18,7 @@ public sealed class DiagnosticsView : Window private readonly Label _keyBase; private readonly Label _keyRune; private readonly Label _cursorColour; + private readonly Label _cellSize; private readonly string? _askedCursorColour; public IKeybindingService Scope => _scopeKeybindings; @@ -35,7 +38,8 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? var kittyStatusLines = WrapText(kittyNegotiationStatus, GetFieldWidth()); var kittyStatusText = string.Join("\n", kittyStatusLines); var cursorColourY = 2 + kittyStatusLines.Length; - var keyHeadingY = cursorColourY + 2; + var cellSizeY = cursorColourY + 1; + var keyHeadingY = cellSizeY + 2; var nameY = keyHeadingY + 1; var hintY = nameY + 6; Height = hintY + 6; @@ -73,6 +77,15 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? Text = cursorColour is null ? "not set by the theme" : $"asked {cursorColour} • asking the terminal…", }; + var cellSizeHeading = Heading("Cell size", cellSizeY); + _cellSize = new Label + { + X = FieldX, + Y = cellSizeY, + Width = GetFieldWidth(), + Text = "asking the terminal…", + }; + var keyHeading = Heading("Last key", keyHeadingY); var nameLabel = Heading(" Name", nameY); var hexLabel = Heading(" Hex", nameY + 1); @@ -100,6 +113,7 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? Add(driverHeading, driverNameLabel, kittyHeading, kittyStatusLabel, cursorColourHeading, _cursorColour, + cellSizeHeading, _cellSize, keyHeading, nameLabel, hexLabel, baseLabel, runeLabel, _keyName, _keyHex, _keyBase, _keyRune, hint, footer); @@ -162,6 +176,21 @@ public void ShowTerminalCursorColour(string? reported) + (CursorColourProbe.Matches(_askedCursorColour, reported) ? "✓" : "✗"); } + public string CellSizeText => _cellSize.Text; + + internal void ShowCellSize(CellMeasurement cell) => _cellSize.Text = DescribeCellSize(cell); + + private static string DescribeCellSize(CellMeasurement cell) + { + var source = cell.Source switch + { + CellSizeSource.Iterm2Report => string.Create(CultureInfo.InvariantCulture, $"iTerm2 report, scale {cell.Scale:0.0##}"), + CellSizeSource.CellResolutionReply => "CSI 16 t", + _ => "no answer, assumed", + }; + return string.Create(CultureInfo.InvariantCulture, $"{cell.Pixels.Width:0.#} × {cell.Pixels.Height:0.#} px • {source}"); + } + public void UpdateLastKey(Key key) { var raw = (uint)key.KeyCode; diff --git a/src/TuiCode.Workbench/WorkbenchHost.cs b/src/TuiCode.Workbench/WorkbenchHost.cs index ce6e85d..db0d05e 100644 --- a/src/TuiCode.Workbench/WorkbenchHost.cs +++ b/src/TuiCode.Workbench/WorkbenchHost.cs @@ -1476,9 +1476,9 @@ private void PresentAbout(AboutView view) view.Present(null); // Measured on every open: the window may have moved to a screen with another scale since the last one. - SixelProbe.MeasureCell(driver, cellPixels => _app.Invoke(() => + SixelProbe.MeasureCell(driver, cell => _app.Invoke(() => { - if (ReferenceEquals(_activeAbout, view)) view.Present(new SixelSupport(true, cellPixels)); + if (ReferenceEquals(_activeAbout, view)) view.Present(new SixelSupport(true, cell.Pixels)); })); } @@ -2482,11 +2482,18 @@ private void OpenDiagnostics() _scopes.Push(view.Scope); view.SetFocus(); - if (_cursorColour is not null && _app.Driver is { } driver) + if (_app.Driver is not { } driver) return; + + if (_cursorColour is not null) CursorColourProbe.Read(driver, reported => _app.Invoke(() => { if (ReferenceEquals(_activeDiagnostics, view)) view.ShowTerminalCursorColour(reported); })); + + SixelProbe.MeasureCell(driver, cell => _app.Invoke(() => + { + if (ReferenceEquals(_activeDiagnostics, view)) view.ShowCellSize(cell); + })); } private string GetKittyNegotiationStatus() diff --git a/tests/TuiCode.Tests/AboutViewTests.cs b/tests/TuiCode.Tests/AboutViewTests.cs index aad81b0..f8f64ca 100644 --- a/tests/TuiCode.Tests/AboutViewTests.cs +++ b/tests/TuiCode.Tests/AboutViewTests.cs @@ -1,4 +1,5 @@ using Terminal.Gui.Drawing; +using Terminal.Gui.Drivers; using Terminal.Gui.Views; using TuiCode.Workbench.About; @@ -82,7 +83,58 @@ public class SixelProbeTests public void ParseIterm2CellSize_reads_the_cell_in_device_pixels(string reply, float width, float height) => Assert.Equal( new System.Drawing.SizeF(width, height), - SixelProbe.ParseIterm2CellSize($"\x1b]1337;ReportCellSize={reply}\x1b\\")); + SixelProbe.ParseIterm2CellSize($"\x1b]1337;ReportCellSize={reply}\x1b\\")?.Pixels); + + [Theory] + [InlineData("17.50;8.00;2.0", 2f)] + [InlineData("17.50;8.00;1.0", 1f)] + [InlineData("17.50;8.00", 1f)] + public void ParseIterm2CellSize_reports_the_scale_it_used(string reply, float scale) + { + var cell = SixelProbe.ParseIterm2CellSize($"\x1b]1337;ReportCellSize={reply}\x1b\\"); + + Assert.Equal(CellSizeSource.Iterm2Report, cell?.Source); + Assert.Equal(scale, cell?.Scale); + } + + [Fact] + public void MeasureCell_reports_the_query_that_answered_first() + { + var requests = new List(); + var found = new List(); + SixelProbe.MeasureCell(requests.Add, found.Add); + + requests[1].ResponseReceived!("\x1b[6;20;10t"); + requests[0].ResponseReceived!("\x1b]1337;ReportCellSize=17.5;8.0;2.0\x1b\\"); + + Assert.Equal([new CellMeasurement(new System.Drawing.SizeF(10, 20), CellSizeSource.CellResolutionReply)], found); + } + + [Fact] + public void MeasureCell_takes_the_other_query_when_the_first_goes_unanswered() + { + var requests = new List(); + var found = new List(); + SixelProbe.MeasureCell(requests.Add, found.Add); + + requests[1].Abandoned!(); + requests[0].ResponseReceived!("\x1b]1337;ReportCellSize=17.5;8.0;2.0\x1b\\"); + + Assert.Equal([new CellMeasurement(new System.Drawing.SizeF(16, 35), CellSizeSource.Iterm2Report, 2)], found); + } + + [Fact] + public void MeasureCell_assumes_a_size_when_neither_query_is_answered() + { + var requests = new List(); + var found = new List(); + SixelProbe.MeasureCell(requests.Add, found.Add); + + requests[0].Abandoned!(); + requests[1].Abandoned!(); + + Assert.Equal([new CellMeasurement(new System.Drawing.SizeF(10, 20), CellSizeSource.Assumed)], found); + } [Theory] [InlineData(null)] diff --git a/tests/TuiCode.Tests/DiagnosticsViewTests.cs b/tests/TuiCode.Tests/DiagnosticsViewTests.cs index f78b2ec..c0d882f 100644 --- a/tests/TuiCode.Tests/DiagnosticsViewTests.cs +++ b/tests/TuiCode.Tests/DiagnosticsViewTests.cs @@ -1,4 +1,6 @@ +using System.Drawing; using Terminal.Gui.Views; +using TuiCode.Workbench.About; using TuiCode.Workbench.Diagnostics; namespace TuiCode.Tests; @@ -73,6 +75,47 @@ public void ShowTerminalCursorColour_says_when_the_terminal_did_not_answer() Assert.Equal("asked #1F2328 • terminal didn't answer", view.CursorColourText); } + [Theory] + [InlineData(16, 35, "Iterm2Report", 2f, "16 × 35 px • iTerm2 report, scale 2.0")] + [InlineData(8, 17.5f, "Iterm2Report", 1f, "8 × 17.5 px • iTerm2 report, scale 1.0")] + [InlineData(12, 26, "Iterm2Report", 1.5f, "12 × 26 px • iTerm2 report, scale 1.5")] + [InlineData(10, 20, "CellResolutionReply", 1f, "10 × 20 px • CSI 16 t")] + [InlineData(10, 20, "Assumed", 1f, "10 × 20 px • no answer, assumed")] + public void ShowCellSize_says_how_big_a_cell_is_and_where_that_came_from( + float width, float height, string source, float scale, string expected) + { + using var view = new DiagnosticsView("ansi", "No"); + + view.ShowCellSize(new CellMeasurement(new SizeF(width, height), Enum.Parse(source), scale)); + + Assert.Equal(expected, view.CellSizeText); + } + + [Theory] + [InlineData("No")] + [InlineData("Yes (DisambiguateEscapeCodes, ReportEventTypes, ReportAlternateKeys, ReportAllKeysAsEscapeCodes)")] + public void Cell_size_row_sits_on_one_line_under_the_cursor_colour_row(string kittyStatus) + { + using var view = new DiagnosticsView("ansi", kittyStatus, "#1F2328"); + view.ShowTerminalCursorColour("#FFFFFF"); + view.ShowCellSize(new CellMeasurement(new SizeF(16, 35), CellSizeSource.Iterm2Report, 2)); + view.Layout(new Size(200, 100)); + + var labels = view.SubViews.OfType