Fixes #4382. StringExtensions.GetColumns method should only return the total text width and not the sum of all runes width (#4383)

* Fixes #4382. StringExtensions.GetColumns method should only return the total text width and not the sum of all runes width

* Trying to fix unit test error

* Update StringExtensions.cs

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

* Add unit test to prove that null and empty string doesn't not throws anything.

---------

Co-authored-by: Tig <[email protected]>
Co-authored-by: Copilot <[email protected]>

Terminal.Gui/Text/StringExtensions.cs

@@ -1,5 +1,6 @@
 #nullable enable
 using System.Buffers;
+using System.Globalization;
 
 namespace Terminal.Gui.Text;
 
@@ -55,7 +56,29 @@ public static class StringExtensions
     /// <remarks>This is a Terminal.Gui extension method to <see cref="string"/> to support TUI text manipulation.</remarks>
     /// <param name="str">The string to measure.</param>
     /// <returns></returns>
-    public static int GetColumns (this string str) { return str is null ? 0 : str.EnumerateRunes ().Sum (r => Math.Max (r.GetColumns (), 0)); }
+    public static int GetColumns (this string str)
+    {
+        if (string.IsNullOrEmpty (str))
+        {
+            return 0;
+        }
+
+        var total = 0;
+        TextElementEnumerator enumerator = StringInfo.GetTextElementEnumerator (str);
+
+        while (enumerator.MoveNext ())
+        {
+            string element = enumerator.GetTextElement ();
+
+            // Get the maximum rune width within this grapheme cluster
+            int width = element
+                        .EnumerateRunes ()
+                        .Max (r => Math.Max (r.GetColumns (), 0));
+            total += width;
+        }
+
+        return total;
+    }
 
     /// <summary>Gets the number of runes in the string.</summary>
     /// <remarks>This is a Terminal.Gui extension method to <see cref="string"/> to support TUI text manipulation.</remarks>

Tests/UnitTests/Views/HexViewTests.cs

@@ -72,7 +72,6 @@ public class HexViewTests
 
         Application.Top.Dispose ();
         Application.ResetState (true);
-
     }
 
     [Fact]
@@ -321,6 +320,7 @@ public class HexViewTests
     [Fact]
     public void PositionChanged_Event ()
     {
+        Application.Navigation = new ApplicationNavigation ();
         var hv = new HexView (LoadStream (null, out _)) { Width = 20, Height = 10 };
         Application.Top = new Toplevel ();
         Application.Top.Add (hv);
@@ -346,6 +346,7 @@ public class HexViewTests
     [Fact]
     public void Source_Sets_Address_To_Zero_If_Greater_Than_Source_Length ()
     {
+        Application.Navigation = new ApplicationNavigation ();
         var hv = new HexView (LoadStream (null, out _)) { Width = 10, Height = 5 };
         Application.Top = new Toplevel ();
         Application.Top.Add (hv);

Tests/UnitTestsParallelizable/Text/RuneTests.cs

@@ -7,7 +7,7 @@ namespace UnitTests_Parallelizable.TextTests;
 public class RuneTests
 {
     [Fact]
-    public void Cast_To_Char_Durrogate_Pair_Return_UTF16 ()
+    public void Cast_To_Char_Surrogate_Pair_Return_UTF16 ()
     {
         Assert.NotEqual ("𝔹", $"{new Rune (unchecked ((char)0x1d539))}");
         Assert.Equal ("픹", $"{new Rune (unchecked ((char)0x1d539))}");
@@ -65,8 +65,11 @@ public class RuneTests
         PrintTextElementCount ("\u0061\u0301", "á", 1, 2, 2, 1);
         PrintTextElementCount ("\u0061\u0301", "á", 1, 2, 2, 1);
         PrintTextElementCount ("\u0065\u0301", "é", 1, 2, 2, 1);
-        PrintTextElementCount ("\U0001f469\U0001f3fd\u200d\U0001f692", "👩🏽‍🚒", 6, 4, 7, 1);
+        PrintTextElementCount ("\U0001f469\U0001f3fd\u200d\U0001f692", "👩🏽‍🚒", 2, 4, 7, 1);
         PrintTextElementCount ("\ud801\udccf", "𐓏", 1, 1, 2, 1);
+        PrintTextElementCount ("\U0001F468\u200D\U0001F469\u200D\U0001F467\u200D\U0001F466", "👨‍👩‍👧‍👦", 2, 7, 11, 1);
+        PrintTextElementCount ("\U0001f469\u200d\U0001f692", "👩‍🚒", 2, 3, 5, 1);
+        PrintTextElementCount ("\u0068\u0069", "hi", 2, 2, 2, 2);
     }
 
     [Theory]
@@ -84,8 +87,8 @@ public class RuneTests
                     2,
                     1
                 )] // the letters 법 join to form the Korean word for "rice:" U+BC95 법 (read from top left to bottom right)
-    [InlineData ("\U0001F468\u200D\U0001F469\u200D\U0001F467", "👨‍👩‍👧", 8, 6, 8)] // Man, Woman and Girl emoji.
-    [InlineData ("\u0915\u093f", "कि", 2, 2, 2)] // Hindi कि with DEVANAGARI LETTER KA and DEVANAGARI VOWEL SIGN I
+    [InlineData ("\U0001F468\u200D\U0001F469\u200D\U0001F467", "👨‍👩‍👧", 8, 2, 8)] // Man, Woman and Girl emoji.
+    [InlineData ("\u0915\u093f", "कि", 2, 1, 2)] // Hindi कि with DEVANAGARI LETTER KA and DEVANAGARI VOWEL SIGN I
     [InlineData (
                     "\u0e4d\u0e32",
                     "ํา",

Tests/UnitTestsParallelizable/Text/StringTests.cs

@@ -33,11 +33,11 @@ public class StringTests
     [InlineData ("🙂", 2)]
     [InlineData ("a🙂", 3)]
     [InlineData ("🙂a", 3)]
-    [InlineData ("👨‍👩‍👦‍👦", 8)]
-    [InlineData ("👨‍👩‍👦‍👦🙂", 10)]
-    [InlineData ("👨‍👩‍👦‍👦🙂a", 11)]
-    [InlineData ("👨‍👩‍👦‍👦a🙂", 11)]
-    [InlineData ("👨‍👩‍👦‍👦👨‍👩‍👦‍👦", 16)]
+    [InlineData ("👨‍👩‍👦‍👦", 2)]
+    [InlineData ("👨‍👩‍👦‍👦🙂", 4)]
+    [InlineData ("👨‍👩‍👦‍👦🙂a", 5)]
+    [InlineData ("👨‍👩‍👦‍👦a🙂", 5)]
+    [InlineData ("👨‍👩‍👦‍👦👨‍👩‍👦‍👦", 4)]
     [InlineData ("山", 2)] // The character for "mountain" in Chinese/Japanese/Korean (山), Unicode U+5C71
     [InlineData ("山🙂", 4)] // The character for "mountain" in Chinese/Japanese/Korean (山), Unicode U+5C71
     //[InlineData ("\ufe20\ufe21", 2)] // Combining Ligature Left Half ︠ - U+fe20 -https://github.com/microsoft/terminal/blob/main/src/types/unicode_width_overrides.xml
@@ -57,4 +57,26 @@ public class StringTests
         var str = "a";
         Assert.Equal (1, str.GetColumns ());
     }
+
+    [Fact]
+    public void TestGetColumns_Zero_Width ()
+    {
+        var str = "\u200D";
+        Assert.Equal (0, str.GetColumns ());
+    }
+
+    [Theory]
+    [InlineData (null)]
+    [InlineData ("")]
+    public void TestGetColumns_Does_Not_Throws_With_Null_And_Empty_String (string? text)
+    {
+        if (text is null)
+        {
+            Assert.Equal (0, StringExtensions.GetColumns (text!));
+        }
+        else
+        {
+            Assert.Equal (0, text.GetColumns ());
+        }
+    }
 }