Size a figure's MXP picture in the cells laid out for it (#52)

A figure's <IMAGE> carried no size, so a client drew the picture at its own
size under the line, outside any box round the figure. The tag now says how
many cells the picture covers (W=12c H=5c), the art's or the PictureCells a
host gave it, so a client can draw it there.

A row that is only the picture's description is marked
PictureRow.IsDescription and gets no cell size: a picture squeezed into one
line of its description would be a strip.


Claude-Session: https://claude.ai/code/session_018ztDNVgzxM8V5WKWBRs2EW

Co-authored-by: Claude <noreply@anthropic.com>

Author: HarryCordewener

CHANGELOG.md

@@ -52,6 +52,11 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixed
 
+- **An MXP client draws a figure's picture in its cells.** A figure's `<IMAGE>` carried no size, so a
+  client drew the picture at its own size under the line it was named on, outside any box round it. It
+  is now sized in the cells laid out for it (`W=12c H=5c`), the art's or `PictureCells`', and a client
+  that honours cell sizes draws it there. A figure with neither, whose row is only its description
+  (`PictureRow.IsDescription`), still leaves the picture its own size.
 - **A figure whose art is the picture's own placeholder is one picture.** Art marked as the picture
   inline (a Markdown image laid out as a figure) carried both marks, so MXP, Pueblo and BBCode wrote the
   picture twice or dropped its cells, and the text beside it moved. A figure's rows now carry its mark

MarkupString.Mxp/Emitters/ElementMxpEmitter.cs

@@ -63,10 +63,15 @@ public void Emit(IMarkup markup, ReadOnlySpan<char> body, in EmitContext context
 					if (image.StartsPicture(context))
 					{
 						var (file, directory) = Split(image.Source);
+						// A figure's picture is sized in the cells laid out for it, so a client draws it over
+						// them rather than at its own size beside or under them.
+						var (width, height) = image.Row is { IsDescription: false } row
+							? ($"{row.Columns}c", $"{row.Rows}c")
+							: (Pixels(image.Width), Pixels(image.Height));
 						tag.Open("IMAGE").Positional(file)
 							.Named("URL", directory)
-							.Named("W", image.Width)
-							.Named("H", image.Height)
+							.Named("W", width)
+							.Named("H", height)
 							.Named("ALIGN", image.Align?.ToString().ToUpperInvariant())
 							.Close();
 					}
@@ -161,6 +166,8 @@ public void Emit(IMarkup markup, ReadOnlySpan<char> body, in EmitContext context
 
 	private bool Supports(string element) => supports?.Invoke(element) ?? true;
 
+	private static string? Pixels(int? pixels) => pixels?.ToString(CultureInfo.InvariantCulture);
+
 	/// <summary>
 	/// MXP names a file and, separately, the address of the directory to fetch it from when the client
 	/// does not have it. An absolute address is split at its last slash; anything else is a file name.

MarkupString.Tests/Layout/FigurePictureTests.cs

@@ -36,7 +36,8 @@ public async Task MxpWritesTheFiguresPictureOnceAndKeepsItsCells()
 
 		var lines = Lines(laid, MarkupFormat.Mxp);
 
-		await Assert.That(lines[0].TrimEnd()).IsEqualTo("<IMAGE cat.png URL=https://example.test/img/>");
+		// Sized in the cells the art takes, so a client draws it over them.
+		await Assert.That(lines[0].TrimEnd()).IsEqualTo("<IMAGE cat.png URL=https://example.test/img/ W=7c H=3c>");
 		await Assert.That(Count(string.Join('\n', lines), "<IMAGE")).IsEqualTo(1);
 		await Assert.That(lines.Skip(1).All(line => line.Trim().Length == 0)).IsTrue();
 		await Assert.That(lines.Length).IsEqualTo(3);
@@ -53,6 +54,21 @@ public async Task AFigureWithNoArtIsItsPictureNotItsDescription()
 		await Assert.That(laid.ToPlainText().TrimEnd()).IsEqualTo("[A cat]");
 	}
 
+	/// <summary>
+	/// A picture given cells of its own and no art is sized in them; its description alone is not cells laid
+	/// out for it, so a figure that has nothing else leaves the picture its own size.
+	/// </summary>
+	[Test]
+	public async Task OnlyCellsLaidOutForThePictureSizeIt()
+	{
+		var context = new LayoutContext { Pictures = (_, _) => new PictureCells(12, 5) };
+		var sized = BlockLayout.Build(new Figure(Cat, MarkupText.Empty), 20, context: context);
+		var described = BlockLayout.Build(new Figure(Cat, MarkupText.Empty), 20);
+
+		await Assert.That(Lines(sized, MarkupFormat.Mxp)[0].TrimEnd()).IsEqualTo("<IMAGE cat.png URL=https://example.test/img/ W=12c H=5c>");
+		await Assert.That(Lines(described, MarkupFormat.Mxp)[0].TrimEnd()).IsEqualTo("<IMAGE cat.png URL=https://example.test/img/>");
+	}
+
 	[Test]
 	public async Task ArtOfLineBreaksAloneIsNoArt()
 	{
@@ -80,7 +96,7 @@ public async Task ArtMarkedAsThePictureIsStillOnePicture(string format)
 		var markupFormat = format switch { "mxp" => MarkupFormat.Mxp, "pueblo" => MarkupFormat.Pueblo, _ => MarkupFormat.BBCode };
 		var tag = format switch
 		{
-			"mxp" => "<IMAGE cat.png URL=https://example.test/img/>",
+			"mxp" => "<IMAGE cat.png URL=https://example.test/img/ W=14c H=1c>",
 			"pueblo" => "<img src=\"https://example.test/img/cat.png\" alt=\"A cat\">",
 			_ => "[img]https://example.test/img/cat.png[/img]",
 		};
@@ -102,8 +118,9 @@ public async Task TextBesideAFloatedPictureStaysWhereItWas()
 		var plain = laid.ToPlainText().Split('\n');
 		var mxp = Lines(laid, MarkupFormat.Mxp);
 
-		await Assert.That(mxp[0]).StartsWith("<IMAGE cat.png URL=https://example.test/img/>");
-		var untagged = mxp.Select(line => line.Replace("<IMAGE cat.png URL=https://example.test/img/>", string.Empty)).ToArray();
+		const string tag = "<IMAGE cat.png URL=https://example.test/img/ W=7c H=3c>";
+		await Assert.That(mxp[0]).StartsWith(tag);
+		var untagged = mxp.Select(line => line.Replace(tag, string.Empty)).ToArray();
 		await Assert.That(untagged.Length).IsEqualTo(plain.Length);
 		// The art is seven cells and the gap two: the picture's cells are blank, and the words start where they did.
 		for (var row = 0; row < plain.Length; row++)
@@ -174,6 +191,18 @@ public async Task HtmlHeldToAPolicyThatRefusesThePictureIsTheArt()
 		await Assert.That(second.Render(MarkupFormat.Html, registry)).IsEqualTo("( o.o )");
 	}
 
+	[Test]
+	public async Task ADescriptionRowSurvivesSerialisation()
+	{
+		var laid = BlockLayout.Build(new Figure(Cat, MarkupText.Empty), 20);
+
+		var back = MarkupTextSerializer.Deserialize(MarkupTextSerializer.Serialize(laid, Registry), Registry);
+
+		await Assert.That(back.Runs.SelectMany(run => run.Markups).OfType<ImageMarkup>().All(image => image.Row is { IsDescription: true }))
+			.IsTrue();
+		await Assert.That(back.Render(MarkupFormat.Mxp, Registry)).IsEqualTo(laid.Render(MarkupFormat.Mxp, Registry));
+	}
+
 	[Test]
 	public async Task ARowOfAPictureSurvivesSerialisation()
 	{

MarkupString/Elements/ElementCodecs.cs

@@ -49,6 +49,7 @@ internal static class ElementCodecs
 					w.WriteNumber("r", row.Row);
 					w.WriteNumber("rs", row.Rows);
 					w.WriteNumber("c", row.Columns);
+					if (row.IsDescription) w.WriteBoolean("dsc", true);
 				}
 			},
 			static e => new ImageMarkup(
@@ -59,7 +60,7 @@ internal static class ElementCodecs
 				Enum.TryParse<ImageAlign>(String(e, "a"), ignoreCase: true, out var align) ? align : null)
 			{
 				Row = Int(e, "r") is { } row && Int(e, "rs") is { } rows && Int(e, "c") is { } columns
-					? new PictureRow(row, rows, columns)
+					? new PictureRow(row, rows, columns) { IsDescription = e.TryGetProperty("dsc", out var described) && described.ValueKind == JsonValueKind.True }
 					: null,
 			}),
 		new Codec<PaneMarkup>("pane",

MarkupString/Elements/ImageMarkup.cs

@@ -26,7 +26,8 @@ public enum ImageAlign
 /// </summary>
 /// <remarks>
 /// <para>MXP writes <c>&lt;IMAGE&gt;</c>, Pueblo and HTML <c>&lt;img&gt;</c>, and BBCode <c>[img]</c>, once
-/// for the picture (<see cref="StartsPicture"/>); over a figure's rows they keep the rest of its cells blank.
+/// for the picture (<see cref="StartsPicture"/>); over a figure's rows they keep the rest of its cells blank,
+/// and MXP sizes the tag to those cells (<c>W=12c H=5c</c>) so a client draws the picture in them.
 /// A terminal draws a figure's rows in its cells when its host has the picture's pixels. Every other format,
 /// and a client that refuses the picture, writes the wrapped text, so a terminal still learns there was a
 /// picture and where it is. Put it inside a link to make it one.</para>

MarkupString/Elements/PictureRow.cs

@@ -14,4 +14,13 @@ namespace MarkupString;
 /// <param name="Row">Which row of the picture this is, from 0.</param>
 /// <param name="Rows">How many rows the picture covers.</param>
 /// <param name="Columns">How many cells wide the picture is.</param>
-public readonly record struct PictureRow(int Row, int Rows, int Columns);
+public readonly record struct PictureRow(int Row, int Rows, int Columns)
+{
+	/// <summary>
+	/// Whether these cells hold the picture's description rather than cells laid out for it: the figure had no
+	/// art, and nothing gave the picture a size in cells. A format that sizes its picture element to the cells
+	/// it covers (MXP's <c>W</c> and <c>H</c>) leaves these alone, since a picture drawn in a line of its
+	/// description would be a strip.
+	/// </summary>
+	public bool IsDescription { get; init; }
+}

MarkupString/Layout/Blocks/Figure.cs

@@ -38,7 +38,7 @@ public override void Draw(LayoutContext context, int width, IList<MarkupText> li
 		if (art is null)
 		{
 			var described = MarkupText.Plain($"[{Description}]").FormatColumn(BlockText.Column(width, context.TextAlignment));
-			lines.AddRange(Image.Source.Length > 0 ? Marked(described, described.Max(line => line.DisplayWidth)) : described);
+			lines.AddRange(Image.Source.Length > 0 ? Marked(described, described.Max(line => line.DisplayWidth), description: true) : described);
 			if (Beside is { } after) context.Draw(after, width, lines);
 			return;
 		}
@@ -130,12 +130,16 @@ public override void Draw(LayoutContext context, int width, IList<MarkupText> li
 		return Marked(under, columns);
 	}
 
-	/// <summary><paramref name="rows"/>, each <paramref name="columns"/> cells wide, as the rows of the picture.</summary>
-	private MarkupText[] Marked(MarkupText[] rows, int columns)
+	/// <summary>
+	/// <paramref name="rows"/>, each <paramref name="columns"/> cells wide, as the rows of the picture: its
+	/// <paramref name="description"/> standing in for it, or cells laid out for it.
+	/// </summary>
+	private MarkupText[] Marked(MarkupText[] rows, int columns, bool description = false)
 	{
 		var marked = new MarkupText[rows.Length];
 		for (var row = 0; row < rows.Length; row++)
-			marked[row] = MarkupText.Wrap(Image with { Row = new PictureRow(row, rows.Length, columns) }, Unpictured(rows[row]));
+			marked[row] = MarkupText.Wrap(Image with { Row = new PictureRow(row, rows.Length, columns) { IsDescription = description } },
+				Unpictured(rows[row]));
 		return marked;
 	}
 

MarkupString/PublicAPI.Unshipped.txt

@@ -35,3 +35,5 @@ static MarkupString.PictureRow.operator !=(MarkupString.PictureRow left, MarkupS
 static MarkupString.PictureRow.operator ==(MarkupString.PictureRow left, MarkupString.PictureRow right) -> bool
 ~override MarkupString.PictureRow.Equals(object obj) -> bool
 ~override MarkupString.PictureRow.ToString() -> string
+MarkupString.PictureRow.IsDescription.get -> bool
+MarkupString.PictureRow.IsDescription.init -> void