Answer a server's MXP SUPPORT and VERSION again

CHANGELOG.md

@@ -112,6 +112,9 @@ and a release's notes are its section. The procedure is in [`docs/RELEASING.md`]
 
 ### Fixed
 
+- MXP pictures draw again. Since the TelnetNegotiationCore update the client stopped answering a
+  server's MXP `<SUPPORT>` and `<VERSION>` questions, so a game such as SharpMUSH decided it could not show
+  `<IMAGE>` and sent the picture's address as text instead.
 - Pueblo worlds work. The client answers a server's Pueblo greeting and switches to Pueblo markup when the
   server does, and Pueblo worlds now show their ANSI colours instead of the escape codes. A world set to
   `mxp` or `ansi` on F5 does not answer the greeting.

src/SharpMUTerm.Core/Protocols/MxpParser.cs

@@ -150,7 +150,16 @@ private sealed record Frame
     public event EventHandler<string>? ClientReply;
 
     /// <summary>What this client answers a VERSION request with.</summary>
-    private const string ClientName = "SharpMUTerm";
+    public const string ClientName = "SharpMUTerm";
+
+    /// <summary>The MXP revision a VERSION reply names (see <see cref="HandleVersionRequest"/>).</summary>
+    public const string MxpRevision = "1.0";
+
+    /// <summary>The style-sheet version a VERSION reply names: none is ever in effect.</summary>
+    public const string StyleVersion = "0";
+
+    /// <summary>The client version a VERSION reply names: a fixed placeholder.</summary>
+    public const string ClientVersion = "1";
 
     /// <summary>
     /// Spec: "the response is sent as a secure-tagged line: <c>&lt;ESC&gt;[1z</c> prefix with newline
@@ -163,7 +172,9 @@ private sealed record Frame
     private const string SecureLinePrefix = "\x1b[1z";
 
     /// <summary>
-    /// The tags this parser genuinely implements, in the spec's <c>+tag</c> form.
+    /// The tags this parser genuinely implements, by name. The telnet layer answers a server's
+    /// <c>&lt;SUPPORT&gt;</c> from this list too (<see cref="AnswerSupport"/>), because the library takes
+    /// that line out of the stream before this parser can see it.
     /// </summary>
     /// <remarks>
     /// An honest list, and deliberately not an aspirational one: a SUPPORTS answer is a claim a
@@ -173,9 +184,85 @@ private sealed record Frame
     /// <see cref="HandleOpener"/> has no case for it — it falls to the default and is silently
     /// dropped — so it is deliberately not claimed here.
     /// </remarks>
-    private static readonly string[] SupportedTags =
-        ["+b", "+i", "+u", "+s", "+color", "+font", "+send", "+a", "+br", "+image",
-         "+!element", "+!entity", "+var", "+gauge", "+stat", "+sound", "+music", "+frame", "+dest"];
+    public static IReadOnlyList<string> SupportedTags { get; } =
+        ["b", "i", "u", "s", "color", "font", "send", "a", "br", "image",
+         "!element", "!entity", "var", "gauge", "stat", "sound", "music", "frame", "dest"];
+
+    /// <summary>
+    /// The attributes a handler here reads, by tag, for a <c>&lt;SUPPORT tag.attr&gt;</c> or
+    /// <c>&lt;SUPPORT tag.*&gt;</c>. The same honesty rule as <see cref="SupportedTags"/>: an attribute
+    /// is listed only when its handler acts on it, so <c>image.ismap</c>, read and ignored, is not.
+    /// A supported tag missing here has no attribute this client claims.
+    /// </summary>
+    public static IReadOnlyDictionary<string, string[]> SupportedAttributes { get; } =
+        new Dictionary<string, string[]>(StringComparer.OrdinalIgnoreCase)
+        {
+            ["color"] = ["fore", "back"],
+            ["send"] = ["href", "hint", "prompt"],
+            ["a"] = ["href", "hint"],
+            ["image"] = ["fname", "url", "t", "w", "h"],
+            ["gauge"] = ["entity", "max", "caption"],
+            ["stat"] = ["entity", "max", "caption"],
+            ["sound"] = ["fname", "v", "l", "p", "t", "u"],
+            ["music"] = ["fname", "v", "l", "c", "t", "u"],
+            ["frame"] = ["name", "action", "title"],
+            ["dest"] = ["name"],
+        };
+
+    /// <summary>
+    /// The answer to a <c>&lt;SUPPORT&gt;</c> that asked about <paramref name="asked"/>: every tag in
+    /// <see cref="SupportedTags"/> when it asked about nothing, otherwise each entry it named, as one this
+    /// client renders or one it does not. <c>tag.attr</c> is supported when <see cref="SupportedAttributes"/>
+    /// lists it, and <c>tag.*</c> is answered with each attribute listed for the tag, as the specification
+    /// asks; a tag this client does not render is answered <c>-tag.*</c>.
+    /// </summary>
+    public static (IReadOnlyList<string> Supported, IReadOnlyList<string> Unsupported) AnswerSupport(IReadOnlyList<string> asked)
+    {
+        if (asked.Count == 0)
+        {
+            return (SupportedTags, []);
+        }
+
+        var supported = new List<string>();
+        var unsupported = new List<string>();
+        foreach (var entry in asked)
+        {
+            var name = entry.Trim('"', '\'');
+            if (name.Length == 0)
+            {
+                continue;
+            }
+
+            var dot = name.IndexOf('.', StringComparison.Ordinal);
+            if (dot < 0)
+            {
+                (SupportedTags.Contains(name, StringComparer.OrdinalIgnoreCase) ? supported : unsupported).Add(name);
+                continue;
+            }
+
+            var tag = name[..dot];
+            var attribute = name[(dot + 1)..];
+            var known = SupportedTags.Contains(tag, StringComparer.OrdinalIgnoreCase);
+            var attributes = known ? SupportedAttributes.GetValueOrDefault(tag) ?? [] : [];
+            if (attribute == "*")
+            {
+                if (known)
+                {
+                    supported.AddRange(attributes.Select(a => $"{tag}.{a}"));
+                }
+                else
+                {
+                    unsupported.Add(name);
+                }
+            }
+            else
+            {
+                (attributes.Contains(attribute, StringComparer.OrdinalIgnoreCase) ? supported : unsupported).Add(name);
+            }
+        }
+
+        return (supported, unsupported);
+    }
 
     /// <summary>True when a partial line, an open tag/entity, or unclosed markup is buffered.</summary>
     public bool HasPendingContent =>
@@ -1019,12 +1106,12 @@ private void HandleOpener(string name, string attrs)
     /// </remarks>
     private void HandleVersionRequest()
     {
-        ClientReply?.Invoke(this, $"{SecureLinePrefix}<VERSION MXP=1.0 STYLE=0 CLIENT={ClientName} VERSION=1>");
+        ClientReply?.Invoke(this, $"{SecureLinePrefix}<VERSION MXP={MxpRevision} STYLE={StyleVersion} CLIENT={ClientName} VERSION={ClientVersion}>");
     }
 
     private void HandleSupportRequest()
     {
-        ClientReply?.Invoke(this, $"{SecureLinePrefix}<SUPPORTS {string.Join(' ', SupportedTags)}>");
+        ClientReply?.Invoke(this, $"{SecureLinePrefix}<SUPPORTS {string.Join(' ', SupportedTags.Select(t => "+" + t))}>");
     }
 
     /// <summary>

src/SharpMUTerm.Core/Telnet/TelnetSession.cs

@@ -3,6 +3,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using SharpMUTerm.Core.Telnet.Mssp;
+using SharpMUTerm.Core.Protocols;
 using SharpMUTerm.Core.Transport;
 using TelnetNegotiationCore.Builders;
 using TelnetNegotiationCore.Gmcp;
@@ -473,6 +474,7 @@ private Task<TelnetInterpreter> BuildInterpreterAsync()
             .AddPlugin<SuppressGoAheadProtocol>().OnPrompt(OnPromptAsync)
             .AddPlugin<PacketPatchProtocol>().OnPrompt(OnPromptAsync)
             .AddPlugin<MXPProtocol>().OnMXPEnabled(OnMxpEnabledAsync)
+                .OnMxpSupportRequested(OnMxpSupportRequestedAsync).OnMxpVersionRequested(OnMxpVersionRequestedAsync)
             .AddPlugin<PuebloProtocol>().OnPuebloOffered(OnPuebloOfferedAsync).OnPuebloEnabled(OnPuebloEnabledAsync)
             .AddPlugin<MCCPProtocol>().OnCompressionEnabled(OnCompressionAsync)
             // MCP is in-band and the server opens it (#$#mcp version), so registering it sends nothing to a
@@ -754,6 +756,52 @@ private ValueTask OnPuebloEnabledAsync(PuebloClient client)
 
     private volatile bool _mxpOn;
 
+    private MXPProtocol? Mxp => _interpreter?.PluginManager?.GetPlugin<MXPProtocol>();
+
+    /// <summary>
+    /// A server's <c>&lt;SUPPORT&gt;</c>. Once MXP mode has started the library takes the line out of the
+    /// stream and sends nothing unless asked to, so <see cref="MxpParser"/> never sees it. Left unanswered, a
+    /// server that waits for the reply (SharpMUSH does) records that this client renders none of the tags it
+    /// asked about, and sends a picture's address as text instead of an <c>&lt;IMAGE&gt;</c>.
+    /// </summary>
+    private async ValueTask OnMxpSupportRequestedAsync(IReadOnlyList<string> asked)
+    {
+        if (Mxp is not { } mxp)
+        {
+            return;
+        }
+
+        var (supported, unsupported) = MxpParser.AnswerSupport(asked);
+        try
+        {
+            await mxp.SendSupportsAsync(supported, unsupported).ConfigureAwait(false);
+        }
+        catch (Exception ex) when (ex is IOException or InvalidOperationException or ObjectDisposedException)
+        {
+            _logger.LogWarning(ex, "Could not answer the server's MXP SUPPORT request.");
+        }
+    }
+
+    /// <summary>A server's <c>&lt;VERSION&gt;</c>, taken out of the stream by the library like <c>&lt;SUPPORT&gt;</c>.</summary>
+    private async ValueTask OnMxpVersionRequestedAsync()
+    {
+        if (Mxp is not { } mxp)
+        {
+            return;
+        }
+
+        try
+        {
+            await mxp.SendVersionAsync(new MxpVersion(
+                MxpParser.MxpRevision, MxpParser.StyleVersion, MxpParser.ClientName, MxpParser.ClientVersion, Registered: null))
+                .ConfigureAwait(false);
+        }
+        catch (Exception ex) when (ex is IOException or InvalidOperationException or ObjectDisposedException)
+        {
+            _logger.LogWarning(ex, "Could not answer the server's MXP VERSION request.");
+        }
+    }
+
     // ----- MCP ----------------------------------------------------------------------
 
     private MudClientProtocol? Mcp => _interpreter?.PluginManager?.GetPlugin<MudClientProtocol>();

src/SharpMUTerm.Tui/ComposeOverlay.cs

@@ -83,8 +83,12 @@ internal ComposeOverlay(ConsoleWindowSystem system, Func<SpellChecker?>? spellin
 
     internal bool IsOpen => _window is not null;
 
-    /// <summary>The buffer as it stands, or empty when the window is shut. Internal for the tests.</summary>
-    internal string Body => _editor?.GetContent() ?? string.Empty;
+    /// <summary>
+    /// The buffer as it stands, or empty when the window is shut, with its lines broken by <c>\n</c>. The editor
+    /// joins them with <see cref="Environment.NewLine"/>, so on Windows a post or an edit would otherwise carry
+    /// CRLF in one host and LF in the other.
+    /// </summary>
+    internal string Body => _editor?.GetContent().Replace("\r\n", "\n", StringComparison.Ordinal) ?? string.Empty;
 
     /// <summary>The escaping mode as it stands — the window's own toggle, not a saved setting.</summary>
     internal ComposeEscaping Escaping => _escaping;

src/SharpMUTerm.Tui/Hosting/ClientHost.cs

@@ -156,14 +156,7 @@ public static ClientHost ForHarness(
     {
         if (driver is null)
         {
-            return Terminal(capabilities) with
-            {
-                Clipboard = clipboard,
-                OpenUrl = openUrl,
-                FocusReporting = focusReporting ?? PlatformDecodesFocusReports,
-                Audio = null, // a test never makes a noise
-                Presence = null, // nor talks to Discord
-            };
+            return HarnessTerminal(capabilities, new NetConsoleDriver(RenderMode.Buffer), focusReporting, openUrl, clipboard);
         }
 
         var inline = driver is HeadlessConsoleDriver;
@@ -176,4 +169,24 @@ public static ClientHost ForHarness(
             RunsInline = inline,
         };
     }
+
+    /// <summary>
+    /// <see cref="ForHarness"/> with no driver of its own: the terminal host on <paramref name="driver"/>, with
+    /// what a test overrides. Apart so a test can read it on a driver that needs no console, because the
+    /// framework's own driver throws on Windows when stdin and stdout are redirected, as they are in CI.
+    /// </summary>
+    internal static ClientHost HarnessTerminal(
+        TerminalCapabilities capabilities,
+        IConsoleDriver driver,
+        bool? focusReporting,
+        Action<string>? openUrl,
+        Action<string>? clipboard) =>
+        Terminal(capabilities, driver) with
+        {
+            Clipboard = clipboard,
+            OpenUrl = openUrl,
+            FocusReporting = focusReporting ?? PlatformDecodesFocusReports,
+            Audio = null, // a test never makes a noise
+            Presence = null, // nor talks to Discord
+        };
 }

tests/SharpMUTerm.Avalonia.Tests/HostParityTests.cs

@@ -13,7 +13,7 @@ namespace SharpMUTerm.Avalonia.Tests;
 /// </summary>
 /// <remarks>Serialised: a headless frame is captured by redirecting the process-global <c>Console.Out</c>.</remarks>
 [NotInParallel]
-public class HostParityTests
+public partial class HostParityTests
 {
     private const int Width = 140;
     private const int Height = 40;
@@ -104,7 +104,7 @@ private static List<string> Differences(string ansi, ScreenFrame frame, int rows
         {
             var expected = FrameGrid.Decode(ansi, Width, Height)[y];
             var actual = frame.RowText(y);
-            if (!string.Equals(expected, actual, StringComparison.Ordinal))
+            if (!string.Equals(expected, actual, StringComparison.Ordinal) && !DifferOnlyInStamp(expected, actual))
             {
                 differences.Add($"row {y}:\n  terminal |{expected}|\n  window   |{actual}|");
                 continue;
@@ -124,6 +124,33 @@ private static List<string> Differences(string ansi, ScreenFrame frame, int rows
         return differences;
     }
 
+    /// <summary>
+    /// True when two rows differ only in the time a message was stamped with: the first <c>hh:mm:ss</c> in
+    /// each, at the same column, with every other cell equal. The client's messages are stamped by the
+    /// logger's wall clock, which no test pins, and the two hosts draw a second apart often enough to fail on
+    /// the second changing. A clock in a message's own text comes after the stamp and is still compared.
+    /// </summary>
+    internal static bool DifferOnlyInStamp(string expected, string actual)
+    {
+        var a = ClockTime().Match(expected);
+        var b = ClockTime().Match(actual);
+        return a.Success && b.Success && a.Index == b.Index && expected.Length == actual.Length
+            && string.Equals(expected[..a.Index], actual[..a.Index], StringComparison.Ordinal)
+            && string.Equals(expected[(a.Index + a.Length)..], actual[(b.Index + b.Length)..], StringComparison.Ordinal);
+    }
+
+    [System.Text.RegularExpressions.GeneratedRegex(@"\b\d{2}:\d{2}:\d{2}\b")]
+    private static partial System.Text.RegularExpressions.Regex ClockTime();
+
+    /// <summary>Only the stamp is allowed to differ: a clock in a message's text is compared like any other text.</summary>
+    [Test]
+    [Arguments("│ 10:00:00 info  hello", "│ 10:00:01 info  hello", true)]
+    [Arguments("│ 10:00:00 info  at 12:34:56", "│ 10:00:00 info  at 12:35:56", false)]
+    [Arguments("│ 10:00:00 info  hello", "│ 10:00:01 info  hullo", false)]
+    [Arguments("│ 10:00:00 info  hello", "│  10:00:01 info hello", false)]
+    public async Task OnlyTheStampMayDifferBetweenHosts(string terminal, string window, bool same) =>
+        await Assert.That(DifferOnlyInStamp(terminal, window)).IsEqualTo(same);
+
     public static IEnumerable<string> Hosts() => ["terminal", "window"];
 
     private static (HostKeys Keys, UsageLauncher Launcher) HostNamed(string name) => name == "window"