Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 55 additions & 0 deletions Tests/UnitTests/Views/TabViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,61 @@
);
}

/// <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]
[SetupFakeApplication]
public void ShowTopLine_True_TabTextWidth_GraphemeCluster ()
Comment thread
tig marked this conversation as resolved.
Outdated
{
// 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);

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

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

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

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

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Build All Configurations

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Build All Configurations

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Integration Tests (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

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

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Parallel Unit Tests (ubuntu-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Parallel Unit Tests (macos-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 1103 in Tests/UnitTests/Views/TabViewTests.cs

View workflow job for this annotation

GitHub Actions / Parallel Unit Tests (windows-latest)

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
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 ()
Expand Down
83 changes: 83 additions & 0 deletions Tests/UnitTests/Views/TableViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

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

/// <summary>
/// Demonstrates that <c>string.GetColumns()</c> should be used instead of
/// <c>EnumerateRunes().Sum(GetColumns)</c> for display width measurement, because the rune-sum approach
/// produces inflated widths for grapheme clusters like ZWJ emoji sequences.
/// This is relevant to <see cref="TableView.CalculateMaxCellWidth" /> and <c>TruncateOrPad</c>.
/// </summary>
[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 ());
}

/// <summary>
/// Verifies that <see cref="TableView" /> auto-sizes columns based on grapheme-aware
/// <c>string.GetColumns()</c>, not <c>EnumerateRunes().Sum(GetColumns)</c>.
/// 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.
/// </summary>
[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]
Expand Down
39 changes: 39 additions & 0 deletions Tests/UnitTests/Views/TreeViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -933,4 +933,43 @@ private TreeView<object> CreateTree (out Factory factory1, out Car car1, out Car
}

#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]
[SetupFakeApplication]
public void TestTreeView_GetWidth_GraphemeCluster ()
Comment thread
tig marked this conversation as resolved.
Outdated
{
// 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);
}
}
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
Loading