Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Terminal.Gui/Views/Autocomplete/AppendAutocomplete.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
3 changes: 1 addition & 2 deletions Terminal.Gui/Views/Color/ColorBar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Terminal.Gui/Views/FileDialogs/FileDialog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Terminal.Gui/Views/TabView/TabView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ internal IEnumerable<Tab> 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
Expand Down
8 changes: 4 additions & 4 deletions Terminal.Gui/Views/TableView/TableView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ private void AddRuneAt (int col, int row, Rune ch)
/// <returns></returns>
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 })
Expand All @@ -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
Expand Down Expand Up @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions Terminal.Gui/Views/TreeView/Branch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 ();
}

/// <summary>Refreshes cached knowledge in this branch e.g. what children an object has.</summary>
Expand Down
3 changes: 3 additions & 0 deletions Tests/UnitTests/Views/TableViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1799,6 +1799,9 @@ 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);

Comment thread
tig marked this conversation as resolved.
// For combining marks, EnumerateRunes().Sum() and GetColumns() agree
Assert.Equal (14, surrogate.GetColumns ());
}

[Fact]
Expand Down
27 changes: 27 additions & 0 deletions Tests/UnitTestsParallelizable/Text/StringTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,33 @@ public void TestGetColumns_MultiRune_WideBMP_Graphemes (string str, int expected
Assert.Equal (expectedWidth, str.GetColumns ());
}

/// <summary>
/// Documents that <c>EnumerateRunes().Sum(GetColumns)</c> produces inflated widths for multi-rune grapheme
/// clusters (e.g., ZWJ emoji), while <c>string.GetColumns()</c> correctly clamps each cluster to max 2 columns.
/// This is the core issue fixed by replacing rune-sum width measurement with grapheme-aware
/// <c>string.GetColumns()</c> in widget code (TableView, TabView, TreeView, FileDialog, ColorBar,
/// AppendAutocomplete).
/// </summary>
[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 ("")]
Expand Down
56 changes: 56 additions & 0 deletions Tests/UnitTestsParallelizable/Views/TabViewTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using JetBrains.Annotations;
using UnitTests;

namespace ViewsTests;

[TestSubject (typeof (TabView))]
public class TabViewTests : TestDriverBase
{
/// <summary>
/// Verifies that <see cref="TabView" /> measures tab text width using grapheme-aware
/// <c>string.GetColumns()</c> rather than <c>EnumerateRunes().Sum(GetColumns)</c>.
/// A ZWJ family emoji should occupy 2 cells as a tab name, not 8.
/// </summary>
[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}'"
);
}
}
55 changes: 54 additions & 1 deletion Tests/UnitTestsParallelizable/Views/TableViewTests.cs
Original file line number Diff line number Diff line change
@@ -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 ()
Expand Down Expand Up @@ -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}'"
);
}
}
36 changes: 35 additions & 1 deletion Tests/UnitTestsParallelizable/Views/TreeViewTests.cs
Original file line number Diff line number Diff line change
@@ -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 ()
Expand Down Expand Up @@ -436,7 +437,7 @@
{
var tree = new TreeView ();

Exception exception = Record.Exception (() => tree.GoToEnd ());

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (ubuntu-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Non-Parallel Unit Tests (macos-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Build All Configurations

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Build All Configurations

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (macos-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Non-Parallel Unit Tests (ubuntu-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (windows-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Non-Parallel Unit Tests (windows-latest)

Converting null literal or possible null value to non-nullable type.

Check warning on line 440 in Tests/UnitTestsParallelizable/Views/TreeViewTests.cs

View workflow job for this annotation

GitHub Actions / Parallel Unit Tests (windows-latest)

Converting null literal or possible null value to non-nullable type.

Assert.Null (exception);
}
Expand Down Expand Up @@ -906,4 +907,37 @@
}

#endregion

/// <summary>
/// Verifies that <see cref="TreeView{T}" /> measures branch text width using grapheme-aware
/// <c>string.GetColumns()</c> rather than <c>string.Length</c>.
/// Wide CJK characters occupy 2 terminal cells each but have <c>string.Length</c> of 1,
/// so <c>.Length</c> under-counts the display width while <c>.GetColumns()</c> is correct.
/// </summary>
[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);
}
}
Loading