From 5672555c716321469c1ee09a2f3bc3a7c2152bfa Mon Sep 17 00:00:00 2001 From: Adam Krantz Date: Fri, 6 Mar 2026 05:25:45 -0800 Subject: [PATCH 1/2] Use string.GetColumns() for display width Replace EnumerateRunes().Sum(GetColumns) with string.GetColumns() for grapheme-aware width measurement. The rune-sum pattern produces incorrect (inflated) results for multi-rune grapheme clusters like ZWJ emoji sequences -- e.g. a family emoji measures 8 columns by rune sum but occupies only 2 terminal cells. Also fix Branch.GetWidth() which used string.Length instead of GetColumns(), under-counting width for wide CJK characters. Fixed instances: - AppendAutocomplete.cs: autocomplete fragment width - ColorBar.cs: color label offset (removed obsolete TODO comment) - FileDialog.cs: feedback text centering - TabView.cs: tab header width - TableView.cs (4 instances): column auto-sizing, truncation, padding - Branch.cs: line truncation check + GetWidth() calculation Added tests: - StringTests: 7 parameterized cases documenting rune-sum vs GetColumns discrepancy for ZWJ emoji, CJK, combining marks, and ASCII - TableViewTests: grapheme cluster width comparison + rendering test - TabViewTests: rendering test verifying emoji tab header width - TreeViewTests: rendering test with CJK wide characters Fixes #4798 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Views/Autocomplete/AppendAutocomplete.cs | 2 +- Terminal.Gui/Views/Color/ColorBar.cs | 3 +- Terminal.Gui/Views/FileDialogs/FileDialog.cs | 2 +- Terminal.Gui/Views/TabView/TabView.cs | 2 +- Terminal.Gui/Views/TableView/TableView.cs | 8 +- Terminal.Gui/Views/TreeView/Branch.cs | 4 +- Tests/UnitTests/Views/TabViewTests.cs | 55 ++++++++++++ Tests/UnitTests/Views/TableViewTests.cs | 83 +++++++++++++++++++ Tests/UnitTests/Views/TreeViewTests.cs | 39 +++++++++ .../Text/StringTests.cs | 27 ++++++ 10 files changed, 214 insertions(+), 11 deletions(-) diff --git a/Terminal.Gui/Views/Autocomplete/AppendAutocomplete.cs b/Terminal.Gui/Views/Autocomplete/AppendAutocomplete.cs index a9d5cd2921..92bff2ae3a 100644 --- a/Terminal.Gui/Views/Autocomplete/AppendAutocomplete.cs +++ b/Terminal.Gui/Views/Autocomplete/AppendAutocomplete.cs @@ -121,7 +121,7 @@ public override void RenderOverlay (Point renderAt) string fragment = suggestion.Replacement.Substring (suggestion.Remove); int spaceAvailable = _textField.Viewport.Width - _textField.Text.GetColumns (); - int spaceRequired = fragment.EnumerateRunes ().Sum (c => c.GetColumns ()); + int spaceRequired = fragment.GetColumns (); if (spaceAvailable < spaceRequired) { diff --git a/Terminal.Gui/Views/Color/ColorBar.cs b/Terminal.Gui/Views/Color/ColorBar.cs index 9a4979cab2..93294419b8 100644 --- a/Terminal.Gui/Views/Color/ColorBar.cs +++ b/Terminal.Gui/Views/Color/ColorBar.cs @@ -90,8 +90,7 @@ protected override void OnSubViewsLaidOut (LayoutEventArgs args) if (!string.IsNullOrWhiteSpace (Text)) { - // TODO: is there a better method than this? this is what it is in TableView - xOffset = Text.EnumerateRunes ().Sum (c => c.GetColumns ()); + xOffset = Text.GetColumns (); } _barWidth = Viewport.Width - xOffset; diff --git a/Terminal.Gui/Views/FileDialogs/FileDialog.cs b/Terminal.Gui/Views/FileDialogs/FileDialog.cs index 90cab0841f..796d3045d1 100644 --- a/Terminal.Gui/Views/FileDialogs/FileDialog.cs +++ b/Terminal.Gui/Views/FileDialogs/FileDialog.cs @@ -400,7 +400,7 @@ protected override bool OnDrawingContent (DrawContext? context) { if (!string.IsNullOrWhiteSpace (_feedback)) { - int feedbackWidth = _feedback.EnumerateRunes ().Sum (c => c.GetColumns ()); + int feedbackWidth = _feedback.GetColumns (); int feedbackPadLeft = (Viewport.Width - feedbackWidth) / 2 - 1; feedbackPadLeft = Math.Min (Viewport.Width, feedbackPadLeft); diff --git a/Terminal.Gui/Views/TabView/TabView.cs b/Terminal.Gui/Views/TabView/TabView.cs index 18b5dd7e56..b3ace8fb30 100644 --- a/Terminal.Gui/Views/TabView/TabView.cs +++ b/Terminal.Gui/Views/TabView/TabView.cs @@ -532,7 +532,7 @@ internal IEnumerable CalculateViewport (Rectangle bounds) tab.Y = 0; // while there is space for the tab - int tabTextWidth = tab.DisplayText.EnumerateRunes ().Sum (c => c.GetColumns ()); + int tabTextWidth = tab.DisplayText.GetColumns (); // The maximum number of characters to use for the tab name as specified // by the user (MaxTabTextWidth). But not more than the width of the view diff --git a/Terminal.Gui/Views/TableView/TableView.cs b/Terminal.Gui/Views/TableView/TableView.cs index 15b6b0effd..904991e63f 100644 --- a/Terminal.Gui/Views/TableView/TableView.cs +++ b/Terminal.Gui/Views/TableView/TableView.cs @@ -397,7 +397,7 @@ private void AddRuneAt (int col, int row, Rune ch) /// private int CalculateMaxCellWidth (int col, ColumnStyle? colStyle, int startRow, int rowsToRender) { - int spaceRequired = _table!.ColumnNames [col].EnumerateRunes ().Sum (c => c.GetColumns ()); + int spaceRequired = _table!.ColumnNames [col].GetColumns (); // if table has no rows if (Table is not { Rows: > 0 }) @@ -408,7 +408,7 @@ private int CalculateMaxCellWidth (int col, ColumnStyle? colStyle, int startRow, for (int i = startRow; i < startRow + rowsToRender; i++) { // expand required space if cell is bigger than the last biggest cell or header - spaceRequired = Math.Max (spaceRequired, GetRepresentation (Table [i, col], colStyle).EnumerateRunes ().Sum (c => c.GetColumns ())); + spaceRequired = Math.Max (spaceRequired, GetRepresentation (Table [i, col], colStyle).GetColumns ()); } // Don't require more space than the style allows @@ -666,13 +666,13 @@ private string TruncateOrPad (object originalCellValue, string representation, i } // if value is too wide - if (representation.EnumerateRunes ().Sum (c => c.GetColumns ()) >= availableHorizontalSpace) + if (representation.GetColumns () >= availableHorizontalSpace) { return new string (representation.TakeWhile (c => (availableHorizontalSpace -= ((Rune)c).GetColumns ()) > 0).ToArray ()); } // pad it out with spaces to the given alignment - int toPad = availableHorizontalSpace - (representation.EnumerateRunes ().Sum (c => c.GetColumns ()) + 1 /*leave 1 space for cell boundary*/); + int toPad = availableHorizontalSpace - (representation.GetColumns () + 1 /*leave 1 space for cell boundary*/); return (colStyle?.GetAlignment (originalCellValue) ?? Alignment.Start) switch { diff --git a/Terminal.Gui/Views/TreeView/Branch.cs b/Terminal.Gui/Views/TreeView/Branch.cs index 481adafacf..80d49c24aa 100644 --- a/Terminal.Gui/Views/TreeView/Branch.cs +++ b/Terminal.Gui/Views/TreeView/Branch.cs @@ -175,7 +175,7 @@ public virtual void Draw (int y, int availableWidth) } // If body of line is too long - if (lineBody.EnumerateRunes ().Sum (l => l.GetColumns ()) > availableWidth) + if (lineBody.GetColumns () > availableWidth) { // remaining space is zero and truncate the line lineBody = new ( @@ -313,7 +313,7 @@ public string GetExpandableSymbol () public virtual int GetWidth () { return - GetLinePrefix ().Sum (r => r.GetColumns ()) + GetExpandableSymbol ().GetColumns () + (_tree.AspectGetter (Model) ?? "").Length; + GetLinePrefix ().Sum (r => r.GetColumns ()) + GetExpandableSymbol ().GetColumns () + (_tree.AspectGetter (Model) ?? "").GetColumns (); } /// Refreshes cached knowledge in this branch e.g. what children an object has. diff --git a/Tests/UnitTests/Views/TabViewTests.cs b/Tests/UnitTests/Views/TabViewTests.cs index 80cbbfad8e..8ffaddb9d7 100644 --- a/Tests/UnitTests/Views/TabViewTests.cs +++ b/Tests/UnitTests/Views/TabViewTests.cs @@ -1061,6 +1061,61 @@ public void ShowTopLine_True_TabsOnBottom_False_With_Unicode () ); } + /// + /// Verifies that measures tab text width using grapheme-aware + /// string.GetColumns() rather than EnumerateRunes().Sum(GetColumns). + /// A ZWJ family emoji should occupy 2 cells as a tab name, not 8. + /// + [Fact] + [SetupFakeApplication] + public void ShowTopLine_True_TabTextWidth_GraphemeCluster () + { + // setup + var tv = new TabView () + { + Driver = ApplicationImpl.Instance.Driver, + Id = "tv" + }; + tv.BeginInit (); + tv.EndInit (); + + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + + tv.AddTab ( + new () { Id = "emojiTab", DisplayText = family, View = new TextField { Id = "tf", Width = 4, Text = "hi" } }, + false + ); + tv.AddTab (new () { Id = "tab2", DisplayText = "B", View = new Label { Id = "lbl", Text = "hi2" } }, false); + tv.Width = 20; + tv.Height = 5; + + // execute + tv.Layout (); + tv.SetClipToScreen (); + tv.Draw (); + + // verify β€” the emoji tab header should be ~2 cells wide (grapheme width), + // not ~8 cells wide (rune sum width) + string actual = Application.Driver!.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + + // Find the tab header row (contains the emoji and "B") + string? headerRow = lines.FirstOrDefault (l => l.Contains ('B') && l.Length > 1); + Assert.NotNull (headerRow); + + // The emoji tab should be narrow. With the fix, the tab text is 2 cells. + // Without the fix, the tab text would be 8 cells, pushing "B" much further right. + // Note: IndexOf gives string index (includes multi-byte emoji chars), + // so we measure display columns of the substring before 'B'. + int bIndex = headerRow.IndexOf ('B'); + int bColumnPosition = headerRow [..bIndex].GetColumns (); + + Assert.True ( + bColumnPosition <= 8, + $"Tab 'B' should be near the start (emoji tab is 2 cells wide), but found at column {bColumnPosition}. Row: '{headerRow}'" + ); + } + [Fact] [SetupFakeApplication] public void ShowTopLine_True_TabsOnBottom_True_TestTabView_Width3 () diff --git a/Tests/UnitTests/Views/TableViewTests.cs b/Tests/UnitTests/Views/TableViewTests.cs index 2a0dcb14a2..3e0200bf97 100644 --- a/Tests/UnitTests/Views/TableViewTests.cs +++ b/Tests/UnitTests/Views/TableViewTests.cs @@ -1799,6 +1799,89 @@ public void Test_SumColumnWidth_UnicodeLength () // The unicode width of this string is shorter than the string length! Assert.Equal (14, surrogate.EnumerateRunes ().Sum (c => c.GetColumns ())); Assert.Equal (15, surrogate.Length); + + // For combining marks, EnumerateRunes().Sum() and GetColumns() agree + Assert.Equal (14, surrogate.GetColumns ()); + } + + /// + /// Demonstrates that string.GetColumns() should be used instead of + /// EnumerateRunes().Sum(GetColumns) for display width measurement, because the rune-sum approach + /// produces inflated widths for grapheme clusters like ZWJ emoji sequences. + /// This is relevant to and TruncateOrPad. + /// + [Fact] + public void Test_SumColumnWidth_GraphemeClusters () + { + // ZWJ family emoji: 4 person emoji (width 2 each) + 3 ZWJ chars (width 0) + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + + // EnumerateRunes().Sum() gives 8 β€” WRONG for display width + Assert.Equal (8, family.EnumerateRunes ().Sum (c => c.GetColumns ())); + + // string.GetColumns() correctly treats the ZWJ sequence as one grapheme cluster (width 2) + Assert.Equal (2, family.GetColumns ()); + + // Woman technologist: 2 emoji (width 2 each) + ZWJ + string technologist = "\U0001F469\u200D\U0001F4BB"; // πŸ‘©β€πŸ’» + Assert.Equal (4, technologist.EnumerateRunes ().Sum (c => c.GetColumns ())); + Assert.Equal (2, technologist.GetColumns ()); + } + + /// + /// Verifies that auto-sizes columns based on grapheme-aware + /// string.GetColumns(), not EnumerateRunes().Sum(GetColumns). + /// With the buggy rune-sum, a ZWJ family emoji (width 8 by rune sum) would cause + /// the column to be much wider than needed. With the fix, the column sizes to 2 cells. + /// + [Fact] + [SetupFakeApplication] + public void Test_CalculateMaxCellWidth_UsesGraphemeWidth () + { + // setup + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + + var tableView = new TableView { App = ApplicationImpl.Instance }; + tableView.BeginInit (); + tableView.EndInit (); + tableView.SchemeName = "Runnable"; + tableView.Viewport = new (0, 0, 25, 5); + tableView.Style.ShowHorizontalHeaderUnderline = true; + tableView.Style.ShowHorizontalHeaderOverline = false; + tableView.Style.AlwaysShowHeaders = true; + + var dt = new DataTable (); + dt.Columns.Add ("A"); + dt.Columns.Add ("B"); + dt.Rows.Add (family, "ok"); + tableView.Table = new DataTableSource (dt); + + // execute + tableView.Layout (); + tableView.SetClipToScreen (); + tableView.Draw (); + + // verify β€” the emoji column "A" should be 2 cells wide (grapheme width), not 8 (rune sum). + // Column "A" data is πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ (2 cells), header "A" is 1 cell β†’ column sizes to 2. + // Column "B" data is "ok" (2 cells), header "B" is 1 cell β†’ column sizes to 2. + // If the bug were present, column "A" would be 8 cells wide. + string actual = Application.Driver!.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + + // The first line is the header separator. Find the header row. + // Header row contains "A" and "B" separated by β”‚ + string headerRow = lines.First (l => l.Contains ('A') && l.Contains ('B')); + + // With the fix, "A" column is narrow (header "A" + padding = ~2 cells) + // Without the fix, "A" column would be ~8 cells wide + // Note: IndexOf gives string index, so measure display columns to the separator. + int separatorIndex = headerRow.IndexOf ('β”‚', 1); // skip first border char + int separatorColumn = headerRow [..separatorIndex].GetColumns (); + + Assert.True ( + separatorColumn <= 5, + $"Column A should be narrow (grapheme width 2), but separator at column {separatorColumn} suggests over-sized column. Header: '{headerRow}'" + ); } [Fact] diff --git a/Tests/UnitTests/Views/TreeViewTests.cs b/Tests/UnitTests/Views/TreeViewTests.cs index 998e42cfaf..edbae0ed79 100644 --- a/Tests/UnitTests/Views/TreeViewTests.cs +++ b/Tests/UnitTests/Views/TreeViewTests.cs @@ -933,4 +933,43 @@ private TreeView CreateTree (out Factory factory1, out Car car1, out Car } #endregion + + /// + /// Verifies that measures branch text width using grapheme-aware + /// string.GetColumns() rather than string.Length. + /// Wide CJK characters occupy 2 terminal cells each but have string.Length of 1, + /// so .Length under-counts the display width while .GetColumns() is correct. + /// + [Fact] + [SetupFakeApplication] + public void TestTreeView_GetWidth_GraphemeCluster () + { + // setup β€” CJK characters: each is 2 terminal cells wide but Length of 1 + string cjkText = "\u4F60\u597D"; // δ½ ε₯½ + Assert.Equal (2, cjkText.Length); // char count + Assert.Equal (4, cjkText.GetColumns ()); // display width β€” correct + + var tv = new TreeView { Driver = ApplicationImpl.Instance.Driver, Width = 20, Height = 5 }; + + var node = new TreeNode (cjkText); + tv.AddObject (node); + tv.SetScheme (new Scheme ()); + tv.Style.ShowBranchLines = false; + + // execute + tv.LayoutSubViews (); + tv.SetClipToScreen (); + tv.Draw (); + + // verify β€” the tree should render the CJK text correctly. + // With the fix (GetColumns), the branch width is 4 cells. + // With the old code (string.Length), the branch would be measured at 2 cells, + // potentially causing incorrect scroll/truncation behavior. + string actual = Application.Driver!.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + string firstLine = lines [0]; + + // The CJK text should appear in the rendered output + Assert.Contains (cjkText, firstLine); + } } diff --git a/Tests/UnitTestsParallelizable/Text/StringTests.cs b/Tests/UnitTestsParallelizable/Text/StringTests.cs index 51525992c2..656fa7ae53 100644 --- a/Tests/UnitTestsParallelizable/Text/StringTests.cs +++ b/Tests/UnitTestsParallelizable/Text/StringTests.cs @@ -85,6 +85,33 @@ public void TestGetColumns_MultiRune_WideBMP_Graphemes (string str, int expected Assert.Equal (expectedWidth, str.GetColumns ()); } + /// + /// Documents that EnumerateRunes().Sum(GetColumns) produces inflated widths for multi-rune grapheme + /// clusters (e.g., ZWJ emoji), while string.GetColumns() correctly clamps each cluster to max 2 columns. + /// This is the core issue fixed by replacing rune-sum width measurement with grapheme-aware + /// string.GetColumns() in widget code (TableView, TabView, TreeView, FileDialog, ColorBar, + /// AppendAutocomplete). + /// + [Theory] + [InlineData ("\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466", 8, 2)] // ZWJ family πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦: 4 Γ— width-2 emoji + 3 Γ— ZWJ(0) = 8 vs grapheme = 2 + [InlineData ("\U0001F469\u200D\U0001F4BB", 4, 2)] // Woman technologist πŸ‘©β€πŸ’»: 2 Γ— width-2 emoji + ZWJ(0) = 4 vs grapheme = 2 + [InlineData ("\U0001F468\u200D\U0001F469\u200D\U0001F467\u200D\U0001F466", 8, 2)] // ZWJ family variant πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦ + [InlineData ("\U0001F642", 2, 2)] // Single emoji πŸ™‚: rune width = grapheme width (no difference) + [InlineData ("abc", 3, 3)] // ASCII: rune width = grapheme width (no difference) + [InlineData ("a\u0301", 1, 1)] // a + combining acute: both give 1 (combining mark is width 0) + [InlineData ("\u5C71", 2, 2)] // CJK ε±±: both give 2 (single wide rune) + public void TestGetColumns_RuneSum_Inflated_For_GraphemeClusters (string str, int expectedRuneSum, int expectedGetColumns) + { + // setup + int runeSum = str.EnumerateRunes ().Sum (r => r.GetColumns ()); + int graphemeWidth = str.GetColumns (); + + // verify β€” shows the discrepancy that caused bugs in widget width calculations + Assert.Equal (expectedRuneSum, runeSum); + Assert.Equal (expectedGetColumns, graphemeWidth); + Assert.True (graphemeWidth <= runeSum, "GetColumns should never exceed rune sum"); + } + [Theory] [InlineData (null)] [InlineData ("")] From 3f07c4f6a9b5109f1eebaf17c262e396688d513f Mon Sep 17 00:00:00 2001 From: Adam Krantz Date: Fri, 6 Mar 2026 07:21:44 -0800 Subject: [PATCH 2/2] Move tests to UnitTestsParallelizable Move grapheme width tests from UnitTests to UnitTestsParallelizable per review feedback. Adapt tests to use CreateTestDriver() pattern instead of [SetupFakeApplication]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tests/UnitTests/Views/TabViewTests.cs | 55 ------------- Tests/UnitTests/Views/TableViewTests.cs | 80 ------------------- Tests/UnitTests/Views/TreeViewTests.cs | 39 --------- .../Views/TabViewTests.cs | 56 +++++++++++++ .../Views/TableViewTests.cs | 55 ++++++++++++- .../Views/TreeViewTests.cs | 36 ++++++++- 6 files changed, 145 insertions(+), 176 deletions(-) create mode 100644 Tests/UnitTestsParallelizable/Views/TabViewTests.cs diff --git a/Tests/UnitTests/Views/TabViewTests.cs b/Tests/UnitTests/Views/TabViewTests.cs index 8ffaddb9d7..80cbbfad8e 100644 --- a/Tests/UnitTests/Views/TabViewTests.cs +++ b/Tests/UnitTests/Views/TabViewTests.cs @@ -1061,61 +1061,6 @@ public void ShowTopLine_True_TabsOnBottom_False_With_Unicode () ); } - /// - /// Verifies that measures tab text width using grapheme-aware - /// string.GetColumns() rather than EnumerateRunes().Sum(GetColumns). - /// A ZWJ family emoji should occupy 2 cells as a tab name, not 8. - /// - [Fact] - [SetupFakeApplication] - public void ShowTopLine_True_TabTextWidth_GraphemeCluster () - { - // setup - var tv = new TabView () - { - Driver = ApplicationImpl.Instance.Driver, - Id = "tv" - }; - tv.BeginInit (); - tv.EndInit (); - - string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ - - tv.AddTab ( - new () { Id = "emojiTab", DisplayText = family, View = new TextField { Id = "tf", Width = 4, Text = "hi" } }, - false - ); - tv.AddTab (new () { Id = "tab2", DisplayText = "B", View = new Label { Id = "lbl", Text = "hi2" } }, false); - tv.Width = 20; - tv.Height = 5; - - // execute - tv.Layout (); - tv.SetClipToScreen (); - tv.Draw (); - - // verify β€” the emoji tab header should be ~2 cells wide (grapheme width), - // not ~8 cells wide (rune sum width) - string actual = Application.Driver!.ToString ()!; - string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); - - // Find the tab header row (contains the emoji and "B") - string? headerRow = lines.FirstOrDefault (l => l.Contains ('B') && l.Length > 1); - Assert.NotNull (headerRow); - - // The emoji tab should be narrow. With the fix, the tab text is 2 cells. - // Without the fix, the tab text would be 8 cells, pushing "B" much further right. - // Note: IndexOf gives string index (includes multi-byte emoji chars), - // so we measure display columns of the substring before 'B'. - int bIndex = headerRow.IndexOf ('B'); - int bColumnPosition = headerRow [..bIndex].GetColumns (); - - Assert.True ( - bColumnPosition <= 8, - $"Tab 'B' should be near the start (emoji tab is 2 cells wide), but found at column {bColumnPosition}. Row: '{headerRow}'" - ); - } - [Fact] [SetupFakeApplication] public void ShowTopLine_True_TabsOnBottom_True_TestTabView_Width3 () diff --git a/Tests/UnitTests/Views/TableViewTests.cs b/Tests/UnitTests/Views/TableViewTests.cs index 3e0200bf97..8eba087c62 100644 --- a/Tests/UnitTests/Views/TableViewTests.cs +++ b/Tests/UnitTests/Views/TableViewTests.cs @@ -1804,86 +1804,6 @@ public void Test_SumColumnWidth_UnicodeLength () Assert.Equal (14, surrogate.GetColumns ()); } - /// - /// Demonstrates that string.GetColumns() should be used instead of - /// EnumerateRunes().Sum(GetColumns) for display width measurement, because the rune-sum approach - /// produces inflated widths for grapheme clusters like ZWJ emoji sequences. - /// This is relevant to and TruncateOrPad. - /// - [Fact] - public void Test_SumColumnWidth_GraphemeClusters () - { - // ZWJ family emoji: 4 person emoji (width 2 each) + 3 ZWJ chars (width 0) - string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ - - // EnumerateRunes().Sum() gives 8 β€” WRONG for display width - Assert.Equal (8, family.EnumerateRunes ().Sum (c => c.GetColumns ())); - - // string.GetColumns() correctly treats the ZWJ sequence as one grapheme cluster (width 2) - Assert.Equal (2, family.GetColumns ()); - - // Woman technologist: 2 emoji (width 2 each) + ZWJ - string technologist = "\U0001F469\u200D\U0001F4BB"; // πŸ‘©β€πŸ’» - Assert.Equal (4, technologist.EnumerateRunes ().Sum (c => c.GetColumns ())); - Assert.Equal (2, technologist.GetColumns ()); - } - - /// - /// Verifies that auto-sizes columns based on grapheme-aware - /// string.GetColumns(), not EnumerateRunes().Sum(GetColumns). - /// With the buggy rune-sum, a ZWJ family emoji (width 8 by rune sum) would cause - /// the column to be much wider than needed. With the fix, the column sizes to 2 cells. - /// - [Fact] - [SetupFakeApplication] - public void Test_CalculateMaxCellWidth_UsesGraphemeWidth () - { - // setup - string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ - - var tableView = new TableView { App = ApplicationImpl.Instance }; - tableView.BeginInit (); - tableView.EndInit (); - tableView.SchemeName = "Runnable"; - tableView.Viewport = new (0, 0, 25, 5); - tableView.Style.ShowHorizontalHeaderUnderline = true; - tableView.Style.ShowHorizontalHeaderOverline = false; - tableView.Style.AlwaysShowHeaders = true; - - var dt = new DataTable (); - dt.Columns.Add ("A"); - dt.Columns.Add ("B"); - dt.Rows.Add (family, "ok"); - tableView.Table = new DataTableSource (dt); - - // execute - tableView.Layout (); - tableView.SetClipToScreen (); - tableView.Draw (); - - // verify β€” the emoji column "A" should be 2 cells wide (grapheme width), not 8 (rune sum). - // Column "A" data is πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ (2 cells), header "A" is 1 cell β†’ column sizes to 2. - // Column "B" data is "ok" (2 cells), header "B" is 1 cell β†’ column sizes to 2. - // If the bug were present, column "A" would be 8 cells wide. - string actual = Application.Driver!.ToString ()!; - string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); - - // The first line is the header separator. Find the header row. - // Header row contains "A" and "B" separated by β”‚ - string headerRow = lines.First (l => l.Contains ('A') && l.Contains ('B')); - - // With the fix, "A" column is narrow (header "A" + padding = ~2 cells) - // Without the fix, "A" column would be ~8 cells wide - // Note: IndexOf gives string index, so measure display columns to the separator. - int separatorIndex = headerRow.IndexOf ('β”‚', 1); // skip first border char - int separatorColumn = headerRow [..separatorIndex].GetColumns (); - - Assert.True ( - separatorColumn <= 5, - $"Column A should be narrow (grapheme width 2), but separator at column {separatorColumn} suggests over-sized column. Header: '{headerRow}'" - ); - } - [Fact] [SetupFakeApplication] public void TestColumnStyle_AllColumnsVisibleFalse_BehavesAsTableNull () diff --git a/Tests/UnitTests/Views/TreeViewTests.cs b/Tests/UnitTests/Views/TreeViewTests.cs index edbae0ed79..998e42cfaf 100644 --- a/Tests/UnitTests/Views/TreeViewTests.cs +++ b/Tests/UnitTests/Views/TreeViewTests.cs @@ -933,43 +933,4 @@ private TreeView CreateTree (out Factory factory1, out Car car1, out Car } #endregion - - /// - /// Verifies that measures branch text width using grapheme-aware - /// string.GetColumns() rather than string.Length. - /// Wide CJK characters occupy 2 terminal cells each but have string.Length of 1, - /// so .Length under-counts the display width while .GetColumns() is correct. - /// - [Fact] - [SetupFakeApplication] - public void TestTreeView_GetWidth_GraphemeCluster () - { - // setup β€” CJK characters: each is 2 terminal cells wide but Length of 1 - string cjkText = "\u4F60\u597D"; // δ½ ε₯½ - Assert.Equal (2, cjkText.Length); // char count - Assert.Equal (4, cjkText.GetColumns ()); // display width β€” correct - - var tv = new TreeView { Driver = ApplicationImpl.Instance.Driver, Width = 20, Height = 5 }; - - var node = new TreeNode (cjkText); - tv.AddObject (node); - tv.SetScheme (new Scheme ()); - tv.Style.ShowBranchLines = false; - - // execute - tv.LayoutSubViews (); - tv.SetClipToScreen (); - tv.Draw (); - - // verify β€” the tree should render the CJK text correctly. - // With the fix (GetColumns), the branch width is 4 cells. - // With the old code (string.Length), the branch would be measured at 2 cells, - // potentially causing incorrect scroll/truncation behavior. - string actual = Application.Driver!.ToString ()!; - string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); - string firstLine = lines [0]; - - // The CJK text should appear in the rendered output - Assert.Contains (cjkText, firstLine); - } } diff --git a/Tests/UnitTestsParallelizable/Views/TabViewTests.cs b/Tests/UnitTestsParallelizable/Views/TabViewTests.cs new file mode 100644 index 0000000000..56a547db8a --- /dev/null +++ b/Tests/UnitTestsParallelizable/Views/TabViewTests.cs @@ -0,0 +1,56 @@ +ο»Ώusing JetBrains.Annotations; +using UnitTests; + +namespace ViewsTests; + +[TestSubject (typeof (TabView))] +public class TabViewTests : TestDriverBase +{ + /// + /// Verifies that measures tab text width using grapheme-aware + /// string.GetColumns() rather than EnumerateRunes().Sum(GetColumns). + /// A ZWJ family emoji should occupy 2 cells as a tab name, not 8. + /// + [Fact] + public void ShowTopLine_True_TabTextWidth_GraphemeCluster () + { + // setup + IDriver driver = CreateTestDriver (); + var tv = new TabView () + { + Driver = driver, + Id = "tv" + }; + tv.BeginInit (); + tv.EndInit (); + + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + + tv.AddTab ( + new () { Id = "emojiTab", DisplayText = family, View = new TextField { Id = "tf", Width = 4, Text = "hi" } }, + false + ); + tv.AddTab (new () { Id = "tab2", DisplayText = "B", View = new Label { Id = "lbl", Text = "hi2" } }, false); + tv.Width = 20; + tv.Height = 5; + + // execute + tv.Layout (); + tv.SetClipToScreen (); + tv.Draw (); + + // verify + string actual = driver.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + string? headerRow = lines.FirstOrDefault (l => l.Contains ('B') && l.Length > 1); + Assert.NotNull (headerRow); + + int bIndex = headerRow.IndexOf ('B'); + int bColumnPosition = headerRow [..bIndex].GetColumns (); + + Assert.True ( + bColumnPosition <= 8, + $"Tab 'B' should be near the start (emoji tab is 2 cells wide), but found at column {bColumnPosition}. Row: '{headerRow}'" + ); + } +} diff --git a/Tests/UnitTestsParallelizable/Views/TableViewTests.cs b/Tests/UnitTestsParallelizable/Views/TableViewTests.cs index f313910c0e..b3b5f2c7d5 100644 --- a/Tests/UnitTestsParallelizable/Views/TableViewTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TableViewTests.cs @@ -1,10 +1,11 @@ ο»Ώusing System.Data; using JetBrains.Annotations; +using UnitTests; namespace ViewsTests; [TestSubject (typeof (TableView))] -public class TableViewTests +public class TableViewTests : TestDriverBase { [Fact] public void CanTabOutOfTableViewUsingCursor_Left () @@ -322,4 +323,56 @@ public void TableView_Enter_FiresCellActivated () tableView.Dispose (); } + + [Fact] + public void Test_SumColumnWidth_GraphemeClusters () + { + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + Assert.Equal (8, family.EnumerateRunes ().Sum (c => c.GetColumns ())); + Assert.Equal (2, family.GetColumns ()); + + string technologist = "\U0001F469\u200D\U0001F4BB"; // πŸ‘©β€πŸ’» + Assert.Equal (4, technologist.EnumerateRunes ().Sum (c => c.GetColumns ())); + Assert.Equal (2, technologist.GetColumns ()); + } + + [Fact] + public void Test_CalculateMaxCellWidth_UsesGraphemeWidth () + { + // setup + IDriver driver = CreateTestDriver (); + string family = "\U0001F468\u200D\U0001F469\u200D\U0001F466\u200D\U0001F466"; // πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦ + + var tableView = new TableView { Driver = driver }; + tableView.BeginInit (); + tableView.EndInit (); + tableView.SchemeName = "Runnable"; + tableView.Viewport = new (0, 0, 25, 5); + tableView.Style.ShowHorizontalHeaderUnderline = true; + tableView.Style.ShowHorizontalHeaderOverline = false; + tableView.Style.AlwaysShowHeaders = true; + + var dt = new DataTable (); + dt.Columns.Add ("A"); + dt.Columns.Add ("B"); + dt.Rows.Add (family, "ok"); + tableView.Table = new DataTableSource (dt); + + // execute + tableView.Layout (); + tableView.SetClipToScreen (); + tableView.Draw (); + + // verify + string actual = driver.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + string headerRow = lines.First (l => l.Contains ('A') && l.Contains ('B')); + int separatorIndex = headerRow.IndexOf ('β”‚', 1); + int separatorColumn = headerRow [..separatorIndex].GetColumns (); + + Assert.True ( + separatorColumn <= 5, + $"Column A should be narrow (grapheme width 2), but separator at column {separatorColumn} suggests over-sized column. Header: '{headerRow}'" + ); + } } diff --git a/Tests/UnitTestsParallelizable/Views/TreeViewTests.cs b/Tests/UnitTestsParallelizable/Views/TreeViewTests.cs index dc7cb2e5f8..c5e7979425 100644 --- a/Tests/UnitTestsParallelizable/Views/TreeViewTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TreeViewTests.cs @@ -1,9 +1,10 @@ ο»Ώusing JetBrains.Annotations; +using UnitTests; namespace ViewsTests; [TestSubject (typeof (TreeView))] -public class TreeViewTests +public class TreeViewTests : TestDriverBase { [Fact] public void CollectionNavigatorMatcher_KeybindingsOverrideNavigator () @@ -906,4 +907,37 @@ private class Car } #endregion + + /// + /// Verifies that measures branch text width using grapheme-aware + /// string.GetColumns() rather than string.Length. + /// Wide CJK characters occupy 2 terminal cells each but have string.Length of 1, + /// so .Length under-counts the display width while .GetColumns() is correct. + /// + [Fact] + public void TestTreeView_GetWidth_GraphemeCluster () + { + // setup + IDriver driver = CreateTestDriver (); + string cjkText = "\u4F60\u597D"; // δ½ ε₯½ + Assert.Equal (2, cjkText.Length); + Assert.Equal (4, cjkText.GetColumns ()); + + var tv = new TreeView { Driver = driver, Width = 20, Height = 5 }; + var node = new TreeNode (cjkText); + tv.AddObject (node); + tv.SetScheme (new Scheme ()); + tv.Style.ShowBranchLines = false; + + // execute + tv.LayoutSubViews (); + tv.SetClipToScreen (); + tv.Draw (); + + // verify + string actual = driver.ToString ()!; + string [] lines = actual.Replace ("\r\n", "\n").Split ('\n'); + string firstLine = lines [0]; + Assert.Contains (cjkText, firstLine); + } }