Expand tabs and replace control characters before measuring and drawing text [patch]

TUI.Test/SpectreConsoleProviderTests.cs

@@ -2,6 +2,7 @@
 
 namespace ktsu.TUI.Test;
 
+using ktsu.TUI.Core.Elements.Primitives;
 using ktsu.TUI.Core.Models;
 using ktsu.TUI.Core.Services;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -283,4 +284,37 @@ public void PartiallyTransparentBackgroundIsDrawnOpaque()
 
 		Assert.Contains("48;2;0;0;255", output.ToString());
 	}
+
+	/// <summary>
+	/// Tests that no control character from element text reaches the terminal, so text cannot
+	/// clear the screen or move the cursor itself (ktsu-dev/TUI#159).
+	/// </summary>
+	[TestMethod]
+	public void TextElementWritesNoControlCharactersToTheTerminal()
+	{
+		(SpectreConsoleProvider provider, StringWriter output) = CreateProvider();
+		TextElement element = new("x\u001b[2Jy")
+		{
+			Position = Position.Origin,
+			Dimensions = new Dimensions(Width, 1),
+		};
+
+		element.Render(provider);
+
+		Assert.AreEqual("\u001b[1;1Hx\uFFFD[2Jy", output.ToString());
+	}
+
+	/// <summary>
+	/// Tests that the provider replaces control characters in text written to it directly, as well
+	/// as in text an element has already normalised (ktsu-dev/TUI#159).
+	/// </summary>
+	[TestMethod]
+	public void WriteAtReplacesControlCharacters()
+	{
+		(SpectreConsoleProvider provider, StringWriter output) = CreateProvider();
+
+		provider.WriteAt("a\tb\u001bc\u009bd", new Position(0, 0));
+
+		Assert.AreEqual("\u001b[1;1Ha\uFFFDb\uFFFDc\uFFFDd", output.ToString());
+	}
 }

TUI.Test/TextElementTests.cs

@@ -588,4 +588,78 @@ public void WideTextIsMeasuredAndAlignedInCells()
 		Assert.AreEqual(6, required.Width);
 		Assert.AreEqual(4, provider.Writes.Single().Position.X);
 	}
+
+	/// <summary>
+	/// A tab inside a border must be measured as the cells it moves the cursor across, and drawn as
+	/// those cells, so the text stops at the border. It used to count as one cell and reach the
+	/// terminal raw, which jumped to the next tab stop and overwrote the right border
+	/// (ktsu-dev/TUI#159).
+	/// </summary>
+	[TestMethod]
+	public void TabInABorderMeasuresToTheWidthItDraws()
+	{
+		// Arrange
+		BorderElement border = [];
+		border.Child = new TextElement("Name\tValue");
+		border.Position = Position.Origin;
+		border.Dimensions = border.CalculateRequiredDimensions();
+		border.ArrangeChildren();
+		RecordingConsoleProvider provider = new();
+
+		// Act
+		border.Render(provider);
+
+		// Assert
+		Assert.AreEqual(new Dimensions(15, 3), border.Dimensions, "\"Name\" then a tab to column 8 then \"Value\" is 13 cells, plus the border");
+		RecordingConsoleProvider.Write write = provider.WritesOf("Name    Value").Single();
+		Assert.AreEqual(1, write.Position.X);
+		Assert.IsLessThanOrEqualTo(border.Dimensions.Width - 1, write.Position.X + CellWidth.Of(write.Text), "The text must stop before the right border");
+	}
+
+	/// <summary>
+	/// A tab must move to the next tab stop counted from the start of its own line, not from the
+	/// start of the text (ktsu-dev/TUI#159).
+	/// </summary>
+	[TestMethod]
+	public void TabsExpandToTheNextTabStopOnEachLine()
+	{
+		// Arrange
+		TextElement element = CreateElement("a\tb\nabcdefgh\tc\n\td");
+		RecordingConsoleProvider provider = new();
+
+		// Act
+		element.Render(provider);
+
+		// Assert
+		string[] written = [.. provider.Writes.Select(write => write.Text)];
+		Assert.HasCount(3, written);
+		Assert.AreEqual("a       b", written[0]);
+		Assert.AreEqual("abcdefgh        c", written[1]);
+		Assert.AreEqual("        d", written[2]);
+	}
+
+	/// <summary>
+	/// An escape or other control character in the text must not be written, or the text could
+	/// clear the screen or move the cursor in the middle of a frame (ktsu-dev/TUI#159).
+	/// </summary>
+	/// <param name="text">The text holding a control character.</param>
+	/// <param name="drawn">What should be drawn instead.</param>
+	[TestMethod]
+	[DataRow("x\u001b[2Jy", "x\uFFFD[2Jy")]
+	[DataRow("bell\u0007", "bell\uFFFD")]
+	[DataRow("\u009b2J", "\uFFFD2J")]
+	[DataRow("del\u007f", "del\uFFFD")]
+	public void ControlCharactersAreDrawnAsAPlaceholder(string text, string drawn)
+	{
+		// Arrange
+		TextElement element = CreateElement(text);
+		RecordingConsoleProvider provider = new();
+
+		// Act
+		element.Render(provider);
+
+		// Assert
+		Assert.AreEqual(drawn, provider.Writes.Single().Text);
+		Assert.AreEqual(new Dimensions(CellWidth.Of(drawn), 1), element.CalculateRequiredDimensions());
+	}
 }

TUI/Elements/Primitives/TextElement.cs

@@ -3,6 +3,7 @@
 namespace ktsu.TUI.Core.Elements.Primitives;
 
 using System.Globalization;
+using System.Text;
 using ktsu.TUI.Core.Contracts;
 using ktsu.TUI.Core.Models;
 using Spectre.Console;
@@ -12,6 +13,18 @@ namespace ktsu.TUI.Core.Elements.Primitives;
 /// </summary>
 public class TextElement : UIElementBase
 {
+	/// <summary>
+	/// The distance between tab stops, in cells. A tab in the text moves to the next multiple of
+	/// this, counted from the start of its line.
+	/// </summary>
+	public const int TabWidth = 8;
+
+	/// <summary>
+	/// What a control character in the text is drawn as, so it takes one visible cell instead of
+	/// reaching the terminal as a command.
+	/// </summary>
+	public const char ControlCharacterPlaceholder = '\uFFFD';
+
 	/// <summary>
 	/// Gets or sets the text to display
 	/// </summary>
@@ -186,14 +199,16 @@ private int CalculateVerticalPosition(int totalLines, int availableHeight, int b
 
 	/// <summary>
 	/// Breaks text into the lines that are measured and drawn: one per embedded line break, each
-	/// then wrapped to <paramref name="wrapWidth"/> when it is positive (ktsu-dev/TUI#135)
+	/// then wrapped to <paramref name="wrapWidth"/> when it is positive (ktsu-dev/TUI#135).
+	/// Tabs and other control characters are normalised first, so that what is measured is what
+	/// is drawn (ktsu-dev/TUI#159).
 	/// </summary>
 	/// <param name="text">The text to break</param>
 	/// <param name="wrapWidth">The width to wrap each line to, or 0 to leave lines unwrapped</param>
 	/// <returns>The lines, in order; a blank line in the text stays as an empty line</returns>
 	private static string[] SplitIntoLines(string text, int wrapWidth)
 	{
-		string[] paragraphs = text.Split(["\r\n", "\n", "\r"], StringSplitOptions.None);
+		string[] paragraphs = [.. text.Split(["\r\n", "\n", "\r"], StringSplitOptions.None).Select(NormalizeControlCharacters)];
 		if (wrapWidth <= 0)
 		{
 			return paragraphs;
@@ -207,6 +222,50 @@ private static string[] SplitIntoLines(string text, int wrapWidth)
 		})];
 	}
 
+	/// <summary>
+	/// Makes one line safe to measure and draw cell by cell. A tab becomes the spaces up to the
+	/// next tab stop, and any other control character becomes <see cref="ControlCharacterPlaceholder"/>.
+	/// Written raw, a tab jumps the terminal cursor past the cells it was measured as, and an
+	/// escape or other C0/C1 character lets the text move the cursor, recolour or clear the
+	/// screen (ktsu-dev/TUI#159).
+	/// </summary>
+	/// <param name="line">The line to normalise, with no line breaks in it</param>
+	/// <returns>The line with no control characters left in it</returns>
+	internal static string NormalizeControlCharacters(string line)
+	{
+		if (!line.Any(char.IsControl))
+		{
+			return line;
+		}
+
+		StringBuilder normalized = new(line.Length);
+		int cells = 0;
+		TextElementEnumerator elements = StringInfo.GetTextElementEnumerator(line);
+		while (elements.MoveNext())
+		{
+			// A control character is always a text element of its own
+			string element = elements.GetTextElement();
+			if (element == "\t")
+			{
+				int spaces = TabWidth - (cells % TabWidth);
+				normalized.Append(' ', spaces);
+				cells += spaces;
+			}
+			else if (element.Length == 1 && char.IsControl(element[0]))
+			{
+				normalized.Append(ControlCharacterPlaceholder);
+				cells++;
+			}
+			else
+			{
+				normalized.Append(element);
+				cells += element.GetCellWidth();
+			}
+		}
+
+		return normalized.ToString();
+	}
+
 	/// <summary>
 	/// Wraps one paragraph to <paramref name="maxWidth"/> cells. Spaces are kept as written,
 	/// including leading indentation and runs between words, and are dropped only where a line

TUI/Services/SpectreConsoleProvider.cs

@@ -3,6 +3,7 @@
 namespace ktsu.TUI.Core.Services;
 
 using ktsu.TUI.Core.Contracts;
+using ktsu.TUI.Core.Elements.Primitives;
 using ktsu.TUI.Core.Models;
 using Spectre.Console;
 
@@ -65,6 +66,11 @@ public void WriteAt(string text, Position position, TextStyle? style = null)
 			return;
 		}
 
+		// Nothing written may command the terminal: a control character from the text would move
+		// the cursor, recolour or clear the screen. Elements normalise their own text, so this only
+		// catches text reaching the provider some other way (ktsu-dev/TUI#159).
+		text = ReplaceControlCharacters(text);
+
 		// Clip to the screen before moving the cursor. SetCursorPosition ignores an off-screen
 		// position, so writing anyway would put the text wherever the last write left the cursor,
 		// and text running past the right edge would wrap onto the next line (ktsu-dev/TUI#139).
@@ -208,4 +214,20 @@ private static string BuildStyleString(TextStyle style)
 
 		return string.Join(" ", parts);
 	}
+
+	private static string ReplaceControlCharacters(string text)
+	{
+		if (!text.Any(char.IsControl))
+		{
+			return text;
+		}
+
+		return string.Create(text.Length, text, static (chars, source) =>
+		{
+			for (int i = 0; i < source.Length; i++)
+			{
+				chars[i] = char.IsControl(source[i]) ? TextElement.ControlCharacterPlaceholder : source[i];
+			}
+		});
+	}
 }