Terminal.Gui: Casting inconsistent between `Key` and `KeyCode`

Describe the bug

Key and KeyCode support equality e.g.

// Passes
Assert.Equal (KeyCode.A, Key.A);
Assert.Equal (KeyCode.F1, Key.F1);

However when casting is involved things behave inconsistently. The most noticeable case is non character values e.g. F1

// Fails
Assert.Equal ((uint)KeyCode.F1, (uint)Key.F1);

The int cast on Key is going into the ‘char’ explicit operator which returns 0

Assert.Equal() Failure: Values differ
Expected: 1048587
Actual:   0

But there is also inconsistent behaviour when it comes to lower case letters

// Pass
Assert.Equal (KeyCode.A, Key.A);
Assert.NotEqual (KeyCode.A, Key.A.WithShift);

// Fail
Assert.Equal ((uint)KeyCode.A, (uint)Key.A);
Assert.NotEqual ((uint)KeyCode.A, (uint)Key.A.WithShift);

I think the code is going down the char route in order to get to uint and I note that the docs describes this as ‘lossy’.

	/// <summary>
	/// Explicitly cast <see cref="Key"/> to a <see langword="char"/>. The conversion is lossy. 
	/// </summary>
	/// <param name="kea"></param>
	public static explicit operator char (Key kea) => (char)kea.AsRune.Value;

Expected behavior Casting to uint should either be impossible or should be consistent

About this issue

Original URL
State: open
Created 6 months ago
Comments: 50

Most upvoted comments

Still a WIP (Merry Christmas!) but I hope this helps!

https://github.com/gui-cs/Terminal.Gui/pull/3089

tig on Dec 25, 2023

Also, it is important to note that caps lock also makes the “WithShift” behavior potentially odd.

ConsoleKeyInfo already handles that.

Here’s the output from the same code as above but with caps lock on:

Key: A | Modifiers: None | Unicode Value: 65(0x41) | Resulting Character: A
Key: A | Modifiers: Shift | Unicode Value: 97(0x61) | Resulting Character: a
Key: A | Modifiers: Alt | Unicode Value: 65(0x41) | Resulting Character: A
Key: A | Modifiers: Alt, Control | Unicode Value: 0(0x0) | Resulting Character:
Key: A | Modifiers: Alt, Shift, Control | Unicode Value: 0(0x0) | Resulting Character:
Key: A | Modifiers: Alt, Shift | Unicode Value: 97(0x61) | Resulting Character: a

The solution in Terminal.Gui is pretty robust, and some mighty fine work, but it breaks down on the types mentioned for this issue (currently), and I wonder if it’s necessary.

dodexahedron on Dec 25, 2023

It’s off-topic of the original issue, anyway, as it was only incidentally related.

I had a sample demonstrating what I was originally getting at, last night, but I’ll have to re-create it. I don’t think I’m on the right/same track as what I was dealing with at that point (or could have missed something then in the first place), so I think I’m creating unnecessary confusion at the moment. I’ll hide the off-topic tangent here for now until I figure out WTH I was specifically hitting and file a separate issue, if it really is relevant. Apologies for the de-rail.

dodexahedron on Dec 25, 2023

Why are we using our own types for this?

ConsoleKeyInfo is cumbersome, it lacks many keys and has ambiguous entries e.g. the Oem keys.

There are environmental issues too, for example some deployments will see only ‘virtual key’ and the unicode char as input.

Terminal.Gui does a lot of standardisation at the console driver level.

I don’t think changing to core types would improve the library user experience. Current issue just seems to be tied up with the use of shift/capital letters.

On Sun, 24 Dec 2023, 03:22 dodexahedron, @.***> wrote:

Also, it is important to note that caps lock also makes the “WithShift” behavior potentially odd.

ConsoleKeyInfo already handles that.

Here’s the output from the same code as above but with caps lock on:

Key: A | Modifiers: None | Unicode Value: 65(0x41) | Resulting Character: A Key: A | Modifiers: Shift | Unicode Value: 97(0x61) | Resulting Character: a Key: A | Modifiers: Alt | Unicode Value: 65(0x41) | Resulting Character: A Key: A | Modifiers: Alt, Control | Unicode Value: 0(0x0) | Resulting Character: Key: A | Modifiers: Alt, Shift, Control | Unicode Value: 0(0x0) | Resulting Character: Key: A | Modifiers: Alt, Shift | Unicode Value: 97(0x61) | Resulting Character: a

— Reply to this email directly, view it on GitHub https://github.com/gui-cs/Terminal.Gui/issues/3071#issuecomment-1868422919, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHO3C5HSCM42GH4CP3INU5LYK6NYVAVCNFSM6AAAAABBARZAJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRYGQZDEOJRHE . You are receiving this because you authored the thread.Message ID: @.***>

tznind on Dec 24, 2023

It’s TGD specific because it is testing the KeyMap and KeybooardManager, and involves other TGD-specific classes.

It’s just for reference of how it breaks.

The last line of the test method and the line two above that are the most interesting for you. After having handled a key event, cast from a char, (third line from bottom), it then checks the last character in the view (last line). For anything requiring a shift (however that is defined) it will be broken.

dodexahedron on Dec 24, 2023

@tig what tis do? I also see this on other places. You are excluding the KeyCode.Space from the wch if lower than or equal to 'z' which is wrong and lands to unexpected results.
			else if (wch <= 'z') {

				k = (KeyCode)wch & ~KeyCode.Space;

			} 

Without context it’s hard to say. It may be a leftover I missed.

tig on Dec 23, 2023

Solution may be as simple as having both char and uint cast operators? (assume that is allowed).

Maybe we should avoid to cast to char although is allowed, because it could return a value which does not represent the real value of the Key, because KeyCode is a flagged enum handling high values to allow having ConsoleKeyInfo’s KeyChar, ConsoleKey and modifiers in only one Key representation. Also it can be cast from uint to char easely.

BDisp on Dec 23, 2023

That makes sense to me.

tznind on Dec 23, 2023