Fixes #2632. Updates RuneJsonConverter to deal with more formats (#2640)

* Remove NStack and replace ustring to string.

* Add unit test and improving some code.

* Adjust code and fix all unit tests errors.

* Add XML Document and move the Rune folder into the Text folder.

* Improve unit tests with byte array on DecodeRune and DecodeLastRune.

* Fix unit test.

* 😂Code review

* Reduce unit tests code.

* Change StringExtensions.Make to StringExtensions.ToString and added some more unit tests.

* Fix merge errors.

* Remove GetTextWidth and calls replaced with StringExtensions.GetColumns.

* Hack to use UseSystemConsole passed in the command line arguments.

* Revert "Hack to use UseSystemConsole passed in the command line arguments."

This reverts commit b74d11c7864fa6e20d40ef5cbead89a42f81ee5e.

* Remove Application.UseSystemConsole from the config file.

* Fix errors related by removing UseSystemConsole from the config file.

* Fixes #2633. DecodeEscSeq throw an exception if cki is null.

* Fix an exception if SelectedItem is -1.

* Set SelectedItem to 0 and remove unnecessary ToString.

* Updated RuneJsonConverter to deal with more formats

* nonBMP apple

* Adjusted unit tests

* Added ConsoleDriver.IsRuneSupported API

* Removed debug code

* Disabled non-BMP in CursesDriver

---------

Co-authored-by: BDisp <[email protected]>

Terminal.Gui/Configuration/ConfigurationManager.cs

@@ -57,7 +57,7 @@ public static partial class ConfigurationManager {
 
 	private static readonly string _configFilename = "config.json";
 
-	private static readonly JsonSerializerOptions _serializerOptions = new JsonSerializerOptions {
+	internal static readonly JsonSerializerOptions _serializerOptions = new JsonSerializerOptions {
 		ReadCommentHandling = JsonCommentHandling.Skip,
 		PropertyNameCaseInsensitive = true,
 		DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,

Terminal.Gui/Configuration/RuneJsonConverter.cs

@@ -1,48 +1,119 @@
 using System;
+using System.Globalization;
+using System.Linq;
 using System.Text;
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using System.Text.RegularExpressions;
 
-namespace Terminal.Gui {
-	/// <summary>
-	/// Json converter for <see cref="Rune"/>. Supports
-	/// A string as one of:
-	/// - unicode char (e.g. "☑")
-	/// - U+hex format (e.g. "U+2611")
-	/// - \u format (e.g. "\\u2611")
-	/// A number
-	/// - The unicode code in decimal
-	/// </summary>
-	internal class RuneJsonConverter : JsonConverter<Rune> {
-		public override Rune Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
-		{
-			if (reader.TokenType == JsonTokenType.String) {
+namespace Terminal.Gui;
+/// <summary>
+/// Json converter for <see cref="Rune"/>. Supports
+/// Json converter for <see cref="Rune"/>. Supports
+/// A string as one of:
+/// - unicode char (e.g. "☑")
+/// - U+hex format (e.g. "U+2611")
+/// - \u format (e.g. "\\u2611")
+/// A number
+/// - The unicode code in decimal
+/// </summary>
+internal class RuneJsonConverter : JsonConverter<Rune> {
+	public override Rune Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+	{
+		switch (reader.TokenType) {
+		case JsonTokenType.String: {
 				var value = reader.GetString ();
-				if (value.StartsWith ("U+", StringComparison.OrdinalIgnoreCase) || value.StartsWith ("\\u")) {
-					try {
-						uint result = uint.Parse (value [2..^0], System.Globalization.NumberStyles.HexNumber);
-						return new Rune (result);
-					} catch (FormatException e) {
-						throw new JsonException ($"Invalid Rune format: {value}.", e);
+				int first = RuneExtensions.MaxUnicodeCodePoint + 1;
+				int second = RuneExtensions.MaxUnicodeCodePoint + 1;
+
+				if (value.StartsWith ("U+", StringComparison.OrdinalIgnoreCase) || value.StartsWith ("\\U", StringComparison.OrdinalIgnoreCase)) {
+					// Handle encoded single char, surrogate pair, or combining mark + char
+					var codePoints = Regex.Matches (value, @"(?:\\[uU]\+?|U\+)([0-9A-Fa-f]{1,8})")
+						.Cast<Match> ()
+						.Select (match => uint.Parse (match.Groups [1].Value, NumberStyles.HexNumber))
+						.ToArray ();
+
+					if (codePoints.Length == 0 || codePoints.Length > 2) {
+						throw new JsonException ($"Invalid Rune: {value}.");
+					}
+
+					if (codePoints.Length > 0) {
+						first = (int)codePoints [0];
+					}
+
+					if (codePoints.Length == 2) {
+						second = (int)codePoints [1];
 					}
 				} else {
-					return new Rune (value [0]);
+					// Handle single character, surrogate pair, or combining mark + char
+					if (value.Length == 0 || value.Length > 2) {
+						throw new JsonException ($"Invalid Rune: {value}.");
+					}
+
+					if (value.Length > 0) {
+						first = value [0];
+					}
+					if (value.Length == 2) {
+						second = value [1];
+					}
+				}
+
+				Rune result;
+				if (second == RuneExtensions.MaxUnicodeCodePoint + 1) {
+					// Single codepoint
+					if (!Rune.TryCreate (first, out result)) {
+						throw new JsonException ($"Invalid Rune: {value}.");
+					}
+					return result;
+				}
+
+				// Surrogate pair?
+				if (Rune.TryCreate ((char)first, (char)second, out result)) {
+					return result;
+				}
+
+				if (!Rune.IsValid (second)) {
+					throw new JsonException ($"The second codepoint is not valid: {second} in ({value})");
+				}
+
+				var cm = new Rune (second);
+				if (!cm.IsCombiningMark ()) {
+					throw new JsonException ($"The second codepoint is not a combining mark: {cm} in ({value})");
+				}
+
+				// not a surrogate pair, so a combining mark + char?
+				var combined = string.Concat ((char)first, (char)second).Normalize ();
+
+				if (!Rune.IsValid (combined [0])) {
+					throw new JsonException ($"Invalid combined Rune ({value})");
+				}
+
+				return new Rune (combined [0]);
+			}
+		case JsonTokenType.Number: {
+				uint num = reader.GetUInt32 ();
+				if (Rune.IsValid (num)) {
+					return new Rune (num);
 				}
-				throw new JsonException ($"Invalid Rune format: {value}.");
-			} else if (reader.TokenType == JsonTokenType.Number) {
-				return new Rune (reader.GetUInt32 ());
+				throw new JsonException ($"Invalid Rune (not a scalar Unicode value): {num}.");
 			}
-			throw new JsonException ($"Unexpected StartObject token when parsing Rune: {reader.TokenType}.");
+		default:
+			throw new JsonException ($"Unexpected token when parsing Rune: {reader.TokenType}.");
 		}
+	}
 
-		public override void Write (Utf8JsonWriter writer, Rune value, JsonSerializerOptions options)
-		{
-			// HACK: Writes a JSON comment in addition to the glyph to ease debugging.
-			// Technically, JSON comments are not valid, but we use relaxed decoding
-			// (ReadCommentHandling = JsonCommentHandling.Skip)
-			writer.WriteCommentValue ($"(U+{value.Value:X4})");
+	public override void Write (Utf8JsonWriter writer, Rune value, JsonSerializerOptions options)
+	{
+		// HACK: Writes a JSON comment in addition to the glyph to ease debugging.
+		// Technically, JSON comments are not valid, but we use relaxed decoding
+		// (ReadCommentHandling = JsonCommentHandling.Skip)
+		writer.WriteCommentValue ($"(U+{value.Value:X8})");
+		var printable = value.MakePrintable ();
+		if (printable == Rune.ReplacementChar) {
+			writer.WriteStringValue (value.ToString ());
+		} else {
 			writer.WriteRawValue ($"\"{value}\"");
 		}
 	}
-#pragma warning restore 1591
 }
+#pragma warning restore 1591

Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs

@@ -751,6 +751,20 @@ namespace Terminal.Gui {
 		/// <param name="row">Row to move the cursor to.</param>
 		public abstract void Move (int col, int row);
 
+		/// <summary>
+		/// Tests if the specified rune is supported by the driver.
+		/// </summary>
+		/// <param name="rune"></param>
+		/// <returns><see langword="true"/> if the rune can be properly presented; <see langword="false"/> if the driver
+		/// does not support displaying this rune.</returns>
+		public virtual bool IsRuneSupported (Rune rune)
+		{
+			if (rune.Value > RuneExtensions.MaxUnicodeCodePoint) {
+				return false;
+			}
+			return true;
+		}
+		
 		/// <summary>
 		/// Adds the specified rune to the display at the current cursor position.
 		/// </summary>

Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs

@@ -48,8 +48,19 @@ namespace Terminal.Gui {
 		}
 
 		static bool sync = false;
+
+		public override bool IsRuneSupported (Rune rune)
+		{
+			// See Issue #2615 - CursesDriver is broken with non-BMP characters
+			return base.IsRuneSupported (rune) && rune.IsBmp;
+		}
+
 		public override void AddRune (Rune rune)
 		{
+			if (!IsRuneSupported (rune)) {
+				rune = Rune.ReplacementChar;
+			}
+
 			rune = rune.MakePrintable ();
 			var runeWidth = rune.GetColumns ();
 			var validClip = IsValidContent (ccol, crow, Clip);

Terminal.Gui/ConsoleDrivers/WindowsDriver.cs

@@ -1518,8 +1518,18 @@ namespace Terminal.Gui {
 			return crow * Cols + ccol;
 		}
 
+		public override bool IsRuneSupported (Rune rune)
+		{
+			// See Issue #2610
+			return base.IsRuneSupported (rune) && rune.IsBmp;
+		}
+
 		public override void AddRune (Rune rune)
 		{
+			if (!IsRuneSupported(rune)) {
+				rune = Rune.ReplacementChar;
+			}
+			
 			rune = rune.MakePrintable ();
 			var runeWidth = rune.GetColumns ();
 			var position = GetOutputBufferPosition ();

Terminal.Gui/Drawing/Glyphs.cs

@@ -8,8 +8,11 @@ namespace Terminal.Gui {
 	/// </summary>
 	/// <remarks>
 	/// <para>
+	/// Access with <see cref="CM.Glyphs"/> (which is a global using alias for <see cref="ConfigurationManager.Glyphs"/>).
+	/// </para>
+	/// <para>
 	/// The default glyphs can be changed via the <see cref="ConfigurationManager"/>. Within a <c>config.json</c> file 
-	/// The JSon property name is the <see cref="GlyphDefinitions"/> property prefixed with "CM.Glyphs.". 
+	/// The Json property name is the property name prefixed with "Glyphs.". 
 	/// </para>
 	/// <para>
 	/// The JSon property can be either a decimal number or a string. The string may be one of:
@@ -137,9 +140,19 @@ namespace Terminal.Gui {
 		public Rune Collapse { get; set; } = (Rune)'-';
 
 		/// <summary>
-		/// Apple. Because snek.
+		/// Apple (non-BMP). Because snek. And because it's an example of a non-BMP surrogate pair. See Issue #2610.
+		/// </summary>
+		public Rune Apple { get; set; } = "🍎".ToRunes () [0]; // nonBMP
+
+		/// <summary>
+		/// Apple (BMP). Because snek. See Issue #2610.
 		/// </summary>
-		public Rune Apple { get; set; } = (Rune)'❦' ; // BUGBUG: "🍎"[0] should work, but doesn't 
+		public Rune AppleBMP { get; set; } = (Rune)'❦';
+
+		///// <summary>
+		///// A nonprintable (low surrogate) that should fail to ctor.
+		///// </summary>
+		//public Rune InvalidGlyph { get; set; } = (Rune)'\ud83d';
 
 		#endregion
 

UICatalog/Scenarios/CharacterMap.cs

@@ -403,7 +403,9 @@ class CharMap : ScrollView {
 				if (cursorRow + ContentOffset.Y + 1 == y && cursorCol + ContentOffset.X + firstColumnX + 1 == x && !HasFocus) {
 					Driver.SetAttribute (GetFocusColor ());
 				}
-				Driver.AddRune (new Rune ((char)(val + col)));
+				
+				Driver.AddRune (new Rune (val + col));
+
 				if (cursorRow + ContentOffset.Y + 1 == y && cursorCol + ContentOffset.X + firstColumnX + 1 == x && !HasFocus) {
 					Driver.SetAttribute (GetNormalColor ());
 				}

UICatalog/Scenarios/Snake.cs

@@ -59,7 +59,7 @@ namespace UICatalog.Scenarios {
 		}
 
 		private class SnakeView : View {
-
+			Rune _appleRune;
 			private Attribute red = new Terminal.Gui.Attribute (Color.Red, Color.Black);
 			private Attribute white = new Terminal.Gui.Attribute (Color.White, Color.Black);
 
@@ -67,6 +67,11 @@ namespace UICatalog.Scenarios {
 
 			public SnakeView (SnakeState state)
 			{
+				_appleRune = CM.Glyphs.Apple;
+				if (!Driver.IsRuneSupported (_appleRune)) {
+					_appleRune = CM.Glyphs.AppleBMP;
+				}
+
 				State = state;
 				CanFocus = true;
 
@@ -116,7 +121,7 @@ namespace UICatalog.Scenarios {
 				}
 
 				Driver.SetAttribute (red);
-				AddRune (State.Apple.X, State.Apple.Y, CM.Glyphs.Apple);
+				AddRune (State.Apple.X, State.Apple.Y, _appleRune);
 				Driver.SetAttribute (white);
 			}
 			public override bool OnKeyDown (KeyEvent keyEvent)

UnitTests/Configuration/RuneJsonConverterTests.cs

@@ -0,0 +1,65 @@
+using System.Text;
+using Xunit;
+using System.Text.Json;
+
+namespace Terminal.Gui.ConfigurationTests;
+public class RunJsonConverterTests {
+
+	[Theory]
+	[InlineData ("a", "a")]
+	[InlineData ("☑", "☑")]
+	[InlineData ("\\u2611", "☑")]
+	[InlineData ("U+2611", "☑")]
+	[InlineData ("🍎", "🍎")]
+	[InlineData ("U+1F34E", "🍎")]
+	[InlineData ("\\U0001F34E", "🍎")]
+	[InlineData ("\\ud83d \\udc69", "👩")]
+	[InlineData ("\\ud83d\\udc69", "👩")]
+	[InlineData ("U+d83d U+dc69", "👩")]
+	[InlineData ("U+1F469", "👩")]
+	[InlineData ("\\U0001F469", "👩")]
+	[InlineData ("\\u0065\\u0301", "é")]
+	public void RoundTripConversion_Positive (string rune, string expected)
+	{
+		// Arrange
+
+		// Act
+		var json = JsonSerializer.Serialize (rune, ConfigurationManager._serializerOptions);
+		var deserialized = JsonSerializer.Deserialize<Rune> (json, ConfigurationManager._serializerOptions);
+		
+		// Assert
+		Assert.Equal (expected, deserialized.ToString ());
+	}
+
+	[Theory]
+	[InlineData ("aa")]
+	[InlineData ("☑☑")]
+	[InlineData ("\\x2611")]
+	[InlineData ("Z+2611")]
+	[InlineData ("🍎🍎")]
+	[InlineData ("U+FFF1F34E")]
+	[InlineData ("\\UFFF1F34E")]
+	[InlineData ("\\ud83d")] // not printable
+	[InlineData ("\\ud83d \\u1c69")]  // bad surrogate pair
+	[InlineData ("\\ud83ddc69")]
+	// Emoji - Family Unit:
+	// Woman (U+1F469, 👩)
+	// Zero Width Joiner (U+200D)
+	// Woman (U+1F469, 👩)
+	// Zero Width Joiner (U+200D)
+	// Girl (U+1F467, 👧)
+	// Zero Width Joiner (U+200D)
+	// Girl (U+1F467, 👧)
+	[InlineData ("U+1F469 U+200D U+1F469 U+200D U+1F467 U+200D U+1F467")]
+	[InlineData ("\\U0001F469\\u200D\\U0001F469\\u200D\\U0001F467\\u200D\\U0001F467")]
+	public void RoundTripConversion_Negative (string rune)
+	{
+		// Act
+		var json = JsonSerializer.Serialize (rune, ConfigurationManager._serializerOptions);
+
+		// Assert
+		Assert.Throws<JsonException> (() => JsonSerializer.Deserialize<Rune> (json, ConfigurationManager._serializerOptions));
+	}
+
+}
+

UnitTests/Drawing/GlyphTests.cs

@@ -0,0 +1,33 @@
+using System;
+using System.Buffers;
+using System.Collections.Generic;
+using System.Data;
+using System.Globalization;
+using System.Linq;
+using System.Text;
+using System.Text.Json;
+using Xunit;
+
+namespace Terminal.Gui.DrawingTests;
+
+public class GlyphTests {
+	[Fact]
+	public void Default_GlyphDefinitions_Deserialize ()
+	{
+		var defs = new GlyphDefinitions ();
+		// enumerate all properties in GlyphDefinitions
+		foreach (var prop in typeof (GlyphDefinitions).GetProperties ()) {
+			if (prop.PropertyType == typeof (Rune)) {
+
+				// Act
+				var rune = (Rune)prop.GetValue (defs);
+				var json = JsonSerializer.Serialize (rune, ConfigurationManager._serializerOptions);
+				var deserialized = JsonSerializer.Deserialize<Rune> (json, ConfigurationManager._serializerOptions);
+
+				// Assert
+				Assert.Equal (((Rune)prop.GetValue (defs)).Value, deserialized.Value);
+			}
+		}
+
+	}
+}