Fixes #666. Refactor ConsoleDrivers to simplify and remove duplicated code

[tig]

Fixes #666 - Include a terse summary of the change or which issue is fixed.

For v2, and as part of #2606, I'm tackling this. Todo:

• Move() and AddStr() - there's almost no difference in each of the 4 driver's implementation, and the code is complex and hard to understand.
• I have an idea that doing unicode wide manipulation in AddStr is the wrong place to do it and will investigate how to move that logic into UpdateScreen
• Each driver defines _ccol and _crow and _contents the same and uses them in the same way. These should be moved into ConsoleDriver
• Move non-driver specific color stuff to ./Drawning
• NetDriver keyboard handling is broken (e..g. multiple ESC required to quit)
• NetDriver under WSL does not work - (fails to draw correctly)
• Some double clicks not moving focus (WindowsDriver)
• CursesDriver doesn't draw when first started (until first mouse move)
• CursesDriver hangs when moving to ArabicPresentaitonFormsB
• Add more primitive unit tests, especially for unicode handling

These issues are NOT fixed in this PR and need to be addressed separately:

• CursesDriver doesn't render CharMap correctly for wide chars (and other Unicode) - Curses is doing something funky with glyphs that report GetColums() of 1 yet are rendered wide. E.g. 0x2064 (invisible times) is reported as 1 column but is rendered as 2. WindowsDriver & NetDriver correctly render this as 1 column, overlapping the next cell.
• Refactor Color/True Color
• Add TrueColor support to CursesDriver
• Add TrueColor support to NetDriver

Pull Request checklist:

• I've named my PR in the form of "Fixes #issue. Terse description."
• My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• My code follows the Terminal.Gui library design guidelines
• I ran dotnet test before commit
• I have made corresponding changes to the API documentation (using /// style comments)
• My changes generate no new warnings
• I have checked my code and corrected any poor grammar or misspellings
• I conducted basic QA to assure all features are working

[tig]

@BDisp take a look...

Right now, the only driver that's really using my new codepath is FakeDriver. When I read your message about working on the same thing, I was just about to refactor AddRune to not be virtual so there was one implementation that looks something like this:

	public override void AddRune (System.Rune systemRune)
	{
		var rune = new Rune (systemRune).MakePrintable ();
		var runeWidth = rune.GetColumnWidth ();
		var validLocation = IsValidLocation (Col, Row);

		if (validLocation) {
			if (rune.IsCombiningMark () && Col > 0) {
				// Decode the previous rune
				var previousRune = new Rune (Contents [Row, Col - 1, 0]);
				var newCombo = new StringBuilder ();
				ReadOnlySpan<char> remainingInput = previousRune.ToString ().AsSpan ();
				while (!remainingInput.IsEmpty) {
					// Decode
					OperationStatus opStatus = Rune.DecodeFromUtf16 (remainingInput, out Rune result, out int charsConsumed);

					if (opStatus == OperationStatus.DestinationTooSmall || opStatus == OperationStatus.InvalidData) {
						result = Rune.ReplacementChar;
					}

					newCombo.Append (result);
					// Slice and loop again
					remainingInput = remainingInput.Slice (charsConsumed);
				}
				newCombo.Append (rune);

				var combined = newCombo.ToString ();
				var normalized = !combined.IsNormalized () ? combined.Normalize () : combined;
				Contents [Row, Col - 1, 0] = normalized [0];// BUGBUG: This is wrong, we need to handle the case where the rune is more than one char
				Contents [Row, Col - 1, 1] = CurrentAttribute;
				Contents [Row, Col - 1, 2] = 1;
				Col--;
			} else {
				Contents [Row, Col, 0] = rune.Value;
				Contents [Row, Col, 1] = CurrentAttribute;

				if (Col > 0) {
					var left = new Rune (Contents [Row, Col - 1, 0]);
					if (left.GetColumnWidth () > 1) {
						Contents [Row, Col - 1, 0] = Rune.ReplacementChar.Value;
					}
				}

				if (runeWidth > 1) {
					Col++;
					Contents [Row, Col, 0] = Rune.ReplacementChar.Value;
					Contents [Row, Col, 1] = CurrentAttribute;
					Contents [Row, Col, 2] = 1;
				}
			}
		}
		Col++;
	}

To enable CursesDriver to render to curses at AddRune time, vs in UpdateScreen, my plan was to add new virtual methods (e.g. OnContentsChanged(col, row, Rune, Attribute, Dirty).

One thing I just learned is having Contents just be an array of tuples of ints is we'll never be able to support combining sequences that don't normalize into a single unicode char. See ContentsTests.AddStr_With_Combining_Characters for an example that will never work.

To address this, I propose we change the way Contents is defined to be a two dimensional array of structs like this:

struct ContentsCell {
   string runes;
   Attribute attr;
   bool Dirty;
}

Also, look at how I refactored things, including the System.Text.Rune extension methods.

[tig]

See #2616

[BDisp]

You are doing well. I think with your focused on the ConsoleDriver and I with focused on the System.Text.Rune will result on the right combination.

[tig]

@BDisp & @tznind - This huge, massive, PR is now ready for review.

See the first post in this thread for what I've decided to punt to other PRs, so we can merge this.

I want to add some more unicode unit tests too.

[BDisp]

@tig I only left a question. See bellow, thanks.

[BDisp]

Originally when a surrogate pair hasn't enough space to display I replaced it with a space. Now you are using the replacement rune. Is this temporary or you are think let it as is now? It's a matter of personal choice but I only I'm asking if it was for a test or a definitive choice.

[tig]

I am not sure what the right thing to do is.

In some cases (e.g. when debugging), it's helpful to have the replacement char shown. But in real-world cases, it may look better to have spaces.

[tig]

For now, I'm reverting to show the replacement char

[BDisp]

I am not sure what the right thing to do is.

That was controlled on the AddRune method. I referred to surrogate pairs, but it also may be non surrogate UTF16 that has a column width of 2. Even some surrogate pairs have only one column width of 1. Well, a valid surrogate pair is composed by two surrogates (lower and high). When you are reading a surrogate pair that doesn't fit in the available space (you have only one column) you are reading the first char surrogate that isn't printable and you replace it. Before when I checked a rune that doesn't fit I replace with a space, because the user must see the right rune or none, instead of a replacement rune. The condition is the same for all drivers but each driver has it own way to replace it. Only the contents are equal.

In some cases (e.g. when debugging), it's helpful to have the replacement char shown. But in real-world cases, it may look better to have spaces.

Yes is true, I agree. The only driver that is dealing with surrogate pairs with a column width of 1 is the NetDriver because it use a String.Builder list, but I think the FakeDriver may use it too.