From a6b05b83cd88201a6485f3c46e3ea2d12e265634 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 21 May 2023 12:18:48 +0200 Subject: [PATCH] Fixes #2632. Updates RuneJsonConverter to deal with more formats (#2640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../Configuration/ConfigurationManager.cs | 2 +- .../Configuration/RuneJsonConverter.cs | 141 +++++++++++++----- Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs | 14 ++ .../CursesDriver/CursesDriver.cs | 11 ++ Terminal.Gui/ConsoleDrivers/WindowsDriver.cs | 10 ++ Terminal.Gui/Drawing/Glyphs.cs | 19 ++- UICatalog/Scenarios/CharacterMap.cs | 4 +- UICatalog/Scenarios/Snake.cs | 9 +- .../Configuration/RuneJsonConverterTests.cs | 65 ++++++++ UnitTests/Drawing/GlyphTests.cs | 33 ++++ 10 files changed, 266 insertions(+), 42 deletions(-) create mode 100644 UnitTests/Configuration/RuneJsonConverterTests.cs create mode 100644 UnitTests/Drawing/GlyphTests.cs diff --git a/Terminal.Gui/Configuration/ConfigurationManager.cs b/Terminal.Gui/Configuration/ConfigurationManager.cs index e709bca8e..905540587 100644 --- a/Terminal.Gui/Configuration/ConfigurationManager.cs +++ b/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, diff --git a/Terminal.Gui/Configuration/RuneJsonConverter.cs b/Terminal.Gui/Configuration/RuneJsonConverter.cs index 2685f68eb..f7aae685f 100644 --- a/Terminal.Gui/Configuration/RuneJsonConverter.cs +++ b/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 { - /// - /// Json converter for . 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 - /// - internal class RuneJsonConverter : JsonConverter { - public override Rune Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - if (reader.TokenType == JsonTokenType.String) { +namespace Terminal.Gui; +/// +/// Json converter for . Supports +/// Json converter for . 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 +/// +internal class RuneJsonConverter : JsonConverter { + 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 () + .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]); - } - throw new JsonException ($"Invalid Rune format: {value}."); - } else if (reader.TokenType == JsonTokenType.Number) { - return new Rune (reader.GetUInt32 ()); - } - throw new JsonException ($"Unexpected StartObject token when parsing Rune: {reader.TokenType}."); - } + // Handle single character, surrogate pair, or combining mark + char + if (value.Length == 0 || value.Length > 2) { + throw new JsonException ($"Invalid Rune: {value}."); + } - 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})"); + 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 (not a scalar Unicode value): {num}."); + } + 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: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 diff --git a/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs b/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs index a2757d8de..56c6079ba 100644 --- a/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs @@ -751,6 +751,20 @@ namespace Terminal.Gui { /// Row to move the cursor to. public abstract void Move (int col, int row); + /// + /// Tests if the specified rune is supported by the driver. + /// + /// + /// if the rune can be properly presented; if the driver + /// does not support displaying this rune. + public virtual bool IsRuneSupported (Rune rune) + { + if (rune.Value > RuneExtensions.MaxUnicodeCodePoint) { + return false; + } + return true; + } + /// /// Adds the specified rune to the display at the current cursor position. /// diff --git a/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs b/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs index 263bd7600..eaba76d7e 100644 --- a/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs +++ b/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); diff --git a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs index 85ba30cde..b55f36a5e 100644 --- a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs +++ b/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 (); diff --git a/Terminal.Gui/Drawing/Glyphs.cs b/Terminal.Gui/Drawing/Glyphs.cs index 4f8941e32..6edb70107 100644 --- a/Terminal.Gui/Drawing/Glyphs.cs +++ b/Terminal.Gui/Drawing/Glyphs.cs @@ -8,8 +8,11 @@ namespace Terminal.Gui { /// /// /// + /// Access with (which is a global using alias for ). + /// + /// /// The default glyphs can be changed via the . Within a config.json file - /// The JSon property name is the property prefixed with "CM.Glyphs.". + /// The Json property name is the property name prefixed with "Glyphs.". /// /// /// 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)'-'; /// - /// Apple. Because snek. + /// Apple (non-BMP). Because snek. And because it's an example of a non-BMP surrogate pair. See Issue #2610. /// - public Rune Apple { get; set; } = (Rune)'❦' ; // BUGBUG: "🍎"[0] should work, but doesn't + public Rune Apple { get; set; } = "🍎".ToRunes () [0]; // nonBMP + + /// + /// Apple (BMP). Because snek. See Issue #2610. + /// + public Rune AppleBMP { get; set; } = (Rune)'❦'; + + ///// + ///// A nonprintable (low surrogate) that should fail to ctor. + ///// + //public Rune InvalidGlyph { get; set; } = (Rune)'\ud83d'; #endregion diff --git a/UICatalog/Scenarios/CharacterMap.cs b/UICatalog/Scenarios/CharacterMap.cs index f5b31c219..435757593 100644 --- a/UICatalog/Scenarios/CharacterMap.cs +++ b/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 ()); } diff --git a/UICatalog/Scenarios/Snake.cs b/UICatalog/Scenarios/Snake.cs index c1f1f2120..d16b514e7 100644 --- a/UICatalog/Scenarios/Snake.cs +++ b/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) diff --git a/UnitTests/Configuration/RuneJsonConverterTests.cs b/UnitTests/Configuration/RuneJsonConverterTests.cs new file mode 100644 index 000000000..75f5f49e9 --- /dev/null +++ b/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 (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 (() => JsonSerializer.Deserialize (json, ConfigurationManager._serializerOptions)); + } + +} + diff --git a/UnitTests/Drawing/GlyphTests.cs b/UnitTests/Drawing/GlyphTests.cs new file mode 100644 index 000000000..ca214402d --- /dev/null +++ b/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 (json, ConfigurationManager._serializerOptions); + + // Assert + Assert.Equal (((Rune)prop.GetValue (defs)).Value, deserialized.Value); + } + } + + } +}