From d7044e70a39ef0a9f8e5579cf64cd27e212af1b5 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 3 Jun 2019 13:25:33 -0700 Subject: [PATCH 1/7] Fix the conversion from ConsoleKeyInfo/PSKeyInfo to PSKeyInfo/ConsoleKeyInfo --- PSReadLine/Keys.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PSReadLine/Keys.cs b/PSReadLine/Keys.cs index a1ad81cfc..b9c9a4a09 100644 --- a/PSReadLine/Keys.cs +++ b/PSReadLine/Keys.cs @@ -268,8 +268,8 @@ void AppendPart(string str) break; case '\0': - // This is ugly but familiar. - s = "@"; + // This could be a special kind of a modifier key (dead key) on a particular keyboard layout. + s = key.Key.ToString(); break; case char _ when (c >= 1 && c <= 26): From 25b7fa32c299602af233b39ff7defed5d3dfbcdb Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 5 Jun 2019 10:17:13 -0700 Subject: [PATCH 2/7] Keep using '@' when KeyChar = '\0' on non-Windows --- PSReadLine/Keys.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/PSReadLine/Keys.cs b/PSReadLine/Keys.cs index b9c9a4a09..d1bf339ad 100644 --- a/PSReadLine/Keys.cs +++ b/PSReadLine/Keys.cs @@ -268,8 +268,15 @@ void AppendPart(string str) break; case '\0': - // This could be a special kind of a modifier key (dead key) on a particular keyboard layout. - s = key.Key.ToString(); + // On Windows: + // This could be a special kind of a modifier key (dead key) for a particular keyboard layout. + // We use the text form of the virtual key in such case, so the resulted PSKeyInfo can be converted back to ConsoleKeyInfo correctly later on, + // and be properly ignore during rendering. + // On non-Windows: + // The dead key is not an issue when there is a tty involved. + // But the virtual key is not captured as accurately as on Windows, e.g. Ctrl+2 results in `key.Key = 0`. + // So on non-Windows, we use `@` in case `key.KeyChar = '\0'`. This is ugly but familiar. + s = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? key.Key.ToString() : "@"; break; case char _ when (c >= 1 && c <= 26): From b372e99877e4041e1d0cb15902ea55856cc85062 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 5 Jun 2019 14:28:24 -0700 Subject: [PATCH 3/7] More updates and add tests --- PSReadLine/Keys.cs | 32 ++++++++++++++++++++------------ test/DeadKeyTest.cs | 21 +++++++++++++++++++++ test/KeyInfo-en-US-windows.json | 12 ++++++++++++ test/KeyInfo-fr-FR-windows.json | 14 ++++++++++++++ test/KeyboardLayouts.cs | 2 +- 5 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 test/DeadKeyTest.cs diff --git a/PSReadLine/Keys.cs b/PSReadLine/Keys.cs index d1bf339ad..1e5ccfd42 100644 --- a/PSReadLine/Keys.cs +++ b/PSReadLine/Keys.cs @@ -125,7 +125,7 @@ public static extern int ToUnicode( static readonly ThreadLocal toUnicodeBuffer = new ThreadLocal(() => new char[2]); static readonly ThreadLocal toUnicodeStateBuffer = new ThreadLocal(() => new byte[256]); - internal static void TryGetCharFromConsoleKey(ConsoleKeyInfo key, ref char result) + internal static void TryGetCharFromConsoleKey(ConsoleKeyInfo key, ref char result, ref bool isDeadKey) { var modifiers = key.Modifiers; var virtualKey = key.Key; @@ -149,11 +149,20 @@ internal static void TryGetCharFromConsoleKey(ConsoleKeyInfo key, ref char resul } int charCount = ToUnicode(virtualKey, scanCode, state, chars, chars.Length, flags); - // TODO: support diacriticals (charCount == 2) if (charCount == 1) { result = chars[0]; } + else if (charCount == -1 || charCount >=2) + { + // Quoted from https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-tounicode#return-value: + // "Return value -1 -- + // The specified virtual key is a dead-key character (accent or diacritic). + // Return value >=2 -- + // Two or more characters were written to the buffer specified by pwszBuff. The most common cause for this is that a dead-key character + // (accent or diacritic) stored in the keyboard layout could not be combined with the specified virtual key to form a single character." + isDeadKey = true; + } } static readonly ThreadLocal keyInfoStringBuilder = new ThreadLocal(() => new StringBuilder()); @@ -217,6 +226,7 @@ void AppendPart(string str) } var c = key.KeyChar; + var isDeadKey = false; if (char.IsControl(c) ) { // We have the virtual key code and Windows has a handy api to map that to the non-control @@ -226,7 +236,7 @@ void AppendPart(string str) if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var keySansControl = new ConsoleKeyInfo(key.KeyChar, key.Key, isShift, isAlt, control: false); - TryGetCharFromConsoleKey(keySansControl, ref c); + TryGetCharFromConsoleKey(keySansControl, ref c, ref isDeadKey); } } else if (isAlt && isCtrl) @@ -268,15 +278,13 @@ void AppendPart(string str) break; case '\0': - // On Windows: - // This could be a special kind of a modifier key (dead key) for a particular keyboard layout. - // We use the text form of the virtual key in such case, so the resulted PSKeyInfo can be converted back to ConsoleKeyInfo correctly later on, - // and be properly ignore during rendering. - // On non-Windows: - // The dead key is not an issue when there is a tty involved. - // But the virtual key is not captured as accurately as on Windows, e.g. Ctrl+2 results in `key.Key = 0`. - // So on non-Windows, we use `@` in case `key.KeyChar = '\0'`. This is ugly but familiar. - s = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? key.Key.ToString() : "@"; + // This could be a dead key for a particular keyboard layout in Windows console. + // The dead key is not an issue when there is tty involved, so on non-Windows, `isDeadKey` is always false. + // + // When we believe it's a dead key, we use the text form of the virtual key so the resulted PSKeyInfo can be + // converted back to ConsoleKeyInfo correctly later on, and be properly ignore during rendering. + // Otherwise, we use `@` in case `key.KeyChar = '\0'`. This is ugly but familiar. + s = isDeadKey ? key.Key.ToString() : "@"; break; case char _ when (c >= 1 && c <= 26): diff --git a/test/DeadKeyTest.cs b/test/DeadKeyTest.cs new file mode 100644 index 000000000..69241ebc7 --- /dev/null +++ b/test/DeadKeyTest.cs @@ -0,0 +1,21 @@ +using System; +using Xunit; + +namespace Test +{ + public partial class ReadLine + { + [SkippableFact] + public void DeadKeyShouldBeIgnored() + { + Skip.If(this.Fixture.Lang != "fr-FR", "The dead key test requires Keyboard layout to be set to 'fr-FR'"); + TestSetup(KeyMode.Cmd); + + Test("aa", Keys("aa", _.DeadKey_Backtick)); + Test("aab", Keys("aa", _.DeadKey_Backtick, 'b')); + + Test("aa", Keys("aa", _.DeadKey_Tilde)); + Test("aab", Keys("aa", _.DeadKey_Tilde, 'b')); + } + } +} diff --git a/test/KeyInfo-en-US-windows.json b/test/KeyInfo-en-US-windows.json index 53243ab27..925352a54 100644 --- a/test/KeyInfo-en-US-windows.json +++ b/test/KeyInfo-en-US-windows.json @@ -1414,5 +1414,17 @@ "KeyChar": "z", "ConsoleKey": "Z", "Modifiers": "0" + }, + { + "Key": "DeadKey_Backtick", + "KeyChar": "\u0000", + "ConsoleKey": "Oem3", + "Modifiers": "0" + }, + { + "Key": "DeadKey_Tilde", + "KeyChar": "\u0000", + "ConsoleKey": "Oem3", + "Modifiers": "Shift" } ] diff --git a/test/KeyInfo-fr-FR-windows.json b/test/KeyInfo-fr-FR-windows.json index e6a39bc02..5194d5976 100644 --- a/test/KeyInfo-fr-FR-windows.json +++ b/test/KeyInfo-fr-FR-windows.json @@ -1636,5 +1636,19 @@ "ConsoleKey": "Z", "Modifiers": "0", "Investigate": false + }, + { + "Key": "DeadKey_Backtick", + "KeyChar": "\u0000", + "ConsoleKey": "D7", + "Modifiers": "Alt", + "Investigate": false + }, + { + "Key": "DeadKey_Tilde", + "KeyChar": "\u0000", + "ConsoleKey": "D2", + "Modifiers": "Alt", + "Investigate": false } ] diff --git a/test/KeyboardLayouts.cs b/test/KeyboardLayouts.cs index bccf39895..29e4629ad 100644 --- a/test/KeyboardLayouts.cs +++ b/test/KeyboardLayouts.cs @@ -87,7 +87,7 @@ public string KeyAsPropertyName() } var key = (alt != null) ? Key.Substring(0, Key.Length - 1) + alt - : Key; + : Key; return key.Replace('+', '_'); } From b92b649625bb45840c3c7b76fd19671e10db3ad4 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 5 Jun 2019 14:30:55 -0700 Subject: [PATCH 4/7] Revert the change in KeyInfo-en-US-windows.json --- test/KeyInfo-en-US-windows.json | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/KeyInfo-en-US-windows.json b/test/KeyInfo-en-US-windows.json index 925352a54..53243ab27 100644 --- a/test/KeyInfo-en-US-windows.json +++ b/test/KeyInfo-en-US-windows.json @@ -1414,17 +1414,5 @@ "KeyChar": "z", "ConsoleKey": "Z", "Modifiers": "0" - }, - { - "Key": "DeadKey_Backtick", - "KeyChar": "\u0000", - "ConsoleKey": "Oem3", - "Modifiers": "0" - }, - { - "Key": "DeadKey_Tilde", - "KeyChar": "\u0000", - "ConsoleKey": "Oem3", - "Modifiers": "Shift" } ] From 5ffca1a0f2acd7f92e416b18f13aae7777be1771 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 5 Jun 2019 15:16:17 -0700 Subject: [PATCH 5/7] Fix the test on fr-FR OS --- test/DeadKeyTest.cs | 7 ++----- test/KeyInfo-fr-FR-windows.json | 13 +++---------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/test/DeadKeyTest.cs b/test/DeadKeyTest.cs index 69241ebc7..849aa484e 100644 --- a/test/DeadKeyTest.cs +++ b/test/DeadKeyTest.cs @@ -11,11 +11,8 @@ public void DeadKeyShouldBeIgnored() Skip.If(this.Fixture.Lang != "fr-FR", "The dead key test requires Keyboard layout to be set to 'fr-FR'"); TestSetup(KeyMode.Cmd); - Test("aa", Keys("aa", _.DeadKey_Backtick)); - Test("aab", Keys("aa", _.DeadKey_Backtick, 'b')); - - Test("aa", Keys("aa", _.DeadKey_Tilde)); - Test("aab", Keys("aa", _.DeadKey_Tilde, 'b')); + Test("aa", Keys("aa", _.DeadKey_Caret)); + Test("aab", Keys("aa", _.DeadKey_Caret, 'b')); } } } diff --git a/test/KeyInfo-fr-FR-windows.json b/test/KeyInfo-fr-FR-windows.json index 5194d5976..da5c4e678 100644 --- a/test/KeyInfo-fr-FR-windows.json +++ b/test/KeyInfo-fr-FR-windows.json @@ -1638,17 +1638,10 @@ "Investigate": false }, { - "Key": "DeadKey_Backtick", + "Key": "DeadKey_Caret", "KeyChar": "\u0000", - "ConsoleKey": "D7", - "Modifiers": "Alt", - "Investigate": false - }, - { - "Key": "DeadKey_Tilde", - "KeyChar": "\u0000", - "ConsoleKey": "D2", - "Modifiers": "Alt", + "ConsoleKey": "Oem6", + "Modifiers": "0", "Investigate": false } ] From cecebbfddd5b736687eae5a5e3065d9696eaf14f Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 7 Jun 2019 09:10:37 -0700 Subject: [PATCH 6/7] Address Jason's comment --- test/DeadKeyTest.cs | 1 + test/KeyInfo-fr-FR-windows.json | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/test/DeadKeyTest.cs b/test/DeadKeyTest.cs index 849aa484e..bf0c85b72 100644 --- a/test/DeadKeyTest.cs +++ b/test/DeadKeyTest.cs @@ -13,6 +13,7 @@ public void DeadKeyShouldBeIgnored() Test("aa", Keys("aa", _.DeadKey_Caret)); Test("aab", Keys("aa", _.DeadKey_Caret, 'b')); + Test("aaâ", Keys("aa", _.DeadKey_Caret_A)); } } } diff --git a/test/KeyInfo-fr-FR-windows.json b/test/KeyInfo-fr-FR-windows.json index da5c4e678..0aad3a2b7 100644 --- a/test/KeyInfo-fr-FR-windows.json +++ b/test/KeyInfo-fr-FR-windows.json @@ -1643,5 +1643,12 @@ "ConsoleKey": "Oem6", "Modifiers": "0", "Investigate": false + }, + { + "Key": "DeadKey_Caret+A", + "KeyChar": "\u00e2", + "ConsoleKey": "A", + "Modifiers": "0", + "Investigate": false } ] From b283a04686b9d90e2099a00d45a3d56ff961fba1 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 7 Jun 2019 09:47:06 -0700 Subject: [PATCH 7/7] Fix a typo --- PSReadLine/Keys.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PSReadLine/Keys.cs b/PSReadLine/Keys.cs index 1e5ccfd42..b08b14a3e 100644 --- a/PSReadLine/Keys.cs +++ b/PSReadLine/Keys.cs @@ -282,7 +282,7 @@ void AppendPart(string str) // The dead key is not an issue when there is tty involved, so on non-Windows, `isDeadKey` is always false. // // When we believe it's a dead key, we use the text form of the virtual key so the resulted PSKeyInfo can be - // converted back to ConsoleKeyInfo correctly later on, and be properly ignore during rendering. + // converted back to ConsoleKeyInfo correctly later on, and be properly ignored during rendering. // Otherwise, we use `@` in case `key.KeyChar = '\0'`. This is ugly but familiar. s = isDeadKey ? key.Key.ToString() : "@"; break;