Fix wrong return in ModeHandlerMap.get (#4581)

Fixes #4479, fixes #3277
VSCodeVim · Feb 25, 2020 · d67eacb · d67eacb
1 parent 1f84d03
commit d67eacb
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 20 deletions.
diff --git a/src/editorIdentity.ts b/src/editorIdentity.ts
@@ -10,18 +10,7 @@ export class EditorIdentity {
     return new EditorIdentity(textEditor?.document?.fileName ?? '');
   }
 
-  /**
-   * For use in tests
-   */
-  public static createRandomEditorIdentity(): EditorIdentity {
-    return new EditorIdentity(
-      Math.random()
-        .toString(36)
-        .substring(7)
-    );
-  }
-
-  private constructor(fileName: string) {
+  public constructor(fileName: string) {
     this._fileName = fileName;
   }
 

diff --git a/src/mode/modeHandlerMap.ts b/src/mode/modeHandlerMap.ts
@@ -9,12 +9,8 @@ class ModeHandlerMapImpl {
 
   public async getOrCreate(editorId: EditorIdentity): Promise<[ModeHandler, boolean]> {
     let isNew = false;
-    let modeHandler: ModeHandler | undefined;
-    for (const [key, value] of this.modeHandlerMap.entries()) {
-      if (key.isEqual(editorId)) {
-        modeHandler = value;
-      }
-    }
+    let modeHandler: ModeHandler | undefined = this.get(editorId);
+
     if (!modeHandler) {
       isNew = true;
       modeHandler = await ModeHandler.Create();
@@ -24,7 +20,12 @@ class ModeHandlerMapImpl {
   }
 
   public get(editorId: EditorIdentity): ModeHandler | undefined {
-    return this.modeHandlerMap.get(editorId);
+    for (const [key, value] of this.modeHandlerMap.entries()) {
+      if (key.isEqual(editorId)) {
+        return value;
+      }
+    }
+    return undefined;
   }
 
   public getKeys(): EditorIdentity[] {

diff --git a/test/mode/modeHandlerMap.test.ts b/test/mode/modeHandlerMap.test.ts
@@ -2,6 +2,16 @@ import * as assert from 'assert';
 
 import { ModeHandlerMap } from '../../src/mode/modeHandlerMap';
 import { EditorIdentity } from '../../src/editorIdentity';
+import { testIt } from '../testSimplifier';
+import { KeypressState } from '../../src/actions/base';
+
+function createRandomEditorIdentity(): EditorIdentity {
+  return new EditorIdentity(
+    Math.random()
+      .toString(36)
+      .substring(7)
+  );
+}
 
 suite('Mode Handler Map', () => {
   setup(() => {
@@ -13,7 +23,7 @@ suite('Mode Handler Map', () => {
   });
 
   test('getOrCreate', async () => {
-    const key = EditorIdentity.createRandomEditorIdentity();
+    const key = createRandomEditorIdentity();
     let [modeHandler, isNew] = await ModeHandlerMap.getOrCreate(key);
     assert.strictEqual(isNew, true);
     assert.notEqual(modeHandler, undefined);
@@ -34,4 +44,18 @@ suite('Mode Handler Map', () => {
     ModeHandlerMap.delete(key);
     assert.strictEqual(ModeHandlerMap.getAll().length, 0);
   });
+
+  test('get', async () => {
+    // same file name should return the same modehandler
+    const identity = createRandomEditorIdentity();
+
+    let [modeHandler, isNew] = await ModeHandlerMap.getOrCreate(identity);
+    assert.strictEqual(isNew, true);
+    assert.notEqual(modeHandler, undefined);
+
+    const prevModeHandler = modeHandler;
+    [modeHandler, isNew] = await ModeHandlerMap.getOrCreate(new EditorIdentity(identity.fileName));
+    assert.strictEqual(isNew, false);
+    assert.strictEqual(prevModeHandler, modeHandler);
+  });
 });