Merge pull request #162 from tig/BDisp-v2_combining-normalize-fix_2616

Forces non-normalized CMs to be ignored.

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,26 +167,33 @@ 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);
-
-				if (Contents [Row, Col - 1].Rune != default && Contents [Row, Col - 1].Rune != (Rune)' ') {
-					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]);
+			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;
@@ -277,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]);
 		}
 	}
 

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,14 +810,19 @@ public override void UpdateScreen ()
 					}
 					outputWidth++;
 					var rune = (Rune)Contents [row, col].Rune;
-					output.Append (rune.ToString ());
+					output.Append (rune);
 					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) {
+						// 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);
 					}

Terminal.Gui/Drawing/Cell.cs

@@ -9,21 +9,26 @@ namespace Terminal.Gui;
 /// (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 List<Rune> CombiningMarks { get; set; } = new ();
+	/// <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.

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 {

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}");

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.");
+			};
+		}
+	}
+}

UICatalog/Scenarios/TabViewExample.cs

@@ -67,9 +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" + 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);
-
+			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);
 			}

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) };

UnitTests/ConsoleDrivers/ContentsTests.cs

@@ -2,6 +2,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -18,6 +19,23 @@ public ContentsTests (ITestOutputHelper output)
 		this.output = output;
 	}
 
+
+	[Theory]
+	[InlineData (typeof (FakeDriver))]
+	[InlineData (typeof (NetDriver))]
+	//[InlineData (typeof (CursesDriver))] // TODO: Uncomment when #2796 and #2615 are fixed
+	//[InlineData (typeof (WindowsDriver))] // TODO: Uncomment when #2610 is fixed
+	public void AddStr_Combining_Character_1st_Column (Type driverType)
+	{
+		var driver = (ConsoleDriver)Activator.CreateInstance (driverType);
+		driver.Init ();
+		var expected = "\u0301!";
+		driver.AddStr ("\u0301!"); // acute accent + exclamation mark
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
+
+		driver.End ();
+	}
+
 	[Theory]
 	[InlineData (typeof (FakeDriver))]
 	[InlineData (typeof (NetDriver))]
@@ -33,45 +51,41 @@ public void AddStr_With_Combining_Characters (Type driverType)
 		var expected = "é";
 
 		driver.AddStr (combined);
-		TestHelpers.AssertDriverContentsWithFrameAre (expected, output, driver);
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
 
 		// 3 char combine
 		// a + ogonek + acute = <U+0061, U+0328, U+0301> ( ą́ )
 		var ogonek = new System.Text.Rune (0x0328); // Combining ogonek (a small hook or comma shape)
 		combined = "a" + ogonek + acuteaccent;
-		expected = "ą́";
+		expected = ("a" + ogonek).Normalize(NormalizationForm.FormC); // See Issue #2616
 
 		driver.Move (0, 0);
 		driver.AddStr (combined);
-		TestHelpers.AssertDriverContentsWithFrameAre (expected, output, driver);
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
 
 		// e + ogonek + acute = <U+0061, U+0328, U+0301> ( ę́́ )
 		combined = "e" + ogonek + acuteaccent;
-		expected = "ę́́";
+		expected = ("e" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
 
 		driver.Move (0, 0);
 		driver.AddStr (combined);
-		TestHelpers.AssertDriverContentsWithFrameAre (expected, output, driver);
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
 
 		// i + ogonek + acute = <U+0061, U+0328, U+0301> ( į́́́ )
 		combined = "i" + ogonek + acuteaccent;
-		expected = "į́́́";
+		expected = ("i" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
 
 		driver.Move (0, 0);
 		driver.AddStr (combined);
-		TestHelpers.AssertDriverContentsWithFrameAre (expected, output, driver);
-
-		// o + ogonek + acute = <U+0061, U+0328, U+0301> ( ǫ́ )
-		combined = "o" + ogonek + acuteaccent;
-		expected = "ǫ́";
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
 
 		// u + ogonek + acute = <U+0061, U+0328, U+0301> ( ų́́́́ )
 		combined = "u" + ogonek + acuteaccent;
-		expected = "ų́́́́";
+		expected = ("u" + ogonek).Normalize (NormalizationForm.FormC); // See Issue #2616
 
 		driver.Move (0, 0);
 		driver.AddStr (combined);
-		TestHelpers.AssertDriverContentsWithFrameAre (expected, output, driver);
+		TestHelpers.AssertDriverContentsAre (expected, output, driver);
 
 		driver.End ();
 	}

UnitTests/TestHelpers.cs

@@ -165,7 +165,6 @@ public static void AssertDriverContentsAre (string expectedLook, ITestOutputHelp
 
 		for (int r = 0; r < driver.Rows; r++) {
 			for (int c = 0; c < driver.Cols; c++) {
-				// TODO: Remove hard-coded [0] once combining pairs is supported
 				Rune rune = contents [r, c].Rune;
 				if (rune.DecodeSurrogatePair (out char [] spair)) {
 					sb.Append (spair);
@@ -175,9 +174,10 @@ public static void AssertDriverContentsAre (string expectedLook, ITestOutputHelp
 				if (rune.GetColumns () > 1) {
 					c++;
 				}
-				foreach (var combMark in contents [r, c].CombiningMarks) {
-					sb.Append ((char)combMark.Value);
-				}
+				// See Issue #2616
+				//foreach (var combMark in contents [r, c].CombiningMarks) {
+				//	sb.Append ((char)combMark.Value);
+				//}
 			}
 			sb.AppendLine ();
 		}
@@ -248,9 +248,10 @@ public static Rect AssertDriverContentsWithFrameAre (string expectedLook, ITestO
 					h = r - y + 1;
 				}
 				if (x > -1) runes.Add (rune);
-				foreach (var combMark in contents [r, c].CombiningMarks) {
-					runes.Add (combMark);
-				}
+				// See Issue #2616
+				//foreach (var combMark in contents [r, c].CombiningMarks) {
+				//	runes.Add (combMark);
+				//}
 			}
 			if (runes.Count > 0) lines.Add (runes);
 		}