Show an MXP picture instead of its label once it is drawn

CLAUDE.md

@@ -450,7 +450,12 @@ fallbacks) for inline images/maps.
   Tui; the `mxp-image` view). The parser emits a `[image: name]` span whose `SpanInteraction.Image` carries
   the request; the span keeps an enclosing `<SEND>`'s action (the spec's own `<SEND showmap><IMAGE …>` example)
   and otherwise links to the picture. That span **is** the text-only fallback, so logs, search, triggers and a
-  terminal with no graphics all see the same thing. Decisions not to relitigate:
+  terminal with no graphics all see the same thing. **Once the picture is drawn it takes the label's place on
+  screen** and carries the label's link, so clicking the picture opens it or sends the `<SEND>`
+  (`PlaceInlinePicture`, `Linked`); a line that was only the picture becomes the picture's first row. This
+  happens when the picture *arrives*, never at append time, because a picture that fails keeps its label as
+  the only thing standing for it. `PaneLine.Plain` keeps the label, so ⌃F still finds a picture by name.
+  Decisions not to relitigate:
   - **A picture is pane rows, never a control.** Kitty placeholders (`U+10EEEE` + row/column diacritics, the
     image id in the foreground) and half-blocks are both plain markup a `MarkupControl` already draws, so a
     picture scrolls, clips, freezes and trims with the text around it and nothing that indexes a pane buffer

src/SharpMUTerm.Tui/InlineImageHost.cs

@@ -84,7 +84,8 @@ public static IReadOnlyList<InlineImageRequest> RequestsIn(StyledLine line)
 
     /// <summary>
     /// Loads a line's pictures in order and delivers each one's rows through <paramref name="deliver"/>
-    /// on the UI thread. <paramref name="kitty"/> is asked on the UI thread, at delivery: it is the
+    /// on the UI thread, with its index in <paramref name="requests"/>: a picture that fails delivers
+    /// nothing, so the order alone does not say which one arrived. <paramref name="kitty"/> is asked on the UI thread, at delivery: it is the
     /// driver, and the transmit has to happen just before the rows that point at it are painted.
     /// </summary>
     /// <returns>False when the queue is full and nothing was started.</returns>
@@ -93,7 +94,7 @@ public bool Load(
         InlineImagePresentation presentation,
         int availableColumns,
         Func<IGraphicsProtocol?> kitty,
-        Action<IReadOnlyList<string>> deliver,
+        Action<int, IReadOnlyList<string>> deliver,
         int maxRows = InlineImageLayout.MaxRows)
     {
         ArgumentNullException.ThrowIfNull(requests);
@@ -122,12 +123,13 @@ presentation is not (InlineImagePresentation.Kitty or InlineImagePresentation.Ha
             await started.Task.ConfigureAwait(false);
             try
             {
-                foreach (var request in requests)
+                for (var i = 0; i < requests.Count; i++)
                 {
-                    var rows = await RowsForAsync(request, presentation, availableColumns, maxRows, kitty).ConfigureAwait(false);
+                    var index = i;
+                    var rows = await RowsForAsync(requests[index], presentation, availableColumns, maxRows, kitty).ConfigureAwait(false);
                     if (rows is not null && !_cts.IsCancellationRequested)
                     {
-                        _onUi(() => deliver(rows));
+                        _onUi(() => deliver(index, rows));
                     }
                 }
             }

src/SharpMUTerm.Tui/SharpMUTermApp.cs

@@ -3345,14 +3345,16 @@ private void ConsumeReadAwayBars()
 
     /// <summary>
     /// Appends one of a world's lines to a window and, when it names pictures this terminal can draw,
-    /// starts loading them. Until a picture arrives, and for good on a terminal that cannot draw it, the
-    /// line's own <c>[image: name]</c> link is what stands in for it.
+    /// starts loading them. Until a picture arrives, and for good on a terminal that cannot draw it or a
+    /// picture that fails, the line's own <c>[image: name]</c> link is what stands in for it. A picture
+    /// that arrives takes its label's place, so the reader sees the picture and not its name as well, and
+    /// clicking the picture does what clicking the label did (<see cref="Linked"/>).
     /// <para>
     /// A picture the server left cells for (<see cref="InlineImageRequest.Reserved"/>) is drawn in them:
     /// over its label and the blanks after it on this line, and over the same columns of the lines under
     /// it while those are blank (<see cref="ContinueReservedPictures"/>). That is a figure inside a box, and
     /// drawing it anywhere else would push the box's edges apart. Any other picture is inserted under its
-    /// line (<see cref="InsertImageRows"/>).
+    /// line and its label taken off the line (<see cref="PlaceInlinePicture"/>).
     /// </para>
     /// <para>
     /// Only live output loads pictures. A restored line comes back as its link: fetching every
@@ -3370,21 +3372,22 @@ private void AppendSessionLine(string windowId, StyledLine line, string stamp)
         var slots = new List<(long Picture, int Row, int Columns, int Start)>();
         var arrived = ContinueReservedPictures(windowId, line.Text, slots);
 
-        var reserved = new List<(long Picture, InlineImageRequest Request, int Columns, int Rows)>();
-        var inline = new List<InlineImageRequest>();
+        var reserved = new List<(long Picture, InlineImageRequest Request, SpanInteraction Interaction, int Columns, int Rows)>();
+        var inline = new List<int>();
         if (drawable)
         {
             var at = 0;
-            foreach (var span in line.Spans)
+            for (var index = 0; index < line.Spans.Count; index++)
             {
-                if (span.Interaction?.Image is { } request)
+                var span = line.Spans[index];
+                if (span.Interaction is { Image: { } request } interaction)
                 {
                     if (ReservedCells(line.Text, at, span.Text.Length, request) is { } cells
                         && !Overlaps(slots, at, cells.Columns))
                     {
                         var picture = ++_nextImageAnchor;
                         slots.Add((picture, 0, cells.Columns, at));
-                        reserved.Add((picture, request, cells.Columns, cells.Rows));
+                        reserved.Add((picture, request, interaction, cells.Columns, cells.Rows));
                         if (cells.Rows > 1)
                         {
                             if (!_reservedPictures.TryGetValue(windowId, out var open))
@@ -3397,7 +3400,7 @@ private void AppendSessionLine(string windowId, StyledLine line, string stamp)
                     }
                     else
                     {
-                        inline.Add(request);
+                        inline.Add(index);
                     }
                 }
 
@@ -3431,26 +3434,111 @@ private void AppendSessionLine(string windowId, StyledLine line, string stamp)
         IGraphicsProtocol? Kitty() => _system.ConsoleDriver as IGraphicsProtocol;
         if (anchor != 0)
         {
+            var shown = new InlinePictureLine(line, inline);
             _inlineImages.Load(
-                inline,
+                inline.Select(index => line.Spans[index].Interaction!.Image!).ToArray(),
                 presentation,
                 InlineImageColumns(windowId),
                 Kitty,
-                rows => InsertImageRows(windowId, anchor, rows));
+                (index, rows) => PlaceInlinePicture(windowId, anchor, shown, index, rows));
         }
 
-        foreach (var (picture, request, columns, rows) in reserved)
+        foreach (var (picture, request, interaction, columns, rows) in reserved)
         {
             _inlineImages.Load(
                 [request],
                 presentation,
                 columns,
                 Kitty,
-                drawn => DrawReservedPicture(windowId, picture, drawn),
+                (_, drawn) => DrawReservedPicture(windowId, picture, Linked(drawn, interaction)),
                 rows);
         }
     }
 
+    /// <summary>
+    /// A picture's rows as the link its label was, so that clicking a picture does what clicking its
+    /// <c>[image: name]</c> did: opens it, or runs the <c>&lt;SEND&gt;</c> it was inside. The link is
+    /// structural markup and takes no cell, so the rows are as wide as they were.
+    /// </summary>
+    private static IReadOnlyList<string> Linked(IReadOnlyList<string> rows, SpanInteraction? interaction) =>
+        LinkPayload.For(interaction) is { } link ? rows.Select(row => $"[link={link}]{row}[/]").ToArray() : rows;
+
+    /// <summary>
+    /// Puts an inline picture under its line and takes its label off the line, so the picture is shown
+    /// and its name is not. A line that was nothing but this picture is replaced by it outright, rather
+    /// than left behind as an empty row above it.
+    /// <para>
+    /// Done when the picture arrives and not before, because one that never arrives — a failed fetch, a
+    /// full queue, a client closing — leaves its label as the only thing standing for it.
+    /// </para>
+    /// </summary>
+    private void PlaceInlinePicture(string windowId, long anchor, InlinePictureLine shown, int index, IReadOnlyList<string> rows)
+    {
+        if (rows.Count == 0 || !_lines.TryGetValue(windowId, out var buffer))
+        {
+            return;
+        }
+
+        var at = buffer.FindIndex(l => l.ImageAnchor == anchor);
+        if (at < 0)
+        {
+            return;
+        }
+
+        var span = shown.Labels[index];
+        rows = Linked(rows, shown.Line.Spans[span].Interaction);
+
+        // A line that also has cells a server left for a picture is held in pieces around them
+        // (PaneLine.Pictures), and rebuilding it from its spans would undo what was drawn there. It keeps
+        // its labels; the picture still goes under it.
+        if (buffer[at].Pictures is not null)
+        {
+            InsertImageRows(windowId, anchor, rows);
+            return;
+        }
+
+        var first = shown.Drawn.Count == 0;
+        shown.Drawn.Add(span);
+        var rest = new StyledLine(
+            shown.Line.Spans.Where((_, i) => !shown.Drawn.Contains(i)).ToArray(),
+            shown.Line.RuleColor,
+            shown.Line.IsPrompt);
+
+        // Plain keeps the text the server sent, as a reserved picture's line does, so ⌃F still finds a
+        // picture by its name.
+        if (first && string.IsNullOrWhiteSpace(rest.Text))
+        {
+            buffer[at] = buffer[at] with { Markup = rows[0] };
+            InsertImageRows(windowId, anchor, rows.Skip(1).ToArray(), repaint: true);
+        }
+        else if (string.IsNullOrWhiteSpace(rest.Text) && at + 1 < buffer.Count && buffer[at + 1].ImageAnchor == -anchor)
+        {
+            // The last label of a line holding only pictures, with the earlier ones already under it. This
+            // picture cannot take the line, or it would sit above them; the first row under the line moves
+            // up into it instead, so no blank row is left above the pictures.
+            buffer[at] = buffer[at] with { Markup = buffer[at + 1].Markup };
+            RemoveChromeRow(windowId, at + 1);
+            InsertImageRows(windowId, anchor, rows, repaint: true);
+        }
+        else
+        {
+            buffer[at] = buffer[at] with { Markup = _formatter.ToMarkup(rest) };
+            InsertImageRows(windowId, anchor, rows, repaint: true);
+        }
+    }
+
+    /// <summary>A line whose inline pictures are on their way: which of its spans are their labels, and which have arrived.</summary>
+    private sealed class InlinePictureLine(StyledLine line, IReadOnlyList<int> labels)
+    {
+        public StyledLine Line { get; } = line;
+
+        /// <summary>The span holding each picture's label, in the order the pictures were asked for.</summary>
+        public IReadOnlyList<int> Labels { get; } = labels;
+
+        /// <summary>The labels whose pictures have arrived.</summary>
+        public HashSet<int> Drawn { get; } = new();
+    }
+
     /// <summary>
     /// The cells a reserved picture labelled at <paramref name="start"/> may be drawn in, or null when the
     /// line does not have them: the label and blanks to the picture's width. Bounded by the Kitty diacritic
@@ -3616,10 +3704,20 @@ private int InlineImageColumns(string windowId)
     /// below them rather than splitting a picture from its line.
     /// </para>
     /// </summary>
-    private void InsertImageRows(string windowId, long anchor, IReadOnlyList<string> rows)
+    private void InsertImageRows(string windowId, long anchor, IReadOnlyList<string> rows, bool repaint = false)
     {
-        if (rows.Count == 0 || !_lines.TryGetValue(windowId, out var buffer))
+        if (!_lines.TryGetValue(windowId, out var buffer))
+        {
+            return;
+        }
+
+        if (rows.Count == 0)
         {
+            if (repaint)
+            {
+                RepaintPane(windowId);
+            }
+
             return;
         }
 
@@ -3674,8 +3772,9 @@ private void InsertImageRows(string windowId, long anchor, IReadOnlyList<string>
         }
 
         // The common case is a picture under the newest line of a live pane: append, as a line would.
-        // Anywhere else the rows went mid-buffer and the pane is re-fed, once per picture.
-        if (at == buffer.Count - n && !_freezePoints.ContainsKey(windowId) &&
+        // Anywhere else the rows went mid-buffer, or the caller changed a line already painted, and the
+        // pane is re-fed, once per picture.
+        if (!repaint && at == buffer.Count - n && !_freezePoints.ContainsKey(windowId) &&
             _panes.TryGetValue(windowId, out var control))
         {
             foreach (var row in rows)

tests/SharpMUTerm.Tui.Tests/MxpImageTests.cs

@@ -129,10 +129,89 @@ public async Task OnAHalfBlockTerminalThePictureLandsUnderItsLine()
         run.App.RenderNextFrame();
 
         var lines = run.App.PaneLines(MainWindow).ToList();
-        var at = lines.FindIndex(l => l.Contains("[image: map.png]", StringComparison.Ordinal));
+        var at = lines.FindIndex(l => l.Contains("A map:", StringComparison.Ordinal));
         await Assert.That(at).IsGreaterThanOrEqualTo(0);
         var picture = lines.Skip(at + 1).TakeWhile(l => l.Contains('▀')).Count();
         await Assert.That(picture).IsEqualTo(4); // 64×64 px is 8×4 cells
+        await Assert.That(lines.Any(l => l.Contains("[image:", StringComparison.Ordinal))).IsFalse()
+            .Because("a picture that is drawn takes its label's place");
+    }
+
+    /// <summary>
+    /// A line that was nothing but its picture is replaced by the picture, rather than left as an empty row
+    /// above it, and the picture is the link the label was.
+    /// </summary>
+    [Test]
+    public async Task APictureOnALineOfItsOwnIsTheLineAndItsLink()
+    {
+        await using var run = await Start(GraphicsProtocol.HalfBlock);
+        run.App.ImageFetch = (_, _) => Task.FromResult<byte[]?>(Png(64, 64));
+        run.Receive("Before.");
+
+        run.Receive(Secure + "<IMAGE map.png URL=\"https://mud.example/\">");
+        run.Receive("After.");
+        await run.App.InlineImagesSettled;
+        run.App.RenderNextFrame();
+
+        var lines = run.App.PaneLines(MainWindow).ToList();
+        var at = lines.FindIndex(l => l.Contains("Before.", StringComparison.Ordinal));
+        var picture = lines.Skip(at + 1).Take(4).ToList();
+        foreach (var row in picture)
+        {
+            await Assert.That(row).Contains('▀');
+            await Assert.That(row).Contains(LinkPayload.For(SpanInteraction.Link("https://mud.example/map.png"))!);
+        }
+
+        await Assert.That(lines[at + 5]).Contains("After.");
+        await Assert.That(lines.Any(l => l.Contains("[image:", StringComparison.Ordinal))).IsFalse();
+    }
+
+    /// <summary>
+    /// A line of nothing but two pictures leaves no blank row above them once both are drawn, and they keep
+    /// the order the line named them in.
+    /// </summary>
+    [Test]
+    public async Task TwoPicturesOnALineOfTheirOwnLeaveNoBlankRow()
+    {
+        await using var run = await Start(GraphicsProtocol.HalfBlock);
+        run.App.ImageFetch = (url, _) => Task.FromResult<byte[]?>(url.EndsWith("a.png", StringComparison.Ordinal) ? Png(32, 32) : Png(32, 64));
+        run.Receive("Before.");
+
+        run.Receive(Secure + "<IMAGE a.png URL=\"https://mud.example/\"> <IMAGE b.png URL=\"https://mud.example/\">");
+        run.Receive("After.");
+        await run.App.InlineImagesSettled;
+        run.App.RenderNextFrame();
+
+        var lines = run.App.PaneLines(MainWindow).ToList();
+        var at = lines.FindIndex(l => l.Contains("Before.", StringComparison.Ordinal));
+        var a = LinkPayload.For(SpanInteraction.Link("https://mud.example/a.png"))!;
+        var b = LinkPayload.For(SpanInteraction.Link("https://mud.example/b.png"))!;
+        var expected = new[] { a, a, b, b, b, b }; // 32×32 px is two rows, 32×64 four
+        for (var i = 0; i < expected.Length; i++)
+        {
+            await Assert.That(lines[at + 1 + i]).Contains('▀').And.Contains(expected[i]);
+        }
+
+        await Assert.That(lines[at + 1 + expected.Length]).Contains("After.");
+    }
+
+    /// <summary>A picture inside a <c>&lt;SEND&gt;</c> sends that command when clicked, as its label did.</summary>
+    [Test]
+    public async Task APictureInsideASendIsThatCommand()
+    {
+        await using var run = await Start(GraphicsProtocol.HalfBlock);
+        run.App.ImageFetch = (_, _) => Task.FromResult<byte[]?>(Png(32, 32));
+
+        run.Receive(Secure + "<SEND \"look map\"><IMAGE map.png URL=\"https://mud.example/\"></SEND>");
+        await run.App.InlineImagesSettled;
+        run.App.RenderNextFrame();
+
+        var rows = run.App.PaneLines(MainWindow).Where(l => l.Contains('▀')).ToList();
+        await Assert.That(rows.Count).IsEqualTo(2);
+        foreach (var row in rows)
+        {
+            await Assert.That(row).Contains(LinkPayload.For(SpanInteraction.Command("look map", null, false))!);
+        }
     }
 
     /// <summary>
@@ -154,10 +233,9 @@ public async Task APictureThatArrivesLateStillGoesUnderItsOwnLine()
         run.App.RenderNextFrame();
 
         var lines = run.App.PaneLines(MainWindow).ToList();
-        var at = lines.FindIndex(l => l.Contains("[image: map.png]", StringComparison.Ordinal));
+        var at = lines.FindIndex(l => l.Contains('▀'));
         await Assert.That(lines[at + 1]).Contains("▀");
-        await Assert.That(lines[at + 2]).Contains("▀");
-        await Assert.That(lines[at + 3]).Contains("A town guard stands watch.");
+        await Assert.That(lines[at + 2]).Contains("A town guard stands watch.");
         await Assert.That(lines[^1]).Contains("The fountain burbles.");
     }
 
@@ -251,6 +329,7 @@ public async Task APictureTheServerLeftCellsForIsDrawnInThemAndAddsNoRows()
             await Assert.That(plain.Length).IsEqualTo(12);
             await Assert.That(plain).StartsWith("| ").And.EndsWith(" |");
             await Assert.That(plain.Count(c => c == '▀')).IsEqualTo(8);
+            await Assert.That(row).Contains(LinkPayload.WebScheme).Because("clicking the picture opens it, as its label did");
         }
     }