Partially Addresses #2616. Support combining sequences that don't nor…

…malize (#2932) * Fixes #2616. Support combining sequences that don't normalize * Decouples Application from ConsoleDriver in TestHelpers * Updates driver tests to match new arch * Start on making all driver tests test all drivers * Improves handling if combining marks. * Fix unit tests fails. * Fix unit tests fails. * Handling combining mask. * Tying to fix this unit test that sometimes fail. * Add support for combining mask on NetDriver. * Enable CombiningMarks as List<Rune>. * Prevents combining marks on invalid runes default and space. * Formatting for CI tests. * Fix non-normalized combining mark to add 1 to Col. * Reformatting for retest the CI. * Forces non-normalized CMs to be ignored. --------- Co-authored-by: Tig <[email protected]>
gui-cs · Oct 29, 2023 · aa8b952 · aa8b952
1 parent 095013b
commit aa8b952
Show file tree

Hide file tree

Showing 26 changed files with 423 additions and 278 deletions.
diff --git a/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs b/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs
@@ -6,6 +6,8 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using static Terminal.Gui.ColorScheme;
+using System.Linq;
+using System.Data;
 
 namespace Terminal.Gui;
 
@@ -165,21 +167,38 @@ public void AddRune (Rune rune)
 		if (validLocation) {
 			rune = rune.MakePrintable ();
 			runeWidth = rune.GetColumns ();
-			if (runeWidth == 0 && rune.IsCombiningMark () && Col > 0) {
-				// This is a combining character, and we are not at the beginning of the line.
-				// TODO: Remove hard-coded [0] once combining pairs is supported
-
-				// Convert Runes to string and concatenate
-				string combined = Contents [Row, Col - 1].Rune.ToString () + rune.ToString ();
-
-				// Normalize to Form C (Canonical Composition)
-				string normalized = combined.Normalize (NormalizationForm.FormC);
-
-				Contents [Row, Col - 1].Rune = (Rune)normalized [0]; ;
-				Contents [Row, Col - 1].Attribute = CurrentAttribute;
-				Contents [Row, Col - 1].IsDirty = true;
-
-				//Col--;
+			if (runeWidth == 0 && rune.IsCombiningMark ()) {
+				if (Col > 0) {
+					if (Contents [Row, Col - 1].CombiningMarks.Count > 0) {
+						// Just add this mark to the list
+						Contents [Row, Col - 1].CombiningMarks.Add (rune);
+						// 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) {
+							// It normalized! We can just set the Cell to the left with the
+							// normalized codepoint 
+							Contents [Row, Col - 1].Rune = (Rune)normalized [0];
+							// 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);
+							// Don't move to next column (let the driver figure out what to do).
+						}
+					}
+					Contents [Row, Col - 1].Attribute = CurrentAttribute;
+					Contents [Row, Col - 1].IsDirty = true;
+				} else {
+					// Most drivers will render a combining mark at col 0 as the mark
+					Contents [Row, Col].Rune = rune;
+					Contents [Row, Col].Attribute = CurrentAttribute;
+					Contents [Row, Col].IsDirty = true;
+					Col++;
+				}
 			} else {
 				Contents [Row, Col].Attribute = CurrentAttribute;
 				Contents [Row, Col].IsDirty = true;
@@ -267,8 +286,19 @@ public void AddRune (Rune rune)
 	/// <param name="str">String.</param>
 	public void AddStr (string str)
 	{
-		foreach (var rune in str.EnumerateRunes ()) {
-			AddRune (rune);
+		var runes = str.EnumerateRunes ().ToList ();
+		for (var i = 0; i < runes.Count; i++) {
+			//if (runes [i].IsCombiningMark()) {
+
+			//	// Attempt to normalize
+			//	string combined = runes [i-1] + runes [i].ToString();
+
+			//	// Normalize to Form C (Canonical Composition)
+			//	string normalized = combined.Normalize (NormalizationForm.FormC);
+
+			//	runes [i-]
+			//}
+			AddRune (runes [i]);
 		}
 	}
 
@@ -453,7 +483,7 @@ public virtual Attribute MakeColor (Color foreground, Color background)
 	/// Called after a key has been pressed and released. Fires the <see cref="KeyPressed"/> event.
 	/// </summary>
 	/// <param name="a"></param>
-	public void OnKeyPressed (KeyEventEventArgs a) => KeyPressed?.Invoke(this, a);
+	public void OnKeyPressed (KeyEventEventArgs a) => KeyPressed?.Invoke (this, a);
 
 	/// <summary>
 	/// Event fired when a key is released.
@@ -476,7 +506,7 @@ public virtual Attribute MakeColor (Color foreground, Color background)
 	/// </summary>
 	/// <param name="a"></param>
 	public void OnKeyDown (KeyEventEventArgs a) => KeyDown?.Invoke (this, a);
-	
+
 	/// <summary>
 	/// Event fired when a mouse event occurs.
 	/// </summary>

diff --git a/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs b/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs
@@ -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;

diff --git a/Terminal.Gui/ConsoleDrivers/CursesDriver/binding.cs b/Terminal.Gui/ConsoleDrivers/CursesDriver/binding.cs
@@ -152,12 +152,20 @@ public static int Lines {
 			get {
 				return lines;
 			}
+			internal set {
+				// For unit tests
+				lines = value;
+			}
 		}
 
 		public static int Cols {
 			get {
 				return cols;
 			}
+			internal set {
+				// For unit tests
+				cols = value;
+			}
 		}
 
 		//

diff --git a/Terminal.Gui/ConsoleDrivers/NetDriver.cs b/Terminal.Gui/ConsoleDrivers/NetDriver.cs
@@ -784,8 +784,9 @@ public override void UpdateScreen ()
 						} else if (lastCol == -1) {
 							lastCol = col;
 						}
-						if (lastCol + 1 < cols)
+						if (lastCol + 1 < cols) {
 							lastCol++;
+						}
 						continue;
 					}
 
@@ -809,8 +810,19 @@ public override void UpdateScreen ()
 					}
 					outputWidth++;
 					var rune = (Rune)Contents [row, col].Rune;
-					output.Append (rune.ToString ());
-					if (rune.IsSurrogatePair () && rune.GetColumns () < 2) {
+					output.Append (rune);
+					if (Contents [row, col].CombiningMarks.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);
+					} else  if ((rune.IsSurrogatePair () && rune.GetColumns () < 2)) {
 						WriteToConsole (output, ref lastCol, row, ref outputWidth);
 						SetCursorPosition (col - 1, row);
 					}

diff --git a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs
@@ -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);
@@ -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) {
@@ -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 {

diff --git a/Terminal.Gui/Drawing/Cell.cs b/Terminal.Gui/Drawing/Cell.cs
@@ -2,28 +2,33 @@
 using System.Text;
 
 
-namespace Terminal.Gui; 
+namespace Terminal.Gui;
 
 /// <summary>
 /// Represents a single row/column in a Terminal.Gui rendering surface
 /// (e.g. <see cref="LineCanvas"/> and <see cref="ConsoleDriver"/>).
 /// </summary>
 public class Cell {
+	Rune _rune;
 	/// <summary>
 	/// The character to display. If <see cref="Rune"/> is <see langword="null"/>, then <see cref="Rune"/> is ignored.
 	/// </summary>
-	public Rune Rune { get; set; }
+	public Rune Rune {
+		get => _rune;
+		set {
+			CombiningMarks.Clear ();
+			_rune = value;
+		}
+	}
 
-	// TODO: Uncomment this once combining sequences that could not be normalized are supported.
-	///// <summary>
-	///// The combining mark for <see cref="Rune"/> that when combined makes this Cell a combining sequence that could
-	///// not be normalized to a single Rune.
-	///// If <see cref="CombiningMark"/> is <see langword="null"/>, then <see cref="CombiningMark"/> is ignored.
-	///// </summary>
-	///// <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; }
+	/// <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.
+	/// </summary>
+	/// <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 List<Rune> CombiningMarks { get; } = new List<Rune> ();
 
 	/// <summary>
 	/// The attributes to use when drawing the Glyph.

diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs
@@ -1411,7 +1411,8 @@ public void Draw (Rect bounds, Attribute normalColor, Attribute hotColor, Rect c
 					} else {
 						Application.Driver?.AddRune (rune);
 					}
-					var runeWidth = Math.Max (rune.GetColumns (), 1);
+					// BUGBUG: I think this is a bug. If rune is a combining mark current should not be incremented.
+					var runeWidth = rune.GetColumns (); //Math.Max (rune.GetColumns (), 1);
 					if (isVertical) {
 						current++;
 					} else {

diff --git a/UICatalog/Scenarios/CharacterMap.cs b/UICatalog/Scenarios/CharacterMap.cs
@@ -473,7 +473,28 @@ public override void OnDrawContentComplete (Rect contentArea)
 
 				// are we at first row of the row?
 				if (!ShowGlyphWidths || (y - ContentOffset.Y) % _rowHeight > 0) {
-					Driver.AddRune (rune);
+					if (width > 0) {
+						Driver.AddRune (rune);
+					} else {
+						if (rune.IsCombiningMark ()) {
+							// This is a hack to work around the fact that combining marks
+							// a) can't be rendered on their own
+							// b) that don't normalize are not properly supported in 
+							//    any known terminal (esp Windows/AtlasEngine). 
+							// See Issue #2616
+							var sb = new StringBuilder ();
+							sb.Append ('a');
+							sb.Append (rune);
+							// Try normalizing after combining with 'a'. If it normalizes, at least 
+							// it'll show on the 'a'. If not, just show the replacement char.
+							var normal = sb.ToString ().Normalize (NormalizationForm.FormC);
+							if (normal.Length == 1) {
+								Driver.AddRune (normal [0]);
+							} else {
+								Driver.AddRune (Rune.ReplacementChar);
+							}
+						}
+					}
 				} else {
 					Driver.SetAttribute (ColorScheme.HotNormal);
 					Driver.AddStr ($"{width}");

diff --git a/UICatalog/Scenarios/CombiningMarks.cs b/UICatalog/Scenarios/CombiningMarks.cs
@@ -0,0 +1,35 @@
+using Terminal.Gui;
+
+namespace UICatalog.Scenarios {
+	[ScenarioMetadata (Name: "Combining Marks", Description: "Illustrates how Unicode Combining Marks work (or don't).")]
+	[ScenarioCategory ("Text and Formatting")]
+	public class CombiningMarks : Scenario {
+		public override void Init ()
+		{
+			Application.Init ();
+			ConfigurationManager.Themes.Theme = Theme;
+			ConfigurationManager.Apply ();
+			Application.Top.ColorScheme = Colors.ColorSchemes [TopLevelColorScheme];
+		}
+
+		public override void Setup ()
+		{
+			Application.Top.DrawContentComplete += (s, e) => {
+				Application.Driver.Move (0, 0);
+				Application.Driver.AddStr ("Terminal.Gui only supports combining marks that normalize. See Issue #2616.");
+				Application.Driver.Move (0, 2);
+				Application.Driver.AddStr ("\u0301\u0301\u0328<- \"\\u301\\u301\\u328]\" using AddStr.");
+				Application.Driver.Move (0, 3);
+				Application.Driver.AddStr ("[a\u0301\u0301\u0328]<- \"[a\\u301\\u301\\u328]\" using AddStr.");
+				Application.Driver.Move (0, 4);
+				Application.Driver.AddRune ('[');
+				Application.Driver.AddRune ('a');
+				Application.Driver.AddRune ('\u0301');
+				Application.Driver.AddRune ('\u0301');
+				Application.Driver.AddRune ('\u0328');
+				Application.Driver.AddRune (']');
+				Application.Driver.AddStr ("<- \"[a\\u301\\u301\\u328]\" using AddRune for each.");
+			};
+		}
+	}
+}
diff --git a/UICatalog/Scenarios/TabViewExample.cs b/UICatalog/Scenarios/TabViewExample.cs
@@ -67,8 +67,8 @@ public override void Setup ()
 				"Long name Tab, I mean seriously long.  Like you would not believe how long this tab's name is its just too much really woooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooowwww thats long",
 				 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" + '\u0301' + "rables", new Label ("This tab name is unicode")), false);
+			tabView.AddTab (new Tab ("Les Mise" + '\u0328' + '\u0301' + "rables", new Label ("This tab name has two combining marks. Only one will show due to Issue #2616.")), false);
 			for (int i = 0; i < 100; i++) {
 				tabView.AddTab (new Tab ($"Tab{i}", new Label ($"Welcome to tab {i}")), false);
 			}

diff --git a/UICatalog/Scenarios/Unicode.cs b/UICatalog/Scenarios/Unicode.cs
@@ -42,9 +42,12 @@ public override void Setup ()
 
 			label = new Label ("Label (CanFocus):") { X = Pos.X (label), Y = Pos.Bottom (label) + 1 };
 			Win.Add (label);
-			testlabel = new Label ("Стоял &он, дум великих полн") { X = 20, Y = Pos.Y (label), Width = Dim.Percent (50), CanFocus = true, HotKeySpecifier = new Rune ('&') };
+			var sb = new StringBuilder ();
+			sb.Append ('e');
+			sb.Append ('\u0301');
+			sb.Append ('\u0301');
+			testlabel = new Label ($"Should be [e with two accents, but isn't due to #2616]: [{sb}]") { X = 20, Y = Pos.Y (label), Width = Dim.Percent (50), CanFocus = true, HotKeySpecifier = new Rune ('&') };
 			Win.Add (testlabel);
-
 			label = new Label ("Button:") { X = Pos.X (label), Y = Pos.Bottom (label) + 1 };
 			Win.Add (label);
 			var button = new Button ("A123456789♥♦♣♠JQK") { X = 20, Y = Pos.Y (label) };