fix(input/onivim#2980): Bring back fix for onivim#2972, and fix onivi…

…m#2980 (onivim#2983)

__Issue:__ onivim#2977 introduced a regression where the `w` key wasn't working as it should - it would do nothing, and then potentially move on a subsequent key press.

__Defect:__ The new coalesce-input-binding behavior in onivim#2977 introduced a bug - for a binding like `Control+w`, `r` (rotate window) - it would only take the modifiers from the _last_ binding item. This meant that binding would get persisted internally as `w`, `r` - so a bunch of window bindings that should've been `Control+` bindings were now engaged with just pressing `w`. Subsequent key presses would potentially either run a window command (ie, `w` + `j`) would move down a window, or run the `w` if there wasn't a relevant binding.

__Fix:__ Only coalesce character strings if they have equivalent modifiers. We were missing a test case for this in our corpus, so add a test case to avoid a regression like this in the future.

Fixes onivim#2972 
Fixes onivim#2980

CHANGES_CURRENT.md

@@ -51,6 +51,7 @@
 - #2959 - Completion: Implement `"editor.acceptSuggestionOnEnter"` configuration setting
 - #2956 - Layout: Fix extra editor when splitting with a file or terminal (fixes #2900, #2952)
 - #2986 - Theme: Fallback to default theme if invalid theme is specified
+- #2977, #2983 - Input: Handle unicode characters in mappings (fixes #2972, #2980)
 
 ### Performance
 

integration_test/AddRemoveSplitTest.re

@@ -1,7 +1,7 @@
 open Oni_Model;
 open Oni_IntegrationTestLib;
 
-runTest(~name="AddRemoveSplitTest", (dispatch, wait, _runEffects) => {
+runTest(~name="AddRemoveSplitTest", ({dispatch, wait, _}) => {
   wait(~name="Wait for split to be created 1", (state: State.t) => {
     let splitCount =
       state.layout |> Feature_Layout.visibleEditors |> List.length;

integration_test/ClipboardChangeTest.re

@@ -2,7 +2,7 @@ open Oni_Model;
 open Oni_IntegrationTestLib;
 
 // Regression test for #2694
-runTest(~name="ClipboardChangeTest", (dispatch, wait, runEffects) => {
+runTest(~name="ClipboardChangeTest", ({dispatch, wait, runEffects, _}) => {
   wait(
     ~name="Set configuration to always yank to clipboard", (state: State.t) => {
     let configuration = state.configuration;

integration_test/ClipboardInsertModePasteEmptyTest.re

@@ -8,7 +8,7 @@ module Log = (
 
 runTest(
   ~name="InsertMode test - effects batched to runEffects",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );

integration_test/ClipboardInsertModePasteMultiLineTest.re

@@ -8,7 +8,7 @@ module Log = (
 
 runTest(
   ~name="InsertMode test - effects batched to runEffects",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );

integration_test/ClipboardInsertModePasteSingleLineTest.re

@@ -8,7 +8,7 @@ module Log = (
 
 runTest(
   ~name="InsertMode test - effects batched to runEffects",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );

integration_test/ClipboardNormalModePasteTest.re

@@ -17,17 +17,18 @@ module Log = (
 
 runTest(
   ~name="InsertMode test - effects batched to runEffects",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, input, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );
 
   // '*' test case
   setClipboard(Some("abc\n"));
 
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "*"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("\"");
+  input("*");
+  input("P");
+
   runEffects();
 
   wait(~name="Mode switches to insert", (state: State.t) =>
@@ -43,9 +44,10 @@ runTest(
   // '*' multi-line test case
   setClipboard(Some("1\n2\n3\n"));
 
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "*"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("\"");
+  input("*");
+  input("P");
+
   runEffects();
 
   wait(~name="Multi-line paste works correctly", (state: State.t) =>
@@ -70,9 +72,9 @@ runTest(
   // '*' multi-line test case, windows style
   setClipboard(Some("4\r\n5\r\n6\r\n"));
 
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "*"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("\"");
+  input("*");
+  input("P");
   runEffects();
 
   wait(~name="Multi-line paste works correctly", (state: State.t) =>
@@ -96,9 +98,9 @@ runTest(
   // '+' test case
   setClipboard(Some("def\n"));
 
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "*"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("\"");
+  input("*");
+  input("P");
   runEffects();
 
   wait(~name="Mode switches to insert", (state: State.t) =>
@@ -113,17 +115,17 @@ runTest(
 
   // 'a' test case
   // yank current line - def - to 'a' register
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "a"}));
-  dispatch(KeyboardInput({isText: true, input: "y"}));
-  dispatch(KeyboardInput({isText: true, input: "y"}));
+  input("\"");
+  input("a");
+  input("y");
+  input("y");
   runEffects();
 
   setClipboard(Some("ghi\n"));
 
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "a"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("\"");
+  input("a");
+  input("P");
   runEffects();
 
   wait(~name="Mode switches to insert", (state: State.t) =>
@@ -157,10 +159,9 @@ runTest(
     runEffects();
     true;
   });
-
-  dispatch(KeyboardInput({isText: true, input: "y"}));
-  dispatch(KeyboardInput({isText: true, input: "y"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("y");
+  input("y");
+  input("P");
   runEffects();
 
   wait(
@@ -198,9 +199,9 @@ runTest(
     true;
   });
 
-  dispatch(KeyboardInput({isText: true, input: "y"}));
-  dispatch(KeyboardInput({isText: true, input: "y"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+  input("y");
+  input("y");
+  input("P");
   runEffects();
 
   wait(
@@ -218,9 +219,10 @@ runTest(
 
   // Single line case - should paste in front of previous text
   setClipboard(Some("mno"));
-  dispatch(KeyboardInput({isText: true, input: "\""}));
-  dispatch(KeyboardInput({isText: true, input: "*"}));
-  dispatch(KeyboardInput({isText: true, input: "P"}));
+
+  input("\"");
+  input("*");
+  input("P");
   runEffects();
 
   wait(~name="paste with single line, from clipboard", (state: State.t) =>

integration_test/ClipboardYankTest.re

@@ -14,7 +14,7 @@ let printOpt = sOpt =>
   | Some(v) => "Some(" ++ v ++ ")"
   };
 
-runTest(~name="ClipboardYankTest", (dispatch, wait, runEffects) => {
+runTest(~name="ClipboardYankTest", ({dispatch, wait, runEffects, _}) => {
   wait(
     ~name="Set configuration to always yank to clipboard", (state: State.t) => {
     let configuration = state.configuration;

integration_test/ClipboardxpTest.re

@@ -3,7 +3,7 @@ open Oni_Core.Utility;
 open Oni_Model;
 open Oni_IntegrationTestLib;
 
-runTest(~name="ClipboardyypTest", (dispatch, wait, runEffects) => {
+runTest(~name="ClipboardyypTest", ({dispatch, wait, runEffects, _}) => {
   wait(
     ~name="Set configuration to always yank to clipboard", (state: State.t) => {
     let configuration = state.configuration;

integration_test/ClipboardyypTest.re

@@ -3,7 +3,7 @@ open Oni_Core.Utility;
 open Oni_Model;
 open Oni_IntegrationTestLib;
 
-runTest(~name="ClipboardyypTest", (dispatch, wait, runEffects) => {
+runTest(~name="ClipboardyypTest", ({dispatch, wait, runEffects, _}) => {
   wait(
     ~name="Set configuration to always yank to clipboard", (state: State.t) => {
     let configuration = state.configuration;

integration_test/ConfigurationInvalidThemeTest.re

@@ -13,7 +13,7 @@ let configuration = {|
 runTest(
   ~configuration=Some(configuration),
   ~name="ConfigurationInvalidThemeTest (Regression test for #2985)",
-  (_dispatch, wait, _) => {
+  ({wait, _}) => {
     // We should get an error message referencing our very-invalid-theme..
     wait(~name="Wait for error message", (state: State.t) => {
       state.notifications

integration_test/ConfigurationPerFileTypeTest.re

@@ -21,7 +21,7 @@ let configuration = {|
 runTest(
   ~configuration=Some(configuration),
   ~name="ConfigurationPerFileType",
-  (dispatch, wait, _) => {
+  ({dispatch, wait, _}) => {
     wait(~name="Initial mode is normal", (state: State.t) =>
       Selectors.mode(state) |> Vim.Mode.isNormal
     );

integration_test/CopyActiveFilepathToClipboardTest.re

@@ -5,7 +5,7 @@ open Oni_IntegrationTestLib;
 runTest(
   ~name=
     "CopyActiveFilepathToClipboard writes the active buffer's file path to the clipboard",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, _}) => {
     setClipboard(Some("def"));
     dispatch(CopyActiveFilepathToClipboard);
     runEffects();

integration_test/EchoNotificationTest.re

@@ -1,7 +1,7 @@
 open Oni_Model;
 open Oni_IntegrationTestLib;
 
-runTest(~name="EchoNotificationTest", (dispatch, wait, _runEffects) => {
+runTest(~name="EchoNotificationTest", ({dispatch, wait, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );

integration_test/EditorFontFromPathTest.re

@@ -14,7 +14,7 @@ let configuration = {|{ "editor.fontFamily": "|} ++ font ++ {|"}|};
 runTest(
   ~configuration=Some(configuration),
   ~name="EditorFontFromPath",
-  (_, wait, _) => {
+  ({wait, _}) => {
     wait(~name="Initial mode is normal", (state: State.t) =>
       Selectors.mode(state) |> Vim.Mode.isNormal
     );

integration_test/EditorFontValidTest.re

@@ -21,7 +21,7 @@ if (!Revery.Environment.isLinux) {
   runTest(
     ~configuration=Some(configuration),
     ~name="EditorFontValid",
-    (_, wait, _) => {
+    ({wait, _}) => {
       wait(~name="Initial mode is normal", (state: State.t) =>
         Selectors.mode(state) |> Vim.Mode.isNormal
       );

integration_test/EditorSplitFileTest.re

@@ -3,7 +3,7 @@ open Oni_IntegrationTestLib;
 
 runTest(
   ~name="#2900: Splitting with a file should just show file",
-  (dispatch, wait, runEffects) => {
+  ({dispatch, wait, runEffects, _}) => {
   wait(~name="Wait for split to be created 1", (state: State.t) => {
     let splitCount =
       state.layout |> Feature_Layout.visibleEditors |> List.length;

integration_test/EditorUtf8Test.re

@@ -5,7 +5,7 @@ open Oni_Model;
 open Oni_IntegrationTestLib;
 open Feature_Editor;
 
-runTestWithInput(~name="EditorUtf8Test", (input, dispatch, wait, _) => {
+runTest(~name="EditorUtf8Test", ({input, dispatch, wait, _}) => {
   wait(~name="Initial mode is normal", (state: State.t) =>
     Selectors.mode(state) |> Vim.Mode.isNormal
   );

integration_test/ExCommandKeybindingNormTest.re

@@ -1,6 +1,5 @@
 open Oni_Model;
 open Oni_IntegrationTestLib;
-open Actions;
 
 let keybindings =
   Some({|
@@ -12,21 +11,7 @@ let keybindings =
 runTest(
   ~keybindings,
   ~name="ExCommandKeybindingNormTest",
-  (dispatch, wait, _) => {
-    let input = key => {
-      let keyPress =
-        EditorInput.KeyPress.physicalKey(
-          ~key,
-          ~modifiers=EditorInput.Modifiers.none,
-        )
-        |> EditorInput.KeyCandidate.ofKeyPress;
-      let time = Revery.Time.now();
-
-      dispatch(KeyDown({key: keyPress, scancode: 1, time}));
-      //dispatch(TextInput(key));
-      dispatch(KeyUp({scancode: 1, time}));
-    };
-
+  ({dispatch, wait, input, _}) => {
     let testFile = getAssetPath("some-test-file.txt");
     dispatch(Actions.OpenFileByPath(testFile, None, None));
 
@@ -49,7 +34,7 @@ runTest(
     });
 
     // Press k, which is re-bound to 'norm! j'
-    input(EditorInput.Key.Character('k'));
+    input("k");
 
     // Verify cursor is at top of file
     wait(~name="Verify cursor moved down a line", (state: State.t) => {

integration_test/ExCommandKeybindingTest.re

@@ -1,6 +1,5 @@
 open Oni_Model;
 open Oni_IntegrationTestLib;
-open Actions;
 
 let keybindings =
   Some(
@@ -14,30 +13,16 @@ let keybindings =
 runTest(
   ~keybindings,
   ~name="ExCommandKeybindingTest",
-  (dispatch, wait, _) => {
-    let input = key => {
-      let keyPress =
-        EditorInput.KeyPress.physicalKey(
-          ~key,
-          ~modifiers=EditorInput.Modifiers.none,
-        )
-        |> EditorInput.KeyCandidate.ofKeyPress;
-      let time = Revery.Time.now();
-
-      dispatch(KeyDown({key: keyPress, scancode: 1, time}));
-      //dispatch(TextInput(key));
-      dispatch(KeyUp({scancode: 1, time}));
-    };
-
+  ({wait, input, _}) => {
     wait(~name="Initial sanity check", (state: State.t) => {
       let splitCount =
         state.layout |> Feature_Layout.visibleEditors |> List.length;
 
       splitCount == 1;
     });
 
-    input(EditorInput.Key.Character('k'));
-    input(EditorInput.Key.Character('k'));
+    input("k");
+    input("k");
 
     wait(~name="Wait for split to be created", (state: State.t) => {
       let splitCount =