Skip to content

Commit

Permalink
Fixes gui-cs#2616. Support combining sequences that don't normalize.
Browse files Browse the repository at this point in the history
  • Loading branch information
@BDisp
BDisp committed Dec 3, 2024
1 parent d4d0675 commit dbbe3f7
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 69 deletions.
24 changes: 19 additions & 5 deletions Terminal.Gui/Application/Application.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,25 @@ public static string ToString (IConsoleDriver? driver)
{
sb.Append (sp);
}
else if (contents [r, c].CombiningMarks is { Count: > 0 })
{
// AtlasEngine does not support NON-NORMALIZED combining marks in a way
// compatible with the driver architecture. Any CMs (except in the first col)
// are correctly combined with the base char, but are ALSO treated as 1 column
// width codepoints E.g. `echo "[e`u{0301}`u{0301}]"` will output `[é ]`.
//
// For now, we just ignore the list of CMs.
string combine = rune.ToString ();
string? normalized = null;

foreach (Rune combMark in contents [r, c].CombiningMarks)
{
combine += combMark;
normalized = combine.Normalize (NormalizationForm.FormC);
}

sb.Append (normalized);
}
else
{
sb.Append ((char)rune.Value);
Expand All @@ -76,11 +95,6 @@ public static string ToString (IConsoleDriver? driver)
{
c++;
}

// See Issue #2616
//foreach (var combMark in contents [r, c].CombiningMarks) {
// sb.Append ((char)combMark.Value);
//}
}

sb.AppendLine ();
Expand Down
38 changes: 11 additions & 27 deletions Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,35 +208,19 @@ public void AddRune (Rune rune)
// b) Ignoring any CMs that don't normalize
if (Col > 0)
{
if (Contents [Row, Col - 1].CombiningMarks.Count > 0)
for (int i = Col; i > 0; i--)
{
// Just add this mark to the list
Contents [Row, Col - 1].CombiningMarks.Add (rune);

// Ignore. Don't move to next column (let the driver figure out what to do).
}
else
{
// Attempt to normalize the cell to our left combined with this mark
string combined = Contents [Row, Col - 1].Rune + rune.ToString ();

// Normalize to Form C (Canonical Composition)
string normalized = combined.Normalize (NormalizationForm.FormC);

if (normalized.Length == 1)
if (!Contents [Row, i - 1].Rune.IsCombiningMark ())
{
// It normalized! We can just set the Cell to the left with the
// normalized codepoint
Contents [Row, Col - 1].Rune = (Rune)normalized [0];

// Ignore. Don't move to next column because we're already there
}
else
{
// It didn't normalize. Add it to the Cell to left's CM list
Contents [Row, Col - 1].CombiningMarks.Add (rune);

// Ignore. Don't move to next column (let the driver figure out what to do).
if (Contents [Row, i - 1].CombiningMarks is null)
{
Contents [Row, i - 1].CombiningMarks = [];
}
// Just add this mark to the list
Contents [Row, i - 1].CombiningMarks.Add (rune);
Debug.Assert (Contents [Row, i - 1].CombiningMarks.Count > 0);

break;
}
}

Expand Down
11 changes: 6 additions & 5 deletions Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,18 +319,19 @@ public override bool UpdateScreen ()
Rune rune = Contents [row, col].Rune;
output.Append (rune);

if (Contents [row, col].CombiningMarks.Count > 0)
if (Contents [row, col].CombiningMarks is { Count: > 0 })
{
// AtlasEngine does not support NON-NORMALIZED combining marks in a way
// compatible with the driver architecture. Any CMs (except in the first col)
// are correctly combined with the base char, but are ALSO treated as 1 column
// width codepoints E.g. `echo "[e`u{0301}`u{0301}]"` will output `[é ]`.
//
// For now, we just ignore the list of CMs.
//foreach (var combMark in Contents [row, col].CombiningMarks) {
// output.Append (combMark);
//}
// WriteToConsole (output, ref lastCol, row, ref outputWidth);
foreach (var combMark in Contents [row, col].CombiningMarks)
{
output.Append (combMark);
}
WriteToConsole (output, ref lastCol, row, ref outputWidth);
}
else if (rune.IsSurrogatePair () && rune.GetColumns () < 2)
{
Expand Down
11 changes: 6 additions & 5 deletions Terminal.Gui/ConsoleDrivers/NetDriver/NetDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,18 +167,19 @@ public override bool UpdateScreen ()
Rune rune = Contents [row, col].Rune;
output.Append (rune);

if (Contents [row, col].CombiningMarks.Count > 0)
if (Contents [row, col].CombiningMarks is { Count: > 0 })
{
// AtlasEngine does not support NON-NORMALIZED combining marks in a way
// compatible with the driver architecture. Any CMs (except in the first col)
// are correctly combined with the base char, but are ALSO treated as 1 column
// width codepoints E.g. `echo "[e`u{0301}`u{0301}]"` will output `[é ]`.
//
// For now, we just ignore the list of CMs.
//foreach (var combMark in Contents [row, col].CombiningMarks) {
// output.Append (combMark);
//}
// WriteToConsole (output, ref lastCol, row, ref outputWidth);
foreach (var combMark in Contents [row, col].CombiningMarks)
{
output.Append (combMark);
}
WriteToConsole (output, ref lastCol, row, ref outputWidth);
}
else if (rune.IsSurrogatePair () && rune.GetColumns () < 2)
{
Expand Down
9 changes: 9 additions & 0 deletions Terminal.Gui/ConsoleDrivers/WindowsDriver/WindowsConsole.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ public bool WriteToConsole (Size size, ExtendedCharInfo [] charInfoBuffer, Coord
{
_stringBuilder.Append (info.Char);
}

if (info.CombiningMarks is { })
{
foreach (var combMark in info.CombiningMarks)
{
_stringBuilder.Append (combMark);
}
}
}
else
{
Expand Down Expand Up @@ -785,6 +793,7 @@ public struct ExtendedCharInfo
public char Char { get; set; }
public Attribute Attribute { get; set; }
public bool Empty { get; set; } // TODO: Temp hack until virtual terminal sequences
internal List<char>? CombiningMarks;

public ExtendedCharInfo (char character, Attribute attribute)
{
Expand Down
14 changes: 14 additions & 0 deletions Terminal.Gui/ConsoleDrivers/WindowsDriver/WindowsDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,20 @@ public override bool UpdateScreen ()

_outputBuffer [position].Empty = false;

if (Contents [row, col].CombiningMarks is { Count: > 0 })
{
_outputBuffer [position].CombiningMarks = [];

foreach (var combMark in Contents [row, col].CombiningMarks)
{
_outputBuffer [position].CombiningMarks!.Add ((char)combMark.Value);
}
}
else
{
_outputBuffer [position].CombiningMarks = null;
}

if (Contents [row, col].Rune.IsBmp)
{
_outputBuffer [position].Char = (char)Contents [row, col].Rune.Value;
Expand Down
12 changes: 3 additions & 9 deletions Terminal.Gui/Drawing/Cell.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/// Represents a single row/column in a Terminal.Gui rendering surface (e.g. <see cref="LineCanvas"/> and
/// <see cref="IConsoleDriver"/>).
/// </summary>
public record struct Cell (Attribute? Attribute = null, bool IsDirty = false, Rune Rune = default)
public record struct Cell (Attribute? Attribute = null, bool IsDirty = false, Rune Rune = default, List<Rune> CombiningMarks = null)
{
/// <summary>The attributes to use when drawing the Glyph.</summary>
public Attribute? Attribute { get; set; } = Attribute;
Expand All @@ -23,13 +23,11 @@ public Rune Rune
get => _rune;
set
{
CombiningMarks.Clear ();
CombiningMarks = null;
_rune = value;
}
}

private List<Rune> _combiningMarks;

/// <summary>
/// The combining marks for <see cref="Rune"/> that when combined makes this Cell a combining sequence. If
/// <see cref="CombiningMarks"/> empty, then <see cref="CombiningMarks"/> is ignored.
Expand All @@ -38,11 +36,7 @@ public Rune Rune
/// Only valid in the rare case where <see cref="Rune"/> is a combining sequence that could not be normalized to a
/// single Rune.
/// </remarks>
internal List<Rune> CombiningMarks
{
get => _combiningMarks ?? [];
private set => _combiningMarks = value ?? [];
}
internal List<Rune> CombiningMarks { get; set; } = CombiningMarks;

/// <inheritdoc/>
public override string ToString () { return $"[{Rune}, {Attribute}]"; }
Expand Down
18 changes: 15 additions & 3 deletions UICatalog/Scenarios/CombiningMarks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,29 @@ public override void Main ()
top.Move (0, 0);
top.AddStr ("Terminal.Gui only supports combining marks that normalize. See Issue #2616.");
top.Move (0, 2);
top.AddStr ("\u0301\u0301\u0328<- \"\\u301\\u301\\u328]\" using AddStr.");
top.AddStr ("\u0301\u0301\u0328<- \"\\u0301\\u0301\\u0328]\" using AddStr.");
top.Move (0, 3);
top.AddStr ("[a\u0301\u0301\u0328]<- \"[a\\u301\\u301\\u328]\" using AddStr.");
top.AddStr ("[a\u0301\u0301\u0328]<- \"[a\\u0301\\u0301\\u0328]\" using AddStr.");
top.Move (0, 4);
top.AddRune ('[');
top.AddRune ('a');
top.AddRune ('\u0301');
top.AddRune ('\u0301');
top.AddRune ('\u0328');
top.AddRune (']');
top.AddStr ("<- \"[a\\u301\\u301\\u328]\" using AddRune for each.");
top.AddStr ("<- \"[a\\u0301\\u0301\\u0328]\" using AddRune for each.");
top.Move (0, 6);
top.AddStr ("[e\u0301\u0301\u0328]<- \"[e\\u0301\\u0301\\u0328]\" using AddStr.");
top.Move (0, 7);
top.AddStr ("[e\u0301\u0328\u0301]<- \"[e\\u0301\\u0328\\u0301]\" using AddStr.");
top.Move (0, 8);
top.AddStr ("[e\u0328\u0301]<- \"[e\\u0328\\u0301]\" using AddStr.");
top.Move (0, 10);
top.AddStr ("From now on we are using Text Formatter");
TextFormatter tf = new () { Text = "[e\u0301\u0301\u0328]<- \"[e\\u0301\\u0301\\u0328]\" using TextFormatter." };
tf.Draw (new (0, 11, tf.Text.Length, 1), top.ColorScheme.Normal, top.ColorScheme.Normal);
tf.Text = "[e\u0328\u0301]<- \"[e\\u0328\\u0301]\" using TextFormatter.";
tf.Draw (new (0, 12, tf.Text.Length, 1), top.ColorScheme.Normal, top.ColorScheme.Normal);
};

Application.Run (top);
Expand Down
8 changes: 6 additions & 2 deletions UnitTests/ConsoleDrivers/AddRuneTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,19 @@ public void AddRune_Accented_Letter_With_Three_Combining_Unicode_Chars ()

text = "\u0103\u0301";
driver.AddStr (text);
Assert.Equal (expected, driver.Contents [0, 0].Rune);
Assert.Single (driver.Contents [0, 0].CombiningMarks);
string combined = driver.Contents [0, 0].Rune + driver.Contents [0, 0].CombiningMarks [0].ToString ();
Assert.Equal (expected, (Rune)combined.Normalize (NormalizationForm.FormC) [0]);
Assert.Equal ((Rune)' ', driver.Contents [0, 1].Rune);

driver.ClearContents ();
driver.Move (0, 0);

text = "\u0061\u0306\u0301";
driver.AddStr (text);
Assert.Equal (expected, driver.Contents [0, 0].Rune);
Assert.Equal (2, driver.Contents [0, 0].CombiningMarks.Count);
combined = driver.Contents [0, 0].Rune + driver.Contents [0, 0].CombiningMarks [0].ToString () + driver.Contents [0, 0].CombiningMarks [1];
Assert.Equal (expected, (Rune)combined.Normalize (NormalizationForm.FormC) [0]);
Assert.Equal ((Rune)' ', driver.Contents [0, 1].Rune);

// var s = "a\u0301\u0300\u0306";
Expand Down
8 changes: 4 additions & 4 deletions UnitTests/ConsoleDrivers/ContentsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,31 +56,31 @@ public void AddStr_With_Combining_Characters (Type driverType)
// a + ogonek + acute = <U+0061, U+0328, U+0301> ( ą́ )
var ogonek = new Rune (0x0328); // Combining ogonek (a small hook or comma shape)
combined = "a" + ogonek + acuteaccent;
expected = ("a" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
expected = ("á" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616

driver.Move (0, 0);
driver.AddStr (combined);
TestHelpers.AssertDriverContentsAre (expected, output, driver);

// e + ogonek + acute = <U+0061, U+0328, U+0301> ( ę́́ )
combined = "e" + ogonek + acuteaccent;
expected = ("e" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
expected = ("é" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616

driver.Move (0, 0);
driver.AddStr (combined);
TestHelpers.AssertDriverContentsAre (expected, output, driver);

// i + ogonek + acute = <U+0061, U+0328, U+0301> ( į́́́ )
combined = "i" + ogonek + acuteaccent;
expected = ("i" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
expected = ("í" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616

driver.Move (0, 0);
driver.AddStr (combined);
TestHelpers.AssertDriverContentsAre (expected, output, driver);

// u + ogonek + acute = <U+0061, U+0328, U+0301> ( ų́́́́ )
combined = "u" + ogonek + acuteaccent;
expected = ("u" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
expected = ("ú" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616

driver.Move (0, 0);
driver.AddStr (combined);
Expand Down
25 changes: 19 additions & 6 deletions UnitTests/TestHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,23 +411,36 @@ public static Rectangle AssertDriverContentsWithFrameAre (
colIndex++;
}

if (colIndex + 1 > w)
if (colIndex + 1 > w && !runeAtCurrentLocation.IsCombiningMark ())
{
w = colIndex + 1;
}

h = rowIndex - y + 1;
}

if (x > -1)
if (x > -1 && contents [rowIndex, colIndex].CombiningMarks is null)
{
runes.Add (runeAtCurrentLocation);
}

// See Issue #2616
//foreach (var combMark in contents [r, c].CombiningMarks) {
// runes.Add (combMark);
//}
if (contents [rowIndex, colIndex].CombiningMarks is { Count: > 0 })
{
string combine = runeAtCurrentLocation.ToString ();
string? normalized = null;

// See Issue #2616
foreach (var combMark in contents [rowIndex, colIndex].CombiningMarks)
{
combine += combMark;
normalized = combine.Normalize (NormalizationForm.FormC);
}

foreach (Rune enumerateRune in normalized!.EnumerateRunes ())
{
runes.Add (enumerateRune);
}
}
}

if (runes.Count > 0)
Expand Down
6 changes: 3 additions & 3 deletions UnitTests/Text/TextFormatterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4114,8 +4114,8 @@ public void Draw_Vertical_TopBottom_LeftRight_Top (string text, int height, stri
}

[Theory]
[InlineData (14, 1, TextDirection.LeftRight_TopBottom, "Les Misęrables")]
[InlineData (1, 14, TextDirection.TopBottom_LeftRight, "L\ne\ns\n \nM\ni\ns\nę\nr\na\nb\nl\ne\ns")]
[InlineData (14, 1, TextDirection.LeftRight_TopBottom, "Les Misę́rables")]
[InlineData (1, 14, TextDirection.TopBottom_LeftRight, "L\ne\ns\n \nM\ni\ns\nę́\nr\na\nb\nl\ne\ns")]
[InlineData (
4,
4,
Expand All @@ -4124,7 +4124,7 @@ public void Draw_Vertical_TopBottom_LeftRight_Top (string text, int height, stri
LMre
eias
ssb
ęl "
ę́l "
)]
public void Draw_With_Combining_Runes (int width, int height, TextDirection textDirection, string expected)
{
Expand Down

0 comments on commit dbbe3f7

Please sign in to comment.