fix: defer to mkdirp for checking if directory exists

src/history/historyFile.ts

@@ -1,6 +1,6 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import * as util from 'util';
+import { promisify } from 'util';
 import { configuration } from '../configuration/configuration';
 import { logger } from '../util/logger';
 import { getExtensionDirPath } from '../util/util';
@@ -11,13 +11,14 @@ export class HistoryFile {
   private _historyFileName: string;
   private _historyDir: string;
   private _history: string[] = [];
-  private get _historyFilePath(): string {
+
+  public get historyFilePath(): string {
     return path.join(this._historyDir, this._historyFileName);
   }
 
-  constructor(historyDir: string, historyFileName: string) {
-    this._historyDir = historyDir;
+  constructor(historyFileName: string, historyDir?: string) {
     this._historyFileName = historyFileName;
+    this._historyDir = historyDir ? historyDir : getExtensionDirPath();
   }
 
   public async add(value: string | undefined): Promise<void> {
@@ -39,7 +40,7 @@ export class HistoryFile {
       this._history = this._history.slice(this._history.length - configuration.history);
     }
 
-    await this.save();
+    return this.save();
   }
 
   public get(): string[] {
@@ -53,48 +54,25 @@ export class HistoryFile {
 
   public clear() {
     try {
-      fs.unlinkSync(this._historyFilePath);
+      this._history = [];
+      fs.unlinkSync(this.historyFilePath);
     } catch (err) {
-      logger.warn(`historyFile: Unable to delete ${this._historyFilePath}. err=${err}.`);
+      logger.warn(`historyFile: Unable to delete ${this.historyFilePath}. err=${err}.`);
     }
   }
 
-  public async save(): Promise<void> {
-    try {
-      if (!(await util.promisify(fs.exists)(this._historyDir))) {
-        await util.promisify(mkdirp)(this._historyDir, 0o775);
-      }
-    } catch (err) {
-      logger.error(
-        `historyFile: Failed to create directory. path=${this._historyDir}. err=${err}.`
-      );
-      throw err;
-    }
-
-    try {
-      await util.promisify(fs.writeFile)(
-        this._historyFilePath,
-        JSON.stringify(this._history),
-        'utf-8'
-      );
-    } catch (err) {
-      logger.error(`historyFile: Failed to save history. path=${this._historyDir}. err=${err}.`);
-      throw err;
-    }
-  }
-
-  public async load() {
+  public async load(): Promise<void> {
     let data = '';
 
     try {
-      data = await util.promisify(fs.readFile)(this._historyFilePath, 'utf-8');
+      data = await promisify(fs.readFile)(this.historyFilePath, 'utf-8');
     } catch (err) {
       if (err.code === 'ENOENT') {
         logger.debug(`historyFile: History does not exist. path=${this._historyDir}`);
       } else {
         logger.warn(`historyFile: Failed to load history. path=${this._historyDir} err=${err}.`);
-        return;
       }
+      return;
     }
 
     if (data.length === 0) {
@@ -114,16 +92,30 @@ export class HistoryFile {
       this.clear();
     }
   }
+
+  private async save(): Promise<void> {
+    try {
+      // create supplied directory. if directory already exists, do nothing.
+      await promisify(mkdirp)(this._historyDir, 0o775);
+      // create file
+      await promisify(fs.writeFile)(this.historyFilePath, JSON.stringify(this._history), 'utf-8');
+    } catch (err) {
+      logger.error(
+        `historyFile: Failed to save history. filepath=${this.historyFilePath}. err=${err}.`
+      );
+      throw err;
+    }
+  }
 }
 
 export class SearchHistory extends HistoryFile {
-  constructor() {
-    super(getExtensionDirPath(), '.search_history');
+  constructor(historyFileDir?: string) {
+    super('.search_history', historyFileDir);
   }
 }
 
 export class CommandLineHistory extends HistoryFile {
-  constructor() {
-    super(getExtensionDirPath(), '.cmdline_history');
+  constructor(historyFileDir?: string) {
+    super('.cmdline_history', historyFileDir);
   }
 }

src/mode/modeHandler.ts

@@ -348,7 +348,7 @@ export class ModeHandler implements vscode.Disposable {
         this.vimState = await this.handleKeyEventHelper(key, this.vimState);
       }
     } catch (e) {
-      logger.error(`ModeHandler: error handling key=${key}. err=${e}. stack=${e.stack}`);
+      logger.error(`ModeHandler: error handling key=${key}. err=${e}.`);
       throw e;
     }
 

test/cmd_line/commandLineHistory.test.ts → test/cmd_line/historyFile.test.ts

@@ -1,11 +1,14 @@
-import { CommandLineHistory } from '../../src/history/historyFile';
-import { assertEqual, setupWorkspace, cleanUpWorkspace } from '../testUtils';
-import { configuration } from '../../src/configuration/configuration';
 import * as assert from 'assert';
+import * as fs from 'fs';
+import * as os from 'os';
+import { HistoryFile } from '../../src/history/historyFile';
+import { assertEqual, setupWorkspace, cleanUpWorkspace, rndName } from '../testUtils';
+import { configuration } from '../../src/configuration/configuration';
 
-suite('command-line history', () => {
-  let history: CommandLineHistory;
+suite('HistoryFile', () => {
+  let history: HistoryFile;
   let run_cmds: string[];
+  const tmpDir = os.tmpdir();
 
   const assertArrayEquals = (expected: any, actual: any) => {
     assertEqual(expected.length, actual.length);
@@ -14,13 +17,6 @@ suite('command-line history', () => {
     }
   };
 
-  const rndName = () => {
-    return Math.random()
-      .toString(36)
-      .replace(/[^a-z]+/g, '')
-      .substr(0, 10);
-  };
-
   setup(async () => {
     await setupWorkspace();
 
@@ -29,7 +25,7 @@ suite('command-line history', () => {
       run_cmds.push(i.toString());
     }
 
-    history = new CommandLineHistory();
+    history = new HistoryFile(rndName(), tmpDir);
     await history.load();
   });
 
@@ -78,14 +74,21 @@ suite('command-line history', () => {
     assertArrayEquals(expected_raw_history, history.get());
   });
 
-  test('load and save history', async () => {
+  test('file system', async () => {
+    // history file is lazily created, should not exist
+    assert.equal(fs.existsSync(history.historyFilePath), false);
+
     for (let cmd of run_cmds) {
       await history.add(cmd);
     }
 
-    let history2 = new CommandLineHistory();
-    await history2.load();
-    assertArrayEquals(run_cmds.slice(), history2.get());
+    // history file should exist after an `add` operation
+    assert.equal(fs.existsSync(history.historyFilePath), true);
+
+    history.clear();
+
+    // expect history file to be deleted from file system and empty
+    assert.equal(fs.existsSync(history.historyFilePath), false);
   });
 
   test('change configuration.history', async () => {

test/testUtils.ts

@@ -11,7 +11,7 @@ import { TextEditor } from '../src/textEditor';
 import { getAndUpdateModeHandler } from '../extension';
 import { commandLine } from '../src/cmd_line/commandLine';
 
-function rndName() {
+export function rndName(): string {
   return Math.random()
     .toString(36)
     .replace(/[^a-z]+/g, '')