diff --git a/Terminal.Gui/Drivers/AnsiHandling/KittyKeyboardPattern.cs b/Terminal.Gui/Drivers/AnsiHandling/KittyKeyboardPattern.cs index 25182559c7..85aabb2086 100644 --- a/Terminal.Gui/Drivers/AnsiHandling/KittyKeyboardPattern.cs +++ b/Terminal.Gui/Drivers/AnsiHandling/KittyKeyboardPattern.cs @@ -122,6 +122,7 @@ public class KittyKeyboardPattern : AnsiKeyboardParserPattern } string modifierField = match.Groups [4].Value; + modifierField = ApplyImplicitModifierState (key, modifierField); if (!string.IsNullOrEmpty (modifierField)) { @@ -170,6 +171,61 @@ private static string ParseAssociatedText (string textField) return builder.ToString (); } + private static string ApplyImplicitModifierState (Key key, string modifierField) + { + if (!key.IsModifierOnly) + { + return modifierField; + } + + int implicitEncodedModifiers = key.ModifierKey switch + { + ModifierKey.Shift or ModifierKey.LeftShift or ModifierKey.RightShift => 2, + ModifierKey.Ctrl or ModifierKey.LeftCtrl or ModifierKey.RightCtrl => 5, + ModifierKey.Alt or ModifierKey.LeftAlt or ModifierKey.RightAlt or ModifierKey.AltGr => 3, + _ => 1 + }; + + if (string.IsNullOrEmpty (modifierField)) + { + return implicitEncodedModifiers.ToString (CultureInfo.InvariantCulture); + } + + string [] parts = modifierField.Split (':'); + + // Check for release event BEFORE parsing modifiers, to handle case where modifierField is just the event type + bool isRelease = parts.Length > 1 && parts [1] == "3"; + + if (!int.TryParse (parts [0], CultureInfo.InvariantCulture, out int encodedModifiers) || encodedModifiers < 1) + { + parts [0] = implicitEncodedModifiers.ToString (CultureInfo.InvariantCulture); + + return string.Join (':', parts); + } + + // If it's a release event, preserve the event type and don't try to merge implicit modifiers + if (isRelease) + { + // For release events of modifier-only keys, ensure explicit modifiers are correct + int explicitModifiers = encodedModifiers - 1; + int implicitModifiers = implicitEncodedModifiers - 1; + + // Only merge modifiers if the explicit modifiers don't already match the implicit ones + if (explicitModifiers != implicitModifiers) + { + parts [0] = ((explicitModifiers | implicitModifiers) + 1).ToString (CultureInfo.InvariantCulture); + } + + return string.Join (':', parts); + } + + int explicitModifiersPress = encodedModifiers - 1; + int implicitModifiersPress = implicitEncodedModifiers - 1; + parts [0] = ((explicitModifiersPress | implicitModifiersPress) + 1).ToString (CultureInfo.InvariantCulture); + + return string.Join (':', parts); + } + private static (Key Key, string ModifierField) NormalizeShiftedPrintableKey (Key key, string modifierField) { string [] parts = modifierField.Split (':'); @@ -222,6 +278,7 @@ private static (Key Key, string ModifierField) NormalizeShiftedPrintableKey (Key Key printableKey = new (printableRune.Value) { + ModifierKey = key.ModifierKey, ShiftedKeyCode = key.ShiftedKeyCode, BaseLayoutKeyCode = key.BaseLayoutKeyCode, AssociatedText = key.AssociatedText diff --git a/Terminal.Gui/Input/Keyboard/Key.cs b/Terminal.Gui/Input/Keyboard/Key.cs index 0f8fa3be58..b0b64287f9 100644 --- a/Terminal.Gui/Input/Keyboard/Key.cs +++ b/Terminal.Gui/Input/Keyboard/Key.cs @@ -210,6 +210,11 @@ public string AsGrapheme return GetSingleGraphemeOrEmpty (AssociatedText); } + if (IsAlt || IsCtrl) + { + return string.Empty; + } + if (IsShift && ShiftedKeyCode != KeyCode.Null) { Rune shiftedRune = ToRune (ShiftedKeyCode); @@ -266,6 +271,11 @@ public Rune AsRune return enumerator.MoveNext () ? default (Rune) : associatedRune; } + if (IsAlt || IsCtrl) + { + return default (Rune); + } + if (IsShift && ShiftedKeyCode != KeyCode.Null) { Rune shiftedRune = ToRune (ShiftedKeyCode); diff --git a/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardParsingTests.cs b/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardParsingTests.cs index b5aea2dde8..7b08fa79db 100644 --- a/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardParsingTests.cs +++ b/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardParsingTests.cs @@ -133,6 +133,7 @@ public void KittyPattern_LeftShift_Standalone () Assert.NotNull (key); Assert.True (key.IsModifierOnly); Assert.Equal (ModifierKey.LeftShift, key.ModifierKey); + Assert.True (key.IsShift); } [Fact] @@ -144,6 +145,7 @@ public void KittyPattern_LeftCtrl_Standalone () Assert.NotNull (key); Assert.True (key.IsModifierOnly); Assert.Equal (ModifierKey.LeftCtrl, key.ModifierKey); + Assert.True (key.IsCtrl); } [Fact] @@ -155,6 +157,7 @@ public void KittyPattern_LeftAlt_Standalone () Assert.NotNull (key); Assert.True (key.IsModifierOnly); Assert.Equal (ModifierKey.LeftAlt, key.ModifierKey); + Assert.True (key.IsAlt); } [Fact] @@ -250,13 +253,15 @@ public void KittyPattern_AltGr_WithEventType_Release () [Fact] public void KittyPattern_LeftAlt_WithCtrlModifier_PreservesBothStates () { + // ESC[57443;5u = LeftAlt (implicit Alt) with Ctrl held (explicit Ctrl=5) + // After the fix, implicit Alt is combined with explicit Ctrl Key? key = _pattern.GetKey ("\u001b[57443;5u"); Assert.NotNull (key); Assert.True (key.IsModifierOnly); Assert.Equal (ModifierKey.LeftAlt, key.ModifierKey); Assert.True (key.IsCtrl); - Assert.False (key.IsAlt); + Assert.True (key.IsAlt); Assert.Equal (KeyEventType.Press, key.EventType); } @@ -286,6 +291,68 @@ public void KittyPattern_LeftCtrl_Release_WithCtrlModifier_PreservesState () Assert.Equal (KeyEventType.Release, key.EventType); } + [Fact] + public void KittyPattern_LeftCtrl_WithCapsLockModifier_PreservesCtrlState () + { + Key? key = _pattern.GetKey ("\u001b[57442;65u"); + + Assert.NotNull (key); + Assert.True (key.IsModifierOnly); + Assert.Equal (ModifierKey.LeftCtrl, key.ModifierKey); + Assert.True (key.IsCtrl); + Assert.False (key.IsAlt); + Assert.False (key.IsShift); + Assert.Equal (KeyEventType.Press, key.EventType); + } + + [Fact] + public void KittyPattern_LeftShift_WithCapsLockModifier_PreservesShiftState () + { + Key? key = _pattern.GetKey ("\u001b[57441;65u"); + + Assert.NotNull (key); + Assert.True (key.IsModifierOnly); + Assert.Equal (ModifierKey.LeftShift, key.ModifierKey); + Assert.True (key.IsShift); + Assert.False (key.IsAlt); + Assert.False (key.IsCtrl); + Assert.Equal (KeyEventType.Press, key.EventType); + } + + // Regression test for issue where modifier combinations weren't being combined correctly + [Fact] + public void KittyPattern_LeftCtrl_WithShiftModifier_CombinesImplicitAndExplicit () + { + // ESC[57442;2u = LeftCtrl (implicit Ctrl) with Shift held (explicit Shift=2) + // Should combine to Ctrl+Shift, not just Shift + Key? key = _pattern.GetKey ("\u001b[57442;2u"); + + Assert.NotNull (key); + Assert.True (key.IsModifierOnly); + Assert.Equal (ModifierKey.LeftCtrl, key.ModifierKey); + Assert.True (key.IsCtrl); + Assert.True (key.IsShift); + Assert.False (key.IsAlt); + Assert.Equal (KeyEventType.Press, key.EventType); + } + + // Regression test for issue where modifier combinations weren't being combined correctly + [Fact] + public void KittyPattern_LeftAlt_WithShiftAndCtrlModifiers_CombinesAllModifiers () + { + // ESC[57443;6u = LeftAlt (implicit Alt) with Shift+Ctrl held (explicit Shift+Ctrl=6) + // Should combine to Alt+Shift+Ctrl, not just Shift+Ctrl + Key? key = _pattern.GetKey ("\u001b[57443;6u"); + + Assert.NotNull (key); + Assert.True (key.IsModifierOnly); + Assert.Equal (ModifierKey.LeftAlt, key.ModifierKey); + Assert.True (key.IsAlt); + Assert.True (key.IsShift); + Assert.True (key.IsCtrl); + Assert.Equal (KeyEventType.Press, key.EventType); + } + [Fact] public void KittyPattern_NonModifierKey_IsNotModifierOnly () { @@ -308,6 +375,40 @@ public void KittyPattern_LeftSuper_Standalone () Assert.Equal (ModifierKey.LeftSuper, key.ModifierKey); } + [Theory] + [InlineData ("\u001b[57358u", ModifierKey.CapsLock, false, false, false)] + [InlineData ("\u001b[57359u", ModifierKey.ScrollLock, false, false, false)] + [InlineData ("\u001b[57360u", ModifierKey.NumLock, false, false, false)] + [InlineData ("\u001b[57441u", ModifierKey.LeftShift, true, false, false)] + [InlineData ("\u001b[57442u", ModifierKey.LeftCtrl, false, false, true)] + [InlineData ("\u001b[57443u", ModifierKey.LeftAlt, false, true, false)] + [InlineData ("\u001b[57444u", ModifierKey.LeftSuper, false, false, false)] + [InlineData ("\u001b[57445u", ModifierKey.LeftHyper, false, false, false)] + [InlineData ("\u001b[57447u", ModifierKey.RightShift, true, false, false)] + [InlineData ("\u001b[57448u", ModifierKey.RightCtrl, false, false, true)] + [InlineData ("\u001b[57449u", ModifierKey.RightAlt, false, true, false)] + [InlineData ("\u001b[57450u", ModifierKey.RightSuper, false, false, false)] + [InlineData ("\u001b[57451u", ModifierKey.RightHyper, false, false, false)] + [InlineData ("\u001b[57453u", ModifierKey.AltGr, false, true, false)] + public void KittyPattern_AllMappedModifierPresses_ParseWithExpectedImplicitState ( + string sequence, + ModifierKey expectedModifier, + bool expectedShift, + bool expectedAlt, + bool expectedCtrl + ) + { + Key? key = _pattern.GetKey (sequence); + + Assert.NotNull (key); + Assert.True (key.IsModifierOnly); + Assert.Equal (expectedModifier, key.ModifierKey); + Assert.Equal (expectedShift, key.IsShift); + Assert.Equal (expectedAlt, key.IsAlt); + Assert.Equal (expectedCtrl, key.IsCtrl); + Assert.Equal (KeyEventType.Press, key.EventType); + } + #endregion #region CSI ~ and Cursor Key Event Types @@ -507,9 +608,30 @@ public void KittyPattern_AssociatedText_AltModifiedPrintableKey_IsSuppressed () Assert.NotNull (key); Assert.True (key.IsAlt); + Assert.Equal (Key.T.WithAlt, key); Assert.Equal (Key.T.WithAlt.KeyCode, key.KeyCode); Assert.Equal (string.Empty, key.AssociatedText); Assert.Equal (string.Empty, key.GetPrintableText ()); + Assert.Equal (string.Empty, key.AsGrapheme); + Assert.Equal (0, key.AsRune.Value); + } + + [Fact] + public void KittyPattern_AssociatedText_ShiftAltModifiedPrintableKey_IsSuppressed () + { + // ESC[116:84;4;84u = Shift+Alt+T with shifted key 'T' and associated text 'T' + Key? key = _pattern.GetKey ("\u001b[116:84;4;84u"); + + Assert.NotNull (key); + Assert.True (key.IsShift); + Assert.True (key.IsAlt); + Assert.Equal (Key.T.WithShift.WithAlt, key); + Assert.Equal (Key.T.WithShift.WithAlt.KeyCode, key.KeyCode); + Assert.Equal ((KeyCode)'T', key.ShiftedKeyCode); + Assert.Equal (string.Empty, key.AssociatedText); + Assert.Equal (string.Empty, key.GetPrintableText ()); + Assert.Equal (string.Empty, key.AsGrapheme); + Assert.Equal (0, key.AsRune.Value); } [Fact] diff --git a/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardPipelineTests.cs b/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardPipelineTests.cs index e0905b9b45..f59752ba16 100644 --- a/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardPipelineTests.cs +++ b/Tests/UnitTestsParallelizable/Drivers/AnsiHandling/KittyKeyboardPipelineTests.cs @@ -325,6 +325,7 @@ public void Pipeline_LeftShift_PressAndRelease () Assert.Single (down); Assert.True (down [0].IsModifierOnly); Assert.Equal (ModifierKey.LeftShift, down [0].ModifierKey); + Assert.True (down [0].IsShift); Assert.Equal (KeyEventType.Press, down [0].EventType); Assert.Single (up); @@ -382,6 +383,55 @@ public void Pipeline_ModifierPress_RaisesKeyDown (string sequence, ModifierKey e Assert.Empty (up); } + [Fact] + public void Pipeline_ModifierPress_SetsImplicitModifierState () + { + (List down, List up) = InjectRawSequence ("\x1b[57441u", + "\x1b[57442u", + "\x1b[57443u"); + + Assert.Equal (3, down.Count); + Assert.True (down [0].IsShift); + Assert.True (down [1].IsCtrl); + Assert.True (down [2].IsAlt); + Assert.Empty (up); + } + + [Theory] + [InlineData ("\x1b[57358u", ModifierKey.CapsLock, false, false, false)] + [InlineData ("\x1b[57359u", ModifierKey.ScrollLock, false, false, false)] + [InlineData ("\x1b[57360u", ModifierKey.NumLock, false, false, false)] + [InlineData ("\x1b[57441u", ModifierKey.LeftShift, true, false, false)] + [InlineData ("\x1b[57442u", ModifierKey.LeftCtrl, false, false, true)] + [InlineData ("\x1b[57443u", ModifierKey.LeftAlt, false, true, false)] + [InlineData ("\x1b[57444u", ModifierKey.LeftSuper, false, false, false)] + [InlineData ("\x1b[57445u", ModifierKey.LeftHyper, false, false, false)] + [InlineData ("\x1b[57447u", ModifierKey.RightShift, true, false, false)] + [InlineData ("\x1b[57448u", ModifierKey.RightCtrl, false, false, true)] + [InlineData ("\x1b[57449u", ModifierKey.RightAlt, false, true, false)] + [InlineData ("\x1b[57450u", ModifierKey.RightSuper, false, false, false)] + [InlineData ("\x1b[57451u", ModifierKey.RightHyper, false, false, false)] + [InlineData ("\x1b[57453u", ModifierKey.AltGr, false, true, false)] + public void Pipeline_AllMappedModifierPresses_RaiseExpectedImplicitState ( + string sequence, + ModifierKey expectedModifier, + bool expectedShift, + bool expectedAlt, + bool expectedCtrl + ) + { + (List down, List up) = InjectRawSequence (sequence); + + Assert.Single (down); + Assert.True (down [0].IsModifierOnly); + Assert.Equal (expectedModifier, down [0].ModifierKey); + Assert.Equal (expectedShift, down [0].IsShift); + Assert.Equal (expectedAlt, down [0].IsAlt); + Assert.Equal (expectedCtrl, down [0].IsCtrl); + Assert.Equal (KeyEventType.Press, down [0].EventType); + Assert.Empty (up); + } + // Copilot - Opus 4.6 [Theory] [InlineData ("\x1b[57441;1:3u", ModifierKey.LeftShift)] @@ -403,6 +453,7 @@ public void Pipeline_ModifierRelease_RaisesKeyUp (string sequence, ModifierKey e Assert.Equal (KeyEventType.Release, up [0].EventType); } + [Fact] public void Pipeline_LeftAltPress_WithCtrlModifier_PreservesBothStates () { @@ -412,7 +463,37 @@ public void Pipeline_LeftAltPress_WithCtrlModifier_PreservesBothStates () Assert.True (down [0].IsModifierOnly); Assert.Equal (ModifierKey.LeftAlt, down [0].ModifierKey); Assert.True (down [0].IsCtrl); + Assert.True (down [0].IsAlt); + Assert.Empty (up); + } + + [Fact] + public void Pipeline_LeftCtrlPress_WithCapsLockModifier_PreservesCtrlState () + { + (List down, List up) = InjectRawSequence ("\x1b[57442;65u"); + + Assert.Single (down); + Assert.True (down [0].IsModifierOnly); + Assert.Equal (ModifierKey.LeftCtrl, down [0].ModifierKey); + Assert.True (down [0].IsCtrl); Assert.False (down [0].IsAlt); + Assert.False (down [0].IsShift); + Assert.Equal (KeyEventType.Press, down [0].EventType); + Assert.Empty (up); + } + + [Fact] + public void Pipeline_LeftShiftPress_WithCapsLockModifier_PreservesShiftState () + { + (List down, List up) = InjectRawSequence ("\x1b[57441;65u"); + + Assert.Single (down); + Assert.True (down [0].IsModifierOnly); + Assert.Equal (ModifierKey.LeftShift, down [0].ModifierKey); + Assert.True (down [0].IsShift); + Assert.False (down [0].IsAlt); + Assert.False (down [0].IsCtrl); + Assert.Equal (KeyEventType.Press, down [0].EventType); Assert.Empty (up); } @@ -730,5 +811,110 @@ public void Alternative_KittyCodePoints_Map_To_Correct_Keys (int kittyCode, stri Assert.Equal (expectedKey.KeyCode, down [0].KeyCode); Assert.Empty (up); } + + #endregion + + #region Regression tests for modifier key release events in Kitty keyboard protocol. + + [Fact] + public void ModifierKeyRelease_PreservesModifierKey () + { + // ESC[57441;1:3u = LeftShift release + (List down, List up) = InjectRawSequence ("\x1b[57441;1:3u"); + + Assert.Empty (down); + Assert.Single (up); + + // The bug: ModifierKey should be LeftShift, not None + Assert.True (up [0].IsModifierOnly, "Release event should be marked as modifier-only"); + Assert.Equal (ModifierKey.LeftShift, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + Assert.Equal (KeyCode.ShiftMask, up [0].KeyCode); + } + + [Fact] + public void ModifierKeyRelease_LeftCtrl_PreservesModifierKey () + { + // ESC[57442;1:3u = LeftCtrl release + (List down, List up) = InjectRawSequence ("\x1b[57442;1:3u"); + + Assert.Empty (down); + Assert.Single (up); + + Assert.True (up [0].IsModifierOnly); + Assert.Equal (ModifierKey.LeftCtrl, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + } + + [Fact] + public void ModifierKeyRelease_RightAlt_PreservesModifierKey () + { + // ESC[57449;1:3u = RightAlt release + (List down, List up) = InjectRawSequence ("\x1b[57449;1:3u"); + + Assert.Empty (down); + Assert.Single (up); + + Assert.True (up [0].IsModifierOnly); + Assert.Equal (ModifierKey.RightAlt, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + } + + [Fact] + public void ModifierKeyRelease_AltGr_PreservesModifierKey () + { + // ESC[57453;1:3u = AltGr release + (List down, List up) = InjectRawSequence ("\x1b[57453;1:3u"); + + Assert.Empty (down); + Assert.Single (up); + + Assert.True (up [0].IsModifierOnly); + Assert.Equal (ModifierKey.AltGr, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + } + + [Theory] + [InlineData ("\x1b[57441;1:3u", ModifierKey.LeftShift, KeyCode.ShiftMask)] + [InlineData ("\x1b[57442;1:3u", ModifierKey.LeftCtrl, KeyCode.CtrlMask)] + [InlineData ("\x1b[57443;1:3u", ModifierKey.LeftAlt, KeyCode.AltMask)] + [InlineData ("\x1b[57447;1:3u", ModifierKey.RightShift, KeyCode.ShiftMask)] + [InlineData ("\x1b[57448;1:3u", ModifierKey.RightCtrl, KeyCode.CtrlMask)] + [InlineData ("\x1b[57449;1:3u", ModifierKey.RightAlt, KeyCode.AltMask)] + [InlineData ("\x1b[57453;1:3u", ModifierKey.AltGr, KeyCode.AltMask)] + public void ModifierKeyRelease_AllModifiers_PreserveModifierKey (string sequence, ModifierKey expectedModifier, KeyCode keyCode) + { + (List down, List up) = InjectRawSequence (sequence); + + Assert.Empty (down); + Assert.Single (up); + Assert.True (up [0].IsModifierOnly, $"Release event for {expectedModifier} should be modifier-only"); + Assert.Equal (expectedModifier, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + Assert.Equal (keyCode, up [0].KeyCode); + } + + [Fact] + public void ModifierKeyRelease_PressAndRelease_Sequence () + { + // Press LeftShift, then release it + (List down, List up) = InjectRawSequence ("\x1b[57441u", // LeftShift press + "\x1b[57441;1:3u" // LeftShift release + ); + + Assert.Single (down); + Assert.Single (up); + + // Press event + Assert.True (down [0].IsModifierOnly); + Assert.Equal (ModifierKey.LeftShift, down [0].ModifierKey); + Assert.Equal (KeyEventType.Press, down [0].EventType); + + // Release event - should preserve ModifierKey + Assert.True (up [0].IsModifierOnly); + Assert.Equal (ModifierKey.LeftShift, up [0].ModifierKey); + Assert.Equal (KeyEventType.Release, up [0].EventType); + } + + #endregion } -#endregion diff --git a/Tests/UnitTestsParallelizable/Input/Keyboard/KeyTests.cs b/Tests/UnitTestsParallelizable/Input/Keyboard/KeyTests.cs index 3c401f1693..732c42a42d 100644 --- a/Tests/UnitTestsParallelizable/Input/Keyboard/KeyTests.cs +++ b/Tests/UnitTestsParallelizable/Input/Keyboard/KeyTests.cs @@ -290,6 +290,18 @@ public void AsGrapheme_Returns_CombiningSequence_AsSingleGrapheme () Assert.Equal (default (Rune), key.AsRune); } + [Fact] + public void AsGrapheme_And_AsRune_Suppress_ModifiedPrintableFallback_WithoutAssociatedText () + { + Key key = new (Key.T.WithShift.WithAlt) { ShiftedKeyCode = (KeyCode)'T' }; + + Assert.Equal (string.Empty, key.AsGrapheme); + Assert.Equal (default (Rune), key.AsRune); + Assert.Equal (string.Empty, key.GetPrintableText ()); + Assert.False (key.TryGetPrintableRune (out Rune rune)); + Assert.Equal (default (Rune), rune); + } + [Theory] [InlineData ("Barf")] public void Constructor_String_Invalid_Throws (string keyString) { Assert.Throws (() => new Key (keyString)); }