[PoC] Refactor Remapper

Refactor the Remapper and ModeHandler to allow better remapping experience. It will allow to remap operator keys, motion keys and multiple keys when the first key could be handled. Should fix the following issues (maybe more): VSCodeVim#4674 VSCodeVim#4464 VSCodeVim#3988 VSCodeVim#3768 VSCodeVim#3742 VSCodeVim#2975 VSCodeVim#2955 VSCodeVim#2234 VSCodeVim#2041 VSCodeVim#1870 VSCodeVim#1821 VSCodeVim#1579 VSCodeVim#1398 Needs more testing.
berknam · Apr 13, 2020 · 3b22581 · 3b22581
1 parent 6188094
commit 3b22581
Show file tree

Hide file tree

Showing 3 changed files with 141 additions and 38 deletions.
diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts
@@ -60,6 +60,8 @@ export class Remapper implements IRemapper {
   private readonly _logger = Logger.get('Remapper');
 
   private _isPotentialRemap = false;
+  private _allowAmbiguousRemapOnFirstKey = true;
+  private _hasAmbiguousRemap = false;
   get isPotentialRemap(): boolean {
     return this._isPotentialRemap;
   }
@@ -76,6 +78,8 @@ export class Remapper implements IRemapper {
     vimState: VimState
   ): Promise<boolean> {
     this._isPotentialRemap = false;
+    let allowAmbiguousRemap = true;
+    let remainingKeys: string[] = [];
 
     if (!this._remappedModes.includes(vimState.currentMode)) {
       return false;
@@ -88,17 +92,105 @@ export class Remapper implements IRemapper {
         Mode[vimState.currentMode]
       }. keybindings=${this._configKey}.`
     );
+
+    if (keys[keys.length - 1] === '<BufferedKeys>') {
+      // Timeout finished. Don't let an ambiguous remap start another timeout again
+      keys = keys.slice(0, keys.length - 1);
+      allowAmbiguousRemap = false;
+    }
+
     let remapping: IKeyRemapping | undefined = this.findMatchingRemap(
       userDefinedRemappings,
       keys,
       vimState.currentMode
     );
 
+    let isPotentialRemap = false;
+    // Check to see if a remapping could potentially be applied when more keys are received
+    // const keysAsString = keys.join('');
+    const keysAsString = vimState.recordedState.commandWithoutCountPrefix.replace(
+      '<BufferedKeys>',
+      ''
+    );
+    if (keysAsString !== '') {
+      for (let remap of userDefinedRemappings.keys()) {
+        if (remap.startsWith(keysAsString) && remap !== keysAsString) {
+          isPotentialRemap = true;
+          break;
+        }
+      }
+    }
+
+    this._isPotentialRemap =
+      isPotentialRemap && allowAmbiguousRemap && this._allowAmbiguousRemapOnFirstKey;
+
+    if (
+      !remapping &&
+      this._hasAmbiguousRemap &&
+      (!isPotentialRemap || !allowAmbiguousRemap) &&
+      keys.length > 1
+    ) {
+      // Check if the last key broke a previous ambiguous remap
+      const range = Remapper.getRemappedKeysLengthRange(userDefinedRemappings);
+      for (let sliceLength = keys.length - 1; sliceLength >= range[0]; sliceLength--) {
+        const keysSlice = keys.slice(0, sliceLength);
+        let possibleBrokenRemap: IKeyRemapping | undefined = this.findMatchingRemap(
+          userDefinedRemappings,
+          keysSlice,
+          vimState.currentMode
+        );
+        if (possibleBrokenRemap) {
+          remapping = possibleBrokenRemap;
+          isPotentialRemap = false;
+          this._isPotentialRemap = false;
+          remainingKeys = keys.slice(remapping.before.length);
+          break;
+        }
+      }
+      this._hasAmbiguousRemap = false;
+      if (!remapping) {
+        // if there is still no remapping, handle all the keys without allowing
+        // an ambiguous remap on the first key so that we don't repeat everything
+        // again, but still allow for other ambiguous remaps after the first key.
+        //
+        // Example: if 'ii' and 'iiii' are mapped in normal and 'ii' is mapped in
+        // insert mode, and the user presses 'iiia' in normal mode or presses 'iii'
+        // and waits for the timeout to finish, we want the first 'i' to be handled
+        // without allowing ambiguous remaps, which means it will go into insert mode,
+        // but then the next 'ii' should be remapped in insert mode and after the
+        // remap the 'a' should be handled.
+        this._allowAmbiguousRemapOnFirstKey = false;
+        vimState.recordedState.commandList = vimState.recordedState.commandList.slice(
+          0,
+          -keys.length
+        );
+        await modeHandler.handleMultipleKeyEvents(keys);
+        return true;
+      }
+    }
+
+    if (isPotentialRemap && allowAmbiguousRemap && this._allowAmbiguousRemapOnFirstKey) {
+      // There are other potential remaps (ambiguous remaps), wait for other
+      // key or for the timeout to finish.
+      this._hasAmbiguousRemap = true;
+      vimState.recordedState.bufferedKeys = keys.slice(0);
+      vimState.recordedState.bufferedKeysTimeoutObj = setTimeout(() => {
+        modeHandler.handleKeyEvent('<BufferedKeys>');
+      }, configuration.timeout);
+      return true;
+    } else if (!this._allowAmbiguousRemapOnFirstKey) {
+      // First key was already prevented from buffering to wait for other remaps
+      // so we can allow for ambiguous remaps again.
+      this._allowAmbiguousRemapOnFirstKey = true;
+    }
+
     if (remapping) {
       this._logger.debug(
         `${this._configKey}. match found. before=${remapping.before}. after=${remapping.after}. command=${remapping.commands}.`
       );
 
+      this._hasAmbiguousRemap = false;
+
       if (!this._recursive) {
         vimState.isCurrentlyPerformingRemapping = true;
       }
@@ -107,20 +199,19 @@ export class Remapper implements IRemapper {
         await this.handleRemapping(remapping, vimState, modeHandler);
       } finally {
         vimState.isCurrentlyPerformingRemapping = false;
+        if (remainingKeys.length > 0) {
+          vimState.recordedState.commandList = vimState.recordedState.commandList.slice(
+            0,
+            -remainingKeys.length
+          );
+          await modeHandler.handleMultipleKeyEvents(remainingKeys);
+          return true;
+        }
       }
 
       return true;
     }
 
-    // Check to see if a remapping could potentially be applied when more keys are received
-    const keysAsString = keys.join('');
-    for (let remap of userDefinedRemappings.keys()) {
-      if (remap.startsWith(keysAsString)) {
-        this._isPotentialRemap = true;
-        break;
-      }
-    }
-
     return false;
   }
 
@@ -129,26 +220,7 @@ export class Remapper implements IRemapper {
     vimState: VimState,
     modeHandler: ModeHandler
   ) {
-    const numCharsToRemove = remapping.before.length - 1;
-    // Revert previously inserted characters
-    // (e.g. jj remapped to esc, we have to revert the inserted "jj")
-    if (vimState.currentMode === Mode.Insert) {
-      // Revert every single inserted character.
-      // We subtract 1 because we haven't actually applied the last key.
-      await vimState.historyTracker.undoAndRemoveChanges(
-        Math.max(0, numCharsToRemove * vimState.cursors.length)
-      );
-      vimState.cursors = vimState.cursors.map((c) =>
-        c.withNewStop(c.stop.getLeft(numCharsToRemove))
-      );
-    }
-    // We need to remove the keys that were remapped into different keys from the state.
-    vimState.recordedState.actionKeys = vimState.recordedState.actionKeys.slice(
-      0,
-      -numCharsToRemove
-    );
-    vimState.keyHistory = vimState.keyHistory.slice(0, -numCharsToRemove);
-
+    vimState.recordedState.resetCommandList();
     if (remapping.after) {
       const count = vimState.recordedState.count || 1;
       vimState.recordedState.count = 0;

diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts
@@ -250,9 +250,22 @@ export class ModeHandler implements vscode.Disposable {
   public async handleKeyEvent(key: string): Promise<void> {
     const now = Number(new Date());
     const printableKey = Notation.printableKey(key);
+    let hadBufferedKeys = false;
 
     this._logger.debug(`handling key=${printableKey}.`);
 
+    if (
+      (key === '<BufferedKeys>' || this.vimState.recordedState.bufferedKeys.length > 0) &&
+      this.vimState.recordedState.bufferedKeysTimeoutObj
+    ) {
+      // Handle the bufferedKeys or append the new key to the previously bufferedKeys
+      clearTimeout(this.vimState.recordedState.bufferedKeysTimeoutObj);
+      this.vimState.recordedState.bufferedKeysTimeoutObj = undefined;
+      this.vimState.recordedState.commandList = this.vimState.recordedState.bufferedKeys.slice(0);
+      this.vimState.recordedState.bufferedKeys = [];
+      hadBufferedKeys = true;
+    }
+
     // rewrite copy
     if (configuration.overrideCopy) {
       // The conditions when you trigger a "copy" rather than a ctrl-c are
@@ -288,7 +301,7 @@ export class ModeHandler implements vscode.Disposable {
 
     try {
       const isWithinTimeout = now - this.vimState.lastKeyPressedTimestamp < configuration.timeout;
-      if (!isWithinTimeout) {
+      if (!isWithinTimeout && key !== '<BufferedKeys>') {
         // sufficient time has elapsed since the prior keypress,
         // only consider the last keypress for remapping
         this.vimState.recordedState.commandList = [
@@ -306,10 +319,7 @@ export class ModeHandler implements vscode.Disposable {
       // 2. We are not in normal mode performing on an operator
       //    Example: ciwjj should be remapped if jj -> <Esc> in insert mode
       //             dd should not remap the second "d", if d -> "_d in normal mode
-      if (
-        !this.vimState.isCurrentlyPerformingRemapping &&
-        (!isOperatorCombination || this.vimState.currentMode !== Mode.Normal)
-      ) {
+      if (!this.vimState.isCurrentlyPerformingRemapping) {
         handled = await this._remappers.sendKey(
           this.vimState.recordedState.commandList,
           this,
@@ -320,7 +330,23 @@ export class ModeHandler implements vscode.Disposable {
       if (handled) {
         this.vimState.recordedState.resetCommandList();
       } else {
-        this.vimState = await this.handleKeyEventHelper(key, this.vimState);
+        if (key === '<BufferedKeys>') {
+          this.vimState.recordedState.commandList = this.vimState.recordedState.commandList.slice(
+            0,
+            -1
+          );
+          key = this.vimState.recordedState.commandList[
+            this.vimState.recordedState.commandList.length - 1
+          ];
+        }
+        if (hadBufferedKeys) {
+          for (let k of this.vimState.recordedState.commandList) {
+            key = k;
+            this.vimState = await this.handleKeyEventHelper(key, this.vimState);
+          }
+        } else {
+          this.vimState = await this.handleKeyEventHelper(key, this.vimState);
+        }
       }
     } catch (e) {
       if (e instanceof VimError) {
@@ -555,9 +581,7 @@ export class ModeHandler implements vscode.Disposable {
       }
     }
 
-    if (ranAction && vimState.currentMode !== Mode.Insert) {
-      vimState.recordedState.resetCommandList();
-    }
+    vimState.recordedState.resetCommandList();
 
     ranRepeatableAction =
       (ranRepeatableAction && vimState.currentMode === Mode.Normal) ||

diff --git a/src/state/recordedState.ts b/src/state/recordedState.ts
@@ -82,6 +82,13 @@ export class RecordedState {
    */
   public actionsRun: BaseAction[] = [];
 
+  /**
+   * Every key that was buffered to wait for a new key or the timeout to finish
+   * in order to get another potential remap or to solve an ambiguous remap.
+   */
+  public bufferedKeys: string[] = [];
+  public bufferedKeysTimeoutObj: NodeJS.Timeout | undefined = undefined;
+
   public hasRunOperator = false;
 
   public hasRunSurround = false;