Merge pull request #3451 from VSCodeVim/validators

feat: configuration validators
VSCodeVim · Feb 6, 2019 · 4a79e3c · 4a79e3c
2 parents e5c674e + 0fbe990
commit 4a79e3c
Show file tree

Hide file tree

Showing 17 changed files with 514 additions and 268 deletions.
diff --git a/extension.ts b/extension.ts
@@ -68,20 +68,19 @@ export async function getAndUpdateModeHandler(forceSyncAndUpdate = false): Promi
 }
 
 async function loadConfiguration() {
-  const configurationErrors = await configuration.load();
   const logger = Logger.get('Configuration');
 
-  const numErrors = configurationErrors.filter(e => e.level === 'error').length;
-  logger.debug(`${numErrors} errors found with vim configuration`);
+  const validatorResults = await configuration.load();
+  logger.debug(`${validatorResults.numErrors} errors found with vim configuration`);
 
-  if (numErrors > 0) {
-    for (let configurationError of configurationErrors) {
-      switch (configurationError.level) {
+  if (validatorResults.numErrors > 0) {
+    for (let validatorResult of validatorResults.get()) {
+      switch (validatorResult.level) {
         case 'error':
-          logger.error(configurationError.message);
+          logger.error(validatorResult.message);
           break;
         case 'warning':
-          logger.warn(configurationError.message);
+          logger.warn(validatorResult.message);
           break;
       }
     }

diff --git a/src/actions/plugins/imswitcher.ts b/src/actions/plugins/imswitcher.ts
@@ -1,31 +1,24 @@
 import * as util from '../../util/util';
-import { Globals } from '../../globals';
+import { Logger } from '../../util/logger';
 import { ModeName } from '../../mode/mode';
 import { configuration } from '../../configuration/configuration';
-import { exists } from 'fs';
-import { Logger } from '../../util/logger';
-import { promisify } from 'util';
 
 /**
  * InputMethodSwitcher changes input method when mode changed
  */
 export class InputMethodSwitcher {
+  private readonly logger = Logger.get('IMSwitcher');
+  private execute: (cmd: string) => Promise<string>;
+  private savedIMKey = '';
+
   constructor(execute: (cmd: string) => Promise<string> = util.executeShell) {
     this.execute = execute;
   }
 
-  private execute: (cmd: string) => Promise<string>;
-  private readonly logger = Logger.get('IMSwitcher');
-  private savedIMKey = '';
-
   public async switchInputMethod(prevMode: ModeName, newMode: ModeName) {
     if (configuration.autoSwitchInputMethod.enable !== true) {
       return;
     }
-    if (!this.isConfigurationValid()) {
-      this.disableIMSwitch();
-      return;
-    }
     // when you exit from insert-like mode, save origin input method and set it to default
     let isPrevModeInsertLike = this.isInsertLikeMode(prevMode);
     let isNewModeInsertLike = this.isInsertLikeMode(newMode);
@@ -41,21 +34,13 @@ export class InputMethodSwitcher {
   // save origin input method and set input method to default
   private async switchToDefaultIM() {
     const obtainIMCmd = configuration.autoSwitchInputMethod.obtainIMCmd;
-    const rawObtainIMCmd = this.getRawCmd(obtainIMCmd);
-    if ((await promisify(exists)(rawObtainIMCmd)) || Globals.isTesting) {
-      try {
-        const insertIMKey = await this.execute(obtainIMCmd);
-        if (insertIMKey !== undefined) {
-          this.savedIMKey = insertIMKey.trim();
-        }
-      } catch (e) {
-        this.logger.error(`Error switching to default IM. err=${e}`);
+    try {
+      const insertIMKey = await this.execute(obtainIMCmd);
+      if (insertIMKey !== undefined) {
+        this.savedIMKey = insertIMKey.trim();
       }
-    } else {
-      this.logger.error(
-        `Unable to find ${rawObtainIMCmd}. Check your 'vim.autoSwitchInputMethod.obtainIMCmd' in VSCode setting.`
-      );
-      this.disableIMSwitch();
+    } catch (e) {
+      this.logger.error(`Error switching to default IM. err=${e}`);
     }
 
     const defaultIMKey = configuration.autoSwitchInputMethod.defaultIM;
@@ -73,21 +58,13 @@ export class InputMethodSwitcher {
 
   private async switchToIM(imKey: string) {
     let switchIMCmd = configuration.autoSwitchInputMethod.switchIMCmd;
-    const rawSwitchIMCmd = this.getRawCmd(switchIMCmd);
-    if ((await promisify(exists)(rawSwitchIMCmd)) || Globals.isTesting) {
-      if (imKey !== '' && imKey !== undefined) {
-        switchIMCmd = switchIMCmd.replace('{im}', imKey);
-        try {
-          await this.execute(switchIMCmd);
-        } catch (e) {
-          this.logger.error(`Error switching to IM. err=${e}`);
-        }
+    if (imKey !== '' && imKey !== undefined) {
+      switchIMCmd = switchIMCmd.replace('{im}', imKey);
+      try {
+        await this.execute(switchIMCmd);
+      } catch (e) {
+        this.logger.error(`Error switching to IM. err=${e}`);
       }
-    } else {
-      this.logger.error(
-        `Unable to find ${rawSwitchIMCmd}. Check your 'vim.autoSwitchInputMethod.switchIMCmd' in VSCode setting.`
-      );
-      this.disableIMSwitch();
     }
   }
 
@@ -99,35 +76,4 @@ export class InputMethodSwitcher {
     ]);
     return insertLikeModes.has(mode);
   }
-
-  private getRawCmd(cmd: string): string {
-    return cmd.split(' ')[0];
-  }
-
-  private disableIMSwitch() {
-    this.logger.warn('disabling IMSwitch');
-    configuration.autoSwitchInputMethod.enable = false;
-  }
-
-  private isConfigurationValid(): boolean {
-    let switchIMCmd = configuration.autoSwitchInputMethod.switchIMCmd;
-    if (!switchIMCmd.includes('{im}')) {
-      this.logger.error(
-        'vim.autoSwitchInputMethod.switchIMCmd is incorrect, \
-        it should contain the placeholder {im}'
-      );
-      return false;
-    }
-    let obtainIMCmd = configuration.autoSwitchInputMethod.obtainIMCmd;
-    if (obtainIMCmd === undefined || obtainIMCmd === '') {
-      this.logger.error('vim.autoSwitchInputMethod.obtainIMCmd is empty');
-      return false;
-    }
-    let defaultIMKey = configuration.autoSwitchInputMethod.defaultIM;
-    if (defaultIMKey === undefined || defaultIMKey === '') {
-      this.logger.error('vim.autoSwitchInputMethod.defaultIM is empty');
-      return false;
-    }
-    return true;
-  }
 }
diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts
@@ -1,7 +1,7 @@
 import * as vscode from 'vscode';
-import { ConfigurationError } from './configurationError';
 import { Globals } from '../globals';
 import { Notation } from './notation';
+import { ValidatorResults } from './iconfigurationValidator';
 import { VsCodeContext } from '../util/vscode-context';
 import { configurationValidator } from './configurationValidator';
 import {
@@ -66,13 +66,11 @@ class Configuration implements IConfiguration {
     'underline-thin': vscode.TextEditorCursorStyle.UnderlineThin,
   };
 
-  public async load(): Promise<ConfigurationError[]> {
+  public async load(): Promise<ValidatorResults> {
     let vimConfigs: any = Globals.isTesting
       ? Globals.mockConfiguration
       : this.getConfiguration('vim');
 
-    let configurationErrors = new Array<ConfigurationError>();
-
     /* tslint:disable:forin */
     // Disable forin rule here as we make accessors enumerable.`
     for (const option in this) {
@@ -87,72 +85,7 @@ class Configuration implements IConfiguration {
 
     this.leader = Notation.NormalizeKey(this.leader, this.leaderDefault);
 
-    // remapped keys
-    const modeKeyBindingsKeys = [
-      'insertModeKeyBindings',
-      'insertModeKeyBindingsNonRecursive',
-      'normalModeKeyBindings',
-      'normalModeKeyBindingsNonRecursive',
-      'visualModeKeyBindings',
-      'visualModeKeyBindingsNonRecursive',
-    ];
-    for (const modeKeyBindingsKey of modeKeyBindingsKeys) {
-      let keybindings = configuration[modeKeyBindingsKey];
-
-      const modeKeyBindingsMap = new Map<string, IKeyRemapping>();
-      for (let i = keybindings.length - 1; i >= 0; i--) {
-        let remapping = keybindings[i] as IKeyRemapping;
-
-        // validate
-        let remappingErrors = await configurationValidator.isRemappingValid(remapping);
-        configurationErrors = configurationErrors.concat(remappingErrors);
-
-        if (remappingErrors.filter(e => e.level === 'error').length > 0) {
-          // errors with remapping, skip
-          keybindings.splice(i, 1);
-          continue;
-        }
-
-        // normalize
-        if (remapping.before) {
-          remapping.before.forEach(
-            (key, idx) => (remapping.before[idx] = Notation.NormalizeKey(key, this.leader))
-          );
-        }
-
-        if (remapping.after) {
-          remapping.after.forEach(
-            (key, idx) => (remapping.after![idx] = Notation.NormalizeKey(key, this.leader))
-          );
-        }
-
-        // check for duplicates
-        const beforeKeys = remapping.before.join('');
-        if (modeKeyBindingsMap.has(beforeKeys)) {
-          configurationErrors.push({
-            level: 'error',
-            message: `${remapping.before}. Duplicate remapped key for ${beforeKeys}.`,
-          });
-          continue;
-        }
-
-        // add to map
-        modeKeyBindingsMap.set(beforeKeys, remapping);
-      }
-
-      configuration[modeKeyBindingsKey + 'Map'] = modeKeyBindingsMap;
-    }
-
-    // neovim
-    let neovimErrors = await configurationValidator.isNeovimValid(
-      configuration.enableNeovim,
-      configuration.neovimPath
-    );
-    configurationErrors = configurationErrors.concat(neovimErrors);
-    if (neovimErrors.filter(e => e.level === 'error').length > 0) {
-      // if error encountered with configuration, disable neovim
-      configuration.enableNeovim = false;
-    }
+    const validatorResults = await configurationValidator.validate(configuration);
 
     // wrap keys
     this.wrapKeys = {};
@@ -205,7 +138,7 @@ class Configuration implements IConfiguration {
     VsCodeContext.Set('vim.overrideCopy', this.overrideCopy);
     VsCodeContext.Set('vim.overrideCtrlC', this.overrideCopy || this.useCtrlKeys);
 
-    return configurationErrors;
+    return validatorResults;
   }
 
   getConfiguration(section: string = ''): vscode.WorkspaceConfiguration {

diff --git a/src/configuration/configurationError.ts b/src/configuration/configurationError.ts
diff --git a/src/configuration/configurationValidator.ts b/src/configuration/configurationValidator.ts
@@ -1,85 +1,34 @@
-import * as vscode from 'vscode';
-import * as fs from 'fs';
-import { IKeyRemapping } from './iconfiguration';
-import { ConfigurationError } from './configurationError';
-import { promisify } from 'util';
+import { IConfiguration } from './iconfiguration';
+import { IConfigurationValidator, ValidatorResults } from './iconfigurationValidator';
+import { InputMethodSwitcherConfigurationValidator } from './validators/inputMethodSwitcherValidator';
+import { NeovimValidator } from './validators/neovimValidator';
+import { RemappingValidator } from './validators/remappingValidator';
 
 class ConfigurationValidator {
-  private _commandMap: Map<string, boolean>;
-
-  public async isCommandValid(command: string): Promise<boolean> {
-    if (command.startsWith(':')) {
-      return true;
-    }
-
-    return (await this.getCommandMap()).has(command);
+  private _validators: IConfigurationValidator[];
+
+  constructor() {
+    this._validators = [
+      new InputMethodSwitcherConfigurationValidator(),
+      new NeovimValidator(),
+      new RemappingValidator(),
+    ];
   }
 
-  public async isNeovimValid(
-    isNeovimEnabled: boolean,
-    neovimPath: string
-  ): Promise<ConfigurationError[]> {
-    if (isNeovimEnabled) {
-      try {
-        const stat = await promisify(fs.stat)(neovimPath);
-        if (!stat.isFile()) {
-          return [
-            {
-              level: 'error',
-              message: `Invalid neovimPath. Please configure full path to nvim binary.`,
-            },
-          ];
-        }
-      } catch (e) {
-        return [{ level: 'error', message: `Invalid neovimPath. ${e.message}.` }];
-      }
-    }
-    return [];
-  }
-
-  public async isRemappingValid(remapping: IKeyRemapping): Promise<ConfigurationError[]> {
-    if (!remapping.after && !remapping.commands) {
-      return [{ level: 'error', message: `${remapping.before} missing 'after' key or 'command'.` }];
-    }
-
-    if (!(remapping.before instanceof Array)) {
-      return [
-        { level: 'error', message: `Remapping of '${remapping.before}' should be a string array.` },
-      ];
-    }
-
-    if (remapping.after && !(remapping.after instanceof Array)) {
-      return [
-        { level: 'error', message: `Remapping of '${remapping.after}' should be a string array.` },
-      ];
-    }
-
-    if (remapping.commands) {
-      for (const command of remapping.commands) {
-        let cmd: string;
-
-        if (typeof command === 'string') {
-          cmd = command;
-        } else {
-          cmd = command.command;
-        }
+  public async validate(config: IConfiguration): Promise<ValidatorResults> {
+    const results = new ValidatorResults();
 
-        if (!(await configurationValidator.isCommandValid(cmd))) {
-          return [{ level: 'warning', message: `${cmd} does not exist.` }];
-        }
+    for (const validator of this._validators) {
+      let validatorResults = await validator.validate(config);
+      if (validatorResults.hasError) {
+        // errors found in configuration, disable feature
+        validator.disable(config);
       }
-    }
-
-    return [];
-  }
 
-  async getCommandMap(): Promise<Map<string, boolean>> {
-    if (this._commandMap == null) {
-      this._commandMap = new Map(
-        (await vscode.commands.getCommands(true)).map(x => [x, true] as [string, boolean])
-      );
+      results.concat(validatorResults);
     }
-    return this._commandMap;
+
+    return results;
   }
 }
 

diff --git a/src/configuration/iconfiguration.ts b/src/configuration/iconfiguration.ts
@@ -255,6 +255,13 @@ export interface IConfiguration {
   visualModeKeyBindings: IKeyRemapping[];
   visualModeKeyBindingsNonRecursive: IKeyRemapping[];
 
+  insertModeKeyBindingsMap: Map<string, IKeyRemapping>;
+  insertModeKeyBindingsNonRecursiveMap: Map<string, IKeyRemapping>;
+  normalModeKeyBindingsMap: Map<string, IKeyRemapping>;
+  normalModeKeyBindingsNonRecursiveMap: Map<string, IKeyRemapping>;
+  visualModeKeyBindingsMap: Map<string, IKeyRemapping>;
+  visualModeKeyBindingsNonRecursiveMap: Map<string, IKeyRemapping>;
+
   /**
    * Comma-separated list of motion keys that should wrap to next/previous line.
    */