Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Partially Addresses #2616. Support combining sequences that don't normalize #2932

Merged
merged 21 commits into from
Oct 29, 2023
Merged

Partially Addresses #2616. Support combining sequences that don't normalize #2932

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0436019
Fixes #2616. Support combining sequences that don't normalize
BDisp Oct 24, 2023
f1a8308
Merge branch 'v2_develop' into v2_combining-normalize-fix_2616
tig Oct 25, 2023
475a89e
Decouples Application from ConsoleDriver in TestHelpers
tig Oct 25, 2023
860ceb1
Updates driver tests to match new arch
tig Oct 25, 2023
5e3edce
Start on making all driver tests test all drivers
tig Oct 25, 2023
3867df4
Merge pull request #161 from tig/BDisp-v2_combining-normalize-fix_2616
BDisp Oct 25, 2023
fc26a31
Improves handling if combining marks.
BDisp Oct 25, 2023
b491845
Fix unit tests fails.
BDisp Oct 25, 2023
f555243
Fix unit tests fails.
BDisp Oct 25, 2023
917a84f
Fix merge conflicts.
BDisp Oct 25, 2023
6f99b09
Handling combining mask.
BDisp Oct 25, 2023
cda0f01
Tying to fix this unit test that sometimes fail.
BDisp Oct 25, 2023
0293f6a
Add support for combining mask on NetDriver.
BDisp Oct 25, 2023
41cd364
Enable CombiningMarks as List<Rune>.
BDisp Oct 26, 2023
877190b
Prevents combining marks on invalid runes default and space.
BDisp Oct 26, 2023
abcc5f9
Merge branch 'v2_develop' into v2_combining-normalize-fix_2616
tig Oct 27, 2023
0e9b036
Formatting for CI tests.
BDisp Oct 27, 2023
621c68a
Fix non-normalized combining mark to add 1 to Col.
BDisp Oct 27, 2023
9a9e970
Reformatting for retest the CI.
BDisp Oct 27, 2023
6893af5
Forces non-normalized CMs to be ignored.
tig Oct 27, 2023
45e0d0d
Merge pull request #162 from tig/BDisp-v2_combining-normalize-fix_2616
BDisp Oct 27, 2023
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
17 changes: 14 additions & 3 deletions Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,22 @@ public void AddRune (Rune rune)
// Normalize to Form C (Canonical Composition)
string normalized = combined.Normalize (NormalizationForm.FormC);

Contents [Row, Col - 1].Rune = (Rune)normalized [0]; ;
if (Contents [Row, Col - 1].Rune != default && Contents [Row, Col - 1].Rune != (Rune)' ') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not sufficient. Many, many, characters cannot be combined with a combining mark, not just ' '. Try, for example [.

WT (and other platforms) have sophisticated algorithms for figuring out whether a combining mark can combine with a base character. I am trying to find more details on these to see what we can do.

You can easily test this in WT and PS with (Caskadia Cove Nerd Font):

image

In Unicode, not all base characters can be combined with combining characters. Whether a base character can be combined with a combining character depends on the rules defined in the Unicode Standard. The Unicode Consortium, the organization responsible for maintaining the Unicode Standard, specifies these rules.

Here are some general guidelines to determine if a base character can be combined with a combining character:

Character Composition Model: Unicode follows a character composition model, which means that some characters can be composed from a base character and one or more combining characters. This allows for a wide range of characters and diacritics to be represented in text.

Compatibility: Unicode defines compatibility characters and compatibility composition rules to ensure that combining characters work as expected with base characters. If a base character is defined as a "combining character target," it means that it can be combined with one or more combining characters.

Normalization Forms: Unicode defines normalization forms (NFC, NFD, NFKC, NFKD) to handle character composition and decomposition. NFC (Normalization Form C) is the most commonly used form for combining characters, and it ensures that text is represented in a composed form when possible.

Character Properties: Unicode assigns specific properties to each character, including whether it can be used as a base character and whether it can combine with combining characters. You can refer to the Unicode Character Database (UCD) to check the properties of individual characters.

Combining Class: Combining characters are assigned a "combining class" value, which determines their combining behavior. Base characters and combining characters are combined according to their combining class values.

Compatibility Decomposition: Some base characters can be combined using compatibility decomposition mappings, even if they don't have explicit combining characters. These mappings are defined in the Unicode Standard.

To determine whether a specific base character can be combined with a combining character, you can consult the Unicode Standard documentation, specifically the Unicode Character Database (UCD) and the Unicode Technical Reports related to normalization and composition. Additionally, Unicode-aware text processing libraries and programming languages often provide functions or methods for handling character composition and decomposition, making it easier to work with combined characters in text processing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tig I only had to do that because of the CharacterMap scenario using the space to separate the runes to avoid incorrect output. I don't have enough knowledge to do better.

Contents [Row, Col - 1].Rune = (Rune)normalized [0];
if (normalized.Length > 1) {
for (int i = 1; i < normalized.Length; i++) {
Contents [Row, Col - 1].CombiningMarks.Add ((Rune)normalized [i]);
}
}
Contents [Row, Col - 1].Attribute = CurrentAttribute;
Contents [Row, Col - 1].IsDirty = true;
} else {
Contents [Row, Col].Rune = rune;
Contents [Row, Col].Attribute = CurrentAttribute;
Contents [Row, Col].IsDirty = true;
}
Contents [Row, Col - 1].Attribute = CurrentAttribute;
Contents [Row, Col - 1].IsDirty = true;

//Col--;
} else {
Contents [Row, Col].Attribute = CurrentAttribute;
Contents [Row, Col].IsDirty = true;
Expand Down
10 changes: 8 additions & 2 deletions Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ namespace Terminal.Gui;
/// </summary>
internal class CursesDriver : ConsoleDriver {

public override int Cols => Curses.Cols;
public override int Rows => Curses.Lines;
public override int Cols {
get => Curses.Cols;
internal set => Curses.Cols = value;
}
public override int Rows {
get => Curses.Lines;
internal set => Curses.Lines = value;
}

CursorVisibility? _initialCursorVisibility = null;
CursorVisibility? _currentCursorVisibility = null;
Expand Down
8 changes: 8 additions & 0 deletions Terminal.Gui/ConsoleDrivers/CursesDriver/binding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,20 @@ static public Window initscr ()
get {
return lines;
}
internal set {
// For unit tests
lines = value;
}
}

public static int Cols {
get {
return cols;
}
internal set {
// For unit tests
cols = value;
}
}

//
Expand Down
8 changes: 7 additions & 1 deletion Terminal.Gui/ConsoleDrivers/NetDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,13 @@ public override void UpdateScreen ()
outputWidth++;
var rune = (Rune)Contents [row, col].Rune;
output.Append (rune.ToString ());
if (rune.IsSurrogatePair () && rune.GetColumns () < 2) {
if (Contents [row, col].CombiningMarks.Count > 0) {
foreach (var combMark in Contents [row, col].CombiningMarks) {
output.Append (combMark.ToString ());
}
WriteToConsole (output, ref lastCol, row, ref outputWidth);
SetCursorPosition (col - 1, row);
} else if (rune.IsSurrogatePair () && rune.GetColumns () < 2) {
WriteToConsole (output, ref lastCol, row, ref outputWidth);
SetCursorPosition (col - 1, row);
}
Expand Down
44 changes: 21 additions & 23 deletions Terminal.Gui/ConsoleDrivers/WindowsDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,28 +807,26 @@ public WindowsDriver ()
internal override MainLoop Init ()
{
_mainLoopDriver = new WindowsMainLoop (this);
if (RunningUnitTests) {
return new MainLoop (_mainLoopDriver);
}

try {
if (WinConsole != null) {
// BUGBUG: The results from GetConsoleOutputWindow are incorrect when called from Init.
// Our thread in WindowsMainLoop.CheckWin will get the correct results. See #if HACK_CHECK_WINCHANGED
var winSize = WinConsole.GetConsoleOutputWindow (out Point pos);
Cols = winSize.Width;
Rows = winSize.Height;
}
WindowsConsole.SmallRect.MakeEmpty (ref _damageRegion);
if (!RunningUnitTests) {
try {
if (WinConsole != null) {
// BUGBUG: The results from GetConsoleOutputWindow are incorrect when called from Init.
// Our thread in WindowsMainLoop.CheckWin will get the correct results. See #if HACK_CHECK_WINCHANGED
var winSize = WinConsole.GetConsoleOutputWindow (out Point pos);
Cols = winSize.Width;
Rows = winSize.Height;
}
WindowsConsole.SmallRect.MakeEmpty (ref _damageRegion);

if (_isWindowsTerminal) {
Console.Out.Write (EscSeqUtils.CSI_SaveCursorAndActivateAltBufferNoBackscroll);
if (_isWindowsTerminal) {
Console.Out.Write (EscSeqUtils.CSI_SaveCursorAndActivateAltBufferNoBackscroll);
}
} catch (Win32Exception e) {
// We are being run in an environment that does not support a console
// such as a unit test, or a pipe.
Debug.WriteLine ($"Likely running unit tests. Setting WinConsole to null so we can test it elsewhere. Exception: {e}");
WinConsole = null;
}
} catch (Win32Exception e) {
// We are being run in an environment that does not support a console
// such as a unit test, or a pipe.
Debug.WriteLine ($"Likely running unit tests. Setting WinConsole to null so we can test it elsewhere. Exception: {e}");
WinConsole = null;
}

CurrentAttribute = new Attribute (Color.White, Color.Black);
Expand Down Expand Up @@ -885,7 +883,7 @@ private void ChangeWin (Object s, SizeChangedEventArgs e)
// It also is broken when modifiers keys are down too
//
//Key _keyDown = (Key)0xffffffff;

internal void ProcessInput (WindowsConsole.InputRecord inputEvent)
{
switch (inputEvent.EventType) {
Expand Down Expand Up @@ -976,9 +974,9 @@ internal void ProcessInput (WindowsConsole.InputRecord inputEvent)
_keyModifiers ??= new KeyModifiers ();

//if (_keyDown == (Key)0xffffffff) {
// Avoid sending repeat keydowns
// Avoid sending repeat keydowns
// _keyDown = map;
OnKeyDown (new KeyEventEventArgs (new KeyEvent (map, _keyModifiers)));
OnKeyDown (new KeyEventEventArgs (new KeyEvent (map, _keyModifiers)));
//}
OnKeyPressed (new KeyEventEventArgs (new KeyEvent (map, _keyModifiers)));
} else {
Expand Down
4 changes: 2 additions & 2 deletions Terminal.Gui/Drawing/Cell.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
using System.Text;


namespace Terminal.Gui;
namespace Terminal.Gui;

/// <summary>
/// Represents a single row/column in a Terminal.Gui rendering surface
Expand All @@ -23,7 +23,7 @@ public class Cell {
///// <remarks>
///// 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 Rune CombiningMark { get; set; }
internal List<Rune> CombiningMarks { get; set; } = new ();

/// <summary>
/// The attributes to use when drawing the Glyph.
Expand Down
1 change: 1 addition & 0 deletions UICatalog/Scenarios/TabViewExample.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public override void Setup ()
new Label ("This tab has a very long name which should be truncated. See TabView.MaxTabTextWidth")),
false);
tabView.AddTab (new Tab ("Les Mise" + Char.ConvertFromUtf32 (Int32.Parse ("0301", NumberStyles.HexNumber)) + "rables", new Label ("This tab name is unicode")), false);
tabView.AddTab (new Tab ("Les Mise" + Char.ConvertFromUtf32 (Int32.Parse ("0328", NumberStyles.HexNumber)) + Char.ConvertFromUtf32 (Int32.Parse ("0301", NumberStyles.HexNumber)) + "rables", new Label ("This tab name has two unicode combining masks")), false);
BDisp marked this conversation as resolved.
Show resolved Hide resolved

for (int i = 0; i < 100; i++) {
tabView.AddTab (new Tab ($"Tab{i}", new Label ($"Welcome to tab {i}")), false);
Expand Down
13 changes: 9 additions & 4 deletions UnitTests/Application/ApplicationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,20 +422,25 @@ public void Run_Loaded_Ready_Unlodaded_Events ()

#region ShutdownTests
[Fact]
public void Shutdown_Allows_Async ()
public async void Shutdown_Allows_Async ()
{
static async Task TaskWithAsyncContinuation ()
bool isCompletedSuccessfully = false;

async Task TaskWithAsyncContinuation ()
{
await Task.Yield ();
await Task.Yield ();

isCompletedSuccessfully = true;
}

Init ();
Application.Shutdown ();

var task = TaskWithAsyncContinuation ();
Assert.False (isCompletedSuccessfully);
await TaskWithAsyncContinuation ();
Thread.Sleep (100);
Assert.True (task.IsCompletedSuccessfully);
Assert.True (isCompletedSuccessfully);
}

[Fact]
Expand Down
29 changes: 13 additions & 16 deletions UnitTests/ConsoleDrivers/AddRuneTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Buffers;
using System;
using System.Buffers;
using System.Text;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -11,29 +12,33 @@ public class AddRuneTests {

public AddRuneTests (ITestOutputHelper output)
{
ConsoleDriver.RunningUnitTests = true;
this._output = output;
}

[Fact]
public void AddRune ()
[Theory]
[InlineData (typeof (FakeDriver))]
[InlineData (typeof (NetDriver))]
[InlineData (typeof (CursesDriver))]
[InlineData (typeof (WindowsDriver))]
public void AddRune (Type driverType)
{

var driver = new FakeDriver ();
Application.Init (driver);
var driver = (ConsoleDriver)Activator.CreateInstance (driverType);
driver.Init ();

driver.Rows = 25;
driver.Cols = 80;
driver.Init ();
driver.AddRune (new Rune ('a'));
Assert.Equal ((Rune)'a', driver.Contents [0, 0].Rune);

driver.End ();
Application.Shutdown ();
}

[Fact]
public void AddRune_InvalidLocation_DoesNothing ()
{
var driver = new FakeDriver ();
Application.Init (driver);
driver.Init ();

driver.Move (driver.Cols, driver.Rows);
Expand All @@ -46,14 +51,12 @@ public void AddRune_InvalidLocation_DoesNothing ()
}

driver.End ();
Application.Shutdown ();
}

[Fact]
public void AddRune_MovesToNextColumn ()
{
var driver = new FakeDriver ();
Application.Init (driver);
driver.Init ();

driver.AddRune ('a');
Expand Down Expand Up @@ -87,14 +90,12 @@ public void AddRune_MovesToNextColumn ()
}

driver.End ();
Application.Shutdown ();
}

[Fact]
public void AddRune_MovesToNextColumn_Wide ()
{
var driver = new FakeDriver ();
Application.Init (driver);
driver.Init ();

// 🍕 Slice of Pizza "\U0001F355"
Expand Down Expand Up @@ -134,15 +135,12 @@ public void AddRune_MovesToNextColumn_Wide ()
//}

driver.End ();
Application.Shutdown ();
}


[Fact]
public void AddRune_Accented_Letter_With_Three_Combining_Unicode_Chars ()
{
var driver = new FakeDriver ();
Application.Init (driver);
driver.Init ();

var expected = new Rune ('ắ');
Expand Down Expand Up @@ -189,6 +187,5 @@ public void AddRune_Accented_Letter_With_Three_Combining_Unicode_Chars ()
// TestHelpers.AssertDriverContentsWithFrameAre (@"
//ắ", output);
driver.End ();
Application.Shutdown ();
}
}
Loading
Loading