From 7941c0a4058f690492d04bf043b95e37acb3ede3 Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Mon, 28 Jan 2019 20:21:56 -0800 Subject: [PATCH 1/7] fix: validate configurations once, instead of every key press --- extension.ts | 35 +++++++--- src/actions/plugins/imswitcher.ts | 27 ++++---- src/cmd_line/commandLine.ts | 9 +-- src/cmd_line/commands/file.ts | 7 +- src/cmd_line/commands/write.ts | 11 ++-- src/cmd_line/parser.ts | 4 +- src/configuration/configuration.ts | 73 ++++++++++++++------- src/configuration/configurationError.ts | 4 ++ src/configuration/configurationValidator.ts | 49 +++++++++++--- src/configuration/iconfiguration.ts | 10 ++- src/configuration/remapper.ts | 38 +++-------- src/history/historyFile.ts | 19 +++--- src/history/historyTracker.ts | 11 ++-- src/mode/modeHandler.ts | 17 ++--- src/mode/modes.ts | 10 +-- src/neovim/neovim.ts | 13 ++-- src/state/vimState.ts | 6 +- src/taskQueue.ts | 6 +- src/util/clipboard.ts | 6 +- src/util/logger.ts | 48 ++++++++++---- src/util/util.ts | 8 ++- test/testUtils.ts | 2 +- 22 files changed, 252 insertions(+), 161 deletions(-) create mode 100644 src/configuration/configurationError.ts diff --git a/extension.ts b/extension.ts index 7008a11ed66..6c31ad5ec95 100644 --- a/extension.ts +++ b/extension.ts @@ -16,14 +16,13 @@ import { ModeHandler } from './src/mode/modeHandler'; import { ModeHandlerMap } from './src/mode/modeHandlerMap'; import { ModeName } from './src/mode/mode'; import { Notation } from './src/configuration/notation'; +import { Logger } from './src/util/logger'; import { Position } from './src/common/motion/position'; import { StatusBar } from './src/statusBar'; import { VsCodeContext } from './src/util/vscode-context'; import { commandLine } from './src/cmd_line/commandLine'; import { configuration } from './src/configuration/configuration'; -import { configurationValidator } from './src/configuration/configurationValidator'; import { globalState } from './src/state/globalState'; -import { logger } from './src/util/logger'; import { taskQueue } from './src/taskQueue'; let extensionContext: vscode.ExtensionContext; @@ -68,21 +67,40 @@ export async function getAndUpdateModeHandler(forceSyncAndUpdate = false): Promi return curHandler; } +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} found with vim configuration`); + + if (numErrors > 0) { + for (let configurationError of configurationErrors) { + switch (configurationError.level) { + case 'error': + logger.error(configurationError.message); + break; + case 'warning': + logger.warn(configurationError.message); + break; + } + } + } +} export async function activate(context: vscode.ExtensionContext) { - logger.debug('Extension: activating vscodevim.'); + // before we do anything else, + // we need to load the configuration first + await loadConfiguration(); extensionContext = context; extensionContext.subscriptions.push(StatusBar); - logger.debug('Extension: registering event handlers.'); - // workspace events registerEventListener( context, vscode.workspace.onDidChangeConfiguration, - () => { - logger.debug('onDidChangeConfiguration: reloading configuration'); - configuration.reload(); + async () => { + await loadConfiguration(); }, false ); @@ -344,7 +362,6 @@ export async function activate(context: vscode.ExtensionContext) { await Promise.all([ commandLine.load(), globalState.load(), - configurationValidator.initialize(), // This is called last because getAndUpdateModeHandler() will change cursor toggleExtension(configuration.disableExtension, compositionState), ]); diff --git a/src/actions/plugins/imswitcher.ts b/src/actions/plugins/imswitcher.ts index 677c50a1a40..58f59d13788 100644 --- a/src/actions/plugins/imswitcher.ts +++ b/src/actions/plugins/imswitcher.ts @@ -3,7 +3,7 @@ import { Globals } from '../../globals'; import { ModeName } from '../../mode/mode'; import { configuration } from '../../configuration/configuration'; import { exists } from 'fs'; -import { logger } from '../../util/logger'; +import { Logger } from '../../util/logger'; import { promisify } from 'util'; /** @@ -14,8 +14,9 @@ export class InputMethodSwitcher { this.execute = execute; } - private savedIMKey = ''; private execute: (cmd: string) => Promise; + private readonly logger = Logger.get('IMSwitcher'); + private savedIMKey = ''; public async switchInputMethod(prevMode: ModeName, newMode: ModeName) { if (configuration.autoSwitchInputMethod.enable !== true) { @@ -48,11 +49,11 @@ export class InputMethodSwitcher { this.savedIMKey = insertIMKey.trim(); } } catch (e) { - logger.error(`IMSwitcher: Error switching to default IM. err=${e}`); + this.logger.error(`Error switching to default IM. err=${e}`); } } else { - logger.error( - `IMSwitcher: Unable to find ${rawObtainIMCmd}. Check your 'vim.autoSwitchInputMethod.obtainIMCmd' in VSCode setting.` + this.logger.error( + `Unable to find ${rawObtainIMCmd}. Check your 'vim.autoSwitchInputMethod.obtainIMCmd' in VSCode setting.` ); this.disableIMSwitch(); } @@ -79,12 +80,12 @@ export class InputMethodSwitcher { try { await this.execute(switchIMCmd); } catch (e) { - logger.error(`IMSwitcher: Error switching to IM. err=${e}`); + this.logger.error(`Error switching to IM. err=${e}`); } } } else { - logger.error( - `IMSwitcher: Unable to find ${rawSwitchIMCmd}. Check your 'vim.autoSwitchInputMethod.switchIMCmd' in VSCode setting.` + this.logger.error( + `Unable to find ${rawSwitchIMCmd}. Check your 'vim.autoSwitchInputMethod.switchIMCmd' in VSCode setting.` ); this.disableIMSwitch(); } @@ -104,27 +105,27 @@ export class InputMethodSwitcher { } private disableIMSwitch() { - logger.warn('IMSwitcher: disabling IMSwitch'); + this.logger.warn('disabling IMSwitch'); configuration.autoSwitchInputMethod.enable = false; } private isConfigurationValid(): boolean { let switchIMCmd = configuration.autoSwitchInputMethod.switchIMCmd; if (!switchIMCmd.includes('{im}')) { - logger.error( - 'IMSwitcher: vim.autoSwitchInputMethod.switchIMCmd is incorrect, \ + 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 === '') { - logger.error('IMSwitcher: vim.autoSwitchInputMethod.obtainIMCmd is empty'); + this.logger.error('vim.autoSwitchInputMethod.obtainIMCmd is empty'); return false; } let defaultIMKey = configuration.autoSwitchInputMethod.defaultIM; if (defaultIMKey === undefined || defaultIMKey === '') { - logger.error('IMSwitcher: vim.autoSwitchInputMethod.defaultIM is empty'); + this.logger.error('vim.autoSwitchInputMethod.defaultIM is empty'); return false; } return true; diff --git a/src/cmd_line/commandLine.ts b/src/cmd_line/commandLine.ts index ef86dcab413..242c997648f 100644 --- a/src/cmd_line/commandLine.ts +++ b/src/cmd_line/commandLine.ts @@ -2,14 +2,15 @@ import * as parser from './parser'; import * as vscode from 'vscode'; import { CommandLineHistory } from '../history/historyFile'; import { ModeName } from './../mode/mode'; +import { Logger } from '../util/logger'; import { StatusBar } from '../statusBar'; import { VimError, ErrorCode } from '../error'; import { VimState } from '../state/vimState'; import { configuration } from '../configuration/configuration'; -import { logger } from '../util/logger'; class CommandLine { private _history: CommandLineHistory; + private readonly _logger = Logger.get('CommandLine'); /** * Index used for navigating commandline history with and @@ -78,14 +79,14 @@ class CommandLine { ); } } else { - logger.error(`commandLine: Error executing cmd=${command}. err=${e}.`); + this._logger.error(`Error executing cmd=${command}. err=${e}.`); } } } public async PromptAndRun(initialText: string, vimState: VimState): Promise { if (!vscode.window.activeTextEditor) { - logger.debug('commandLine: No active document'); + this._logger.debug('No active document'); return; } let cmd = await vscode.window.showInputBox(this.getInputBoxOptions(initialText)); @@ -103,7 +104,7 @@ class CommandLine { public async ShowHistory(initialText: string, vimState: VimState): Promise { if (!vscode.window.activeTextEditor) { - logger.debug('commandLine: No active document.'); + this._logger.debug('No active document.'); return ''; } diff --git a/src/cmd_line/commands/file.ts b/src/cmd_line/commands/file.ts index 87b93e930c5..ffbc617516c 100644 --- a/src/cmd_line/commands/file.ts +++ b/src/cmd_line/commands/file.ts @@ -3,9 +3,9 @@ import * as node from '../node'; import * as path from 'path'; import * as util from 'util'; import * as vscode from 'vscode'; -import { logger } from '../../util/logger'; +import untildify = require('untildify'); +import { Logger } from '../../util/logger'; -const untildify = require('untildify'); const doesFileExist = util.promisify(fs.exists); export enum FilePosition { @@ -23,6 +23,7 @@ export interface IFileCommandArguments extends node.ICommandArgs { export class FileCommand extends node.CommandBase { protected _arguments: IFileCommandArguments; + private readonly _logger = Logger.get('File'); constructor(args: IFileCommandArguments) { super(); @@ -111,7 +112,7 @@ export class FileCommand extends node.CommandBase { if (this._arguments.createFileIfNotExists) { await util.promisify(fs.close)(await util.promisify(fs.open)(filePath, 'w')); } else { - logger.error(`file: ${filePath} does not exist.`); + this._logger.error(`${filePath} does not exist.`); return; } } diff --git a/src/cmd_line/commands/write.ts b/src/cmd_line/commands/write.ts index fbff94a90c3..5360daca318 100644 --- a/src/cmd_line/commands/write.ts +++ b/src/cmd_line/commands/write.ts @@ -3,9 +3,9 @@ import * as node from '../node'; import * as path from 'path'; import * as util from 'util'; import * as vscode from 'vscode'; +import { Logger } from '../../util/logger'; import { StatusBar } from '../../statusBar'; import { VimState } from '../../state/vimState'; -import { logger } from '../../util/logger'; export interface IWriteCommandArguments extends node.ICommandArgs { opt?: string; @@ -23,6 +23,7 @@ export interface IWriteCommandArguments extends node.ICommandArgs { // export class WriteCommand extends node.CommandBase { protected _arguments: IWriteCommandArguments; + private readonly _logger = Logger.get('Write'); constructor(args: IWriteCommandArguments) { super(); @@ -36,16 +37,16 @@ export class WriteCommand extends node.CommandBase { async execute(vimState: VimState): Promise { if (this.arguments.opt) { - logger.warn('write: not implemented'); + this._logger.warn('not implemented'); return; } else if (this.arguments.file) { - logger.warn('write: not implemented'); + this._logger.warn('not implemented'); return; } else if (this.arguments.append) { - logger.warn('write: not implemented'); + this._logger.warn('not implemented'); return; } else if (this.arguments.cmd) { - logger.warn('write: not implemented'); + this._logger.warn('not implemented'); return; } diff --git a/src/cmd_line/parser.ts b/src/cmd_line/parser.ts index 6210b155fcc..e83c79b501a 100644 --- a/src/cmd_line/parser.ts +++ b/src/cmd_line/parser.ts @@ -1,9 +1,9 @@ import * as lexer from './lexer'; import * as node from './node'; import * as token from './token'; +import { Logger } from '../util/logger'; import { VimError, ErrorCode } from '../error'; import { commandParsers } from './subparser'; -import { logger } from '../util/logger'; interface IParseFunction { (state: ParserState, command: node.CommandLine): IParseFunction | null; @@ -20,6 +20,8 @@ export function parse(input: string): node.CommandLine { } function parseLineRange(state: ParserState, commandLine: node.CommandLine): IParseFunction | null { + const logger = Logger.get('Parser'); + while (true) { let tok = state.next(); switch (tok.type) { diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts index f1836965b28..50fade31de3 100644 --- a/src/configuration/configuration.ts +++ b/src/configuration/configuration.ts @@ -1,8 +1,9 @@ import * as vscode from 'vscode'; - +import { ConfigurationError } from './configurationError'; import { Globals } from '../globals'; import { Notation } from './notation'; -import { taskQueue } from '../taskQueue'; +import { VsCodeContext } from '../util/vscode-context'; +import { configurationValidator } from './configurationValidator'; import { IConfiguration, IKeyRemapping, @@ -10,7 +11,6 @@ import { IAutoSwitchInputMethod, IDebugConfiguration, } from './iconfiguration'; -import { VsCodeContext } from '../util/vscode-context'; const packagejson: { contributes: { @@ -66,15 +66,13 @@ class Configuration implements IConfiguration { 'underline-thin': vscode.TextEditorCursorStyle.UnderlineThin, }; - constructor() { - this.reload(); - } - - reload() { + public async load(): Promise { let vimConfigs: any = Globals.isTesting ? Globals.mockConfiguration : this.getConfiguration('vim'); + let configurationErrors = new Array(); + /* tslint:disable:forin */ // Disable forin rule here as we make accessors enumerable.` for (const option in this) { @@ -99,7 +97,20 @@ class Configuration implements IConfiguration { this.visualModeKeyBindingsNonRecursive, ]; for (const keybindings of keybindingList) { - for (let remapping of keybindings) { + const keybindingMap: { [key: string]: {} } = Object.create(null); + + for (let i = keybindings.length - 1; i >= 0; i--) { + let remapping = keybindings[i]; + + 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; + } + if (remapping.before) { remapping.before.forEach( (key, idx) => (remapping.before[idx] = Notation.NormalizeKey(key, this.leader)) @@ -111,11 +122,22 @@ class Configuration implements IConfiguration { (key, idx) => (remapping.after![idx] = Notation.NormalizeKey(key, this.leader)) ); } + + const keys = remapping.before.join(''); + if (keys in keybindingMap) { + configurationErrors.push({ + level: 'error', + message: `${remapping.before}. Duplicate remapped key for ${keys}.`, + }); + continue; + } else { + keybindingMap[keys] = {}; + } } } + // wrap keys this.wrapKeys = {}; - for (const wrapKey of this.whichwrap.split(',')) { this.wrapKeys[wrapKey] = true; } @@ -164,11 +186,13 @@ class Configuration implements IConfiguration { VsCodeContext.Set('vim.overrideCopy', this.overrideCopy); VsCodeContext.Set('vim.overrideCtrlC', this.overrideCopy || this.useCtrlKeys); + + return configurationErrors; } getConfiguration(section: string = ''): vscode.WorkspaceConfiguration { - let activeTextEditor = vscode.window.activeTextEditor; - let resource = activeTextEditor ? activeTextEditor.document.uri : undefined; + const activeTextEditor = vscode.window.activeTextEditor; + const resource = activeTextEditor ? activeTextEditor.document.uri : undefined; return vscode.workspace.getConfiguration(section, resource); } @@ -380,22 +404,21 @@ function overlapSetting(args: { return; } - taskQueue.enqueueTask(async () => { - if (args.map) { - for (let [vscodeSetting, vimSetting] of args.map.entries()) { - if (value === vimSetting) { - value = vscodeSetting; - break; - } + if (args.map) { + for (let [vscodeSetting, vimSetting] of args.map.entries()) { + if (value === vimSetting) { + value = vscodeSetting; + break; } } + } - await this.getConfiguration('editor').update( - args.settingName, - value, - vscode.ConfigurationTarget.Global - ); - }, 'config'); + // update configuration asynchronously + this.getConfiguration('editor').update( + args.settingName, + value, + vscode.ConfigurationTarget.Global + ); }, enumerable: true, configurable: true, diff --git a/src/configuration/configurationError.ts b/src/configuration/configurationError.ts new file mode 100644 index 00000000000..92ed822ed1b --- /dev/null +++ b/src/configuration/configurationError.ts @@ -0,0 +1,4 @@ +export interface ConfigurationError { + level: 'error' | 'warning'; + message: string; +} diff --git a/src/configuration/configurationValidator.ts b/src/configuration/configurationValidator.ts index cc7574d57d3..ed94292241e 100644 --- a/src/configuration/configurationValidator.ts +++ b/src/configuration/configurationValidator.ts @@ -1,21 +1,50 @@ import * as vscode from 'vscode'; +import { IKeyRemapping } from './iconfiguration'; +import { ConfigurationError } from './configurationError'; class ConfigurationValidator { - private map: Map; + private _commandMap: Map; - public async initialize(): Promise { - this.map = new Map( - (await vscode.commands.getCommands(true)).map(x => [x, true] as [string, boolean]) - ); - } - - public isCommandValid(command: string): boolean { + public async isCommandValid(command: string): Promise { if (command.startsWith(':')) { return true; } - return this.map.get(command) || false; + return (await this.getCommandMap()).get(command) || false; + } + + public async isRemappingValid(remapping: IKeyRemapping): Promise { + if (!remapping.after && !remapping.commands) { + return [{ level: 'error', message: `${remapping.before} missing 'after' key or 'command'.` }]; + } + + if (remapping.commands) { + for (const command of remapping.commands) { + let cmd: string; + + if (typeof command === 'string') { + cmd = command; + } else { + cmd = command.command; + } + + if (!(await configurationValidator.isCommandValid(cmd))) { + return [{ level: 'warning', message: `${cmd} does not exist.` }]; + } + } + } + + return []; + } + + async getCommandMap(): Promise> { + if (this._commandMap == null) { + this._commandMap = new Map( + (await vscode.commands.getCommands(true)).map(x => [x, true] as [string, boolean]) + ); + } + return this._commandMap; } } -export let configurationValidator = new ConfigurationValidator(); +export const configurationValidator = new ConfigurationValidator(); diff --git a/src/configuration/iconfiguration.ts b/src/configuration/iconfiguration.ts index 6a17955aeb8..40560296577 100644 --- a/src/configuration/iconfiguration.ts +++ b/src/configuration/iconfiguration.ts @@ -32,13 +32,13 @@ export interface IDebugConfiguration { * Maximum level of messages to show as VS Code information message * Supported values: ['error', 'warn', 'info', 'verbose', 'debug'] */ - loggingLevelForAlert: string; + loggingLevelForAlert: 'error' | 'warn' | 'info' | 'verbose' | 'debug'; /** * Maximum level of messages to log to console. * Supported values: ['error', 'warn', 'info', 'verbose', 'debug'] */ - loggingLevelForConsole: string; + loggingLevelForConsole: 'error' | 'warn' | 'info' | 'verbose' | 'debug'; } export interface IConfiguration { @@ -197,6 +197,10 @@ export interface IConfiguration { */ relativenumber: boolean; + /** + * keywords contain alphanumeric characters and '_'. + * If not configured `editor.wordSeparators` is used + */ iskeyword: string; /** @@ -253,7 +257,7 @@ export interface IConfiguration { visualModeKeyBindingsNonRecursive: IKeyRemapping[]; /** - * emulate whichwrap + * Comma-separated list of motion keys that should wrap to next/previous line. */ whichwrap: string; diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index a6583d5df5f..0a052838801 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -1,14 +1,12 @@ import * as _ from 'lodash'; import * as vscode from 'vscode'; - import { IKeyRemapping } from './iconfiguration'; +import { Logger } from '../util/logger'; import { ModeHandler } from '../mode/modeHandler'; import { ModeName } from '../mode/mode'; import { VimState } from './../state/vimState'; import { commandLine } from '../cmd_line/commandLine'; import { configuration } from '../configuration/configuration'; -import { configurationValidator } from './configurationValidator'; -import { logger } from '../util/logger'; interface IRemapper { /** @@ -57,6 +55,7 @@ export class Remapper implements IRemapper { private readonly _configKey: string; private readonly _remappedModes: ModeName[]; private readonly _recursive: boolean; + private readonly _logger = Logger.get('Remapper'); private _isPotentialRemap = false; get isPotentialRemap(): boolean { @@ -82,8 +81,8 @@ export class Remapper implements IRemapper { const userDefinedRemappings = this._getRemappings(); - logger.debug( - `Remapper: find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}.` + this._logger.debug( + `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}.` ); let remapping: IKeyRemapping | undefined = Remapper.findMatchingRemap( userDefinedRemappings, @@ -92,8 +91,8 @@ export class Remapper implements IRemapper { ); if (remapping) { - logger.debug( - `Remapper: ${this._configKey}. match found. before=${remapping.before}. after=${ + this._logger.debug( + `${this._configKey}. match found. before=${remapping.before}. after=${ remapping.after }. command=${remapping.commands}.` ); @@ -191,7 +190,6 @@ export class Remapper implements IRemapper { debugMsg += `after=${remapping.after}. `; } - let isCommandValid = true; if (remapping.commands) { for (const command of remapping.commands) { let cmd: string; @@ -206,31 +204,11 @@ export class Remapper implements IRemapper { } debugMsg += `command=${cmd}. args=${args}.`; - - if (!configurationValidator.isCommandValid(cmd)) { - debugMsg += ` Command does not exist.`; - isCommandValid = false; - } } } - if (!remapping.after && !remapping.commands) { - logger.error( - `Remapper: ${this._configKey}. - Invalid configuration. Missing 'after' key or 'command'. ${debugMsg}` - ); - continue; - } - - const keys = remapping.before.join(''); - if (keys in remappings) { - logger.error(`Remapper: ${this._configKey}. Duplicate configuration. ${debugMsg}`); - continue; - } - - let log = (msg: string) => (isCommandValid ? logger.debug(msg) : logger.warn(msg)); - log(`Remapper: ${this._configKey}. ${debugMsg}`); - remappings[keys] = remapping; + this._logger.debug(`${this._configKey}. ${debugMsg}`); + remappings[remapping.before.join('')] = remapping; } return remappings; diff --git a/src/history/historyFile.ts b/src/history/historyFile.ts index 46825a30f0d..31bad299e12 100644 --- a/src/history/historyFile.ts +++ b/src/history/historyFile.ts @@ -1,13 +1,14 @@ import * as fs from 'fs'; import * as path from 'path'; -import { promisify } from 'util'; +import { Logger } from '../util/logger'; import { configuration } from '../configuration/configuration'; -import { logger } from '../util/logger'; import { getExtensionDirPath } from '../util/util'; +import { promisify } from 'util'; const mkdirp = require('mkdirp'); export class HistoryFile { + private readonly _logger = Logger.get('HistoryFile'); private _historyFileName: string; private _historyDir: string; private _history: string[] = []; @@ -57,7 +58,7 @@ export class HistoryFile { this._history = []; fs.unlinkSync(this.historyFilePath); } catch (err) { - logger.warn(`historyFile: Unable to delete ${this.historyFilePath}. err=${err}.`); + this._logger.warn(`Unable to delete ${this.historyFilePath}. err=${err}.`); } } @@ -68,9 +69,9 @@ export class HistoryFile { 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}`); + this._logger.debug(`History does not exist. path=${this._historyDir}`); } else { - logger.warn(`historyFile: Failed to load history. path=${this._historyDir} err=${err}.`); + this._logger.warn(`Failed to load history. path=${this._historyDir} err=${err}.`); } return; } @@ -86,9 +87,7 @@ export class HistoryFile { } this._history = parsedData; } catch (e) { - logger.warn( - `historyFile: Deleting corrupted history file. path=${this._historyDir} err=${e}.` - ); + this._logger.warn(`Deleting corrupted history file. path=${this._historyDir} err=${e}.`); this.clear(); } } @@ -100,9 +99,7 @@ export class HistoryFile { // 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}.` - ); + this._logger.error(`Failed to save history. filepath=${this.historyFilePath}. err=${err}.`); throw err; } } diff --git a/src/history/historyTracker.ts b/src/history/historyTracker.ts index 589bc9c6426..28ae2dc7254 100644 --- a/src/history/historyTracker.ts +++ b/src/history/historyTracker.ts @@ -15,9 +15,9 @@ import * as vscode from 'vscode'; import { Position } from './../common/motion/position'; import { RecordedState } from './../state/recordedState'; +import { Logger } from './../util/logger'; import { VimState } from './../state/vimState'; import { TextEditor } from './../textEditor'; -import { logger } from './../util/logger'; const diffEngine = new DiffMatchPatch.diff_match_patch(); diffEngine.Diff_Timeout = 1; // 1 second @@ -156,6 +156,7 @@ class HistoryStep { } export class HistoryTracker { + private readonly _logger = Logger.get('DocumentChange'); public lastContentChanges: vscode.TextDocumentContentChangeEvent[]; public currentContentChanges: vscode.TextDocumentContentChangeEvent[]; @@ -183,9 +184,9 @@ export class HistoryTracker { private get currentHistoryStep(): HistoryStep { if (this.currentHistoryStepIndex === -1) { - let msg = 'HistoryTracker: Tried to modify history at index -1'; - logger.debug(msg); - throw new Error(msg); + const msg = 'Tried to modify history at index -1'; + this._logger.warn(msg); + throw new Error('HistoryTracker:' + msg); } return this.historySteps[this.currentHistoryStepIndex]; @@ -455,7 +456,7 @@ export class HistoryTracker { */ async undoAndRemoveChanges(n: number): Promise { if (this.currentHistoryStep.changes.length < n) { - logger.debug('HistoryTracker: Something bad happened in removeChange'); + this._logger.warn('Something bad happened in removeChange'); return; } diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index 319d5117a99..2f19007ba9b 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -6,6 +6,7 @@ import { BaseMovement, isIMovement } from './../actions/motion'; import { CommandInsertInInsertMode, CommandInsertPreviousText } from './../actions/commands/insert'; import { Decoration } from '../configuration/decoration'; import { Jump } from '../jumps/jump'; +import { Logger } from '../util/logger'; import { Mode, ModeName, VSCodeVimCursorType } from './mode'; import { PairMatcher } from './../common/matching/matcher'; import { Position, PositionDiff } from './../common/motion/position'; @@ -20,7 +21,6 @@ import { VsCodeContext } from '../util/vscode-context'; import { commandLine } from '../cmd_line/commandLine'; import { configuration } from '../configuration/configuration'; import { getCursorsAfterSync } from '../util/util'; -import { logger } from '../util/logger'; import { BaseCommand, CommandQuitRecordMacro, @@ -36,6 +36,7 @@ export class ModeHandler implements vscode.Disposable { private _disposables: vscode.Disposable[] = []; private _modes: Mode[]; private _remappers: Remappers; + private readonly _logger = Logger.get('ModeHandler'); public vimState: VimState; @@ -234,7 +235,7 @@ export class ModeHandler implements vscode.Disposable { public async handleKeyEvent(key: string): Promise { const now = Number(new Date()); - logger.debug(`ModeHandler: handling key=${key}.`); + this._logger.debug(`handling key=${key}.`); // rewrite copy if (configuration.overrideCopy) { @@ -301,7 +302,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}.`); + this._logger.error(`error handling key=${key}. err=${e}.`); throw e; } @@ -735,7 +736,7 @@ export class ModeHandler implements vscode.Disposable { let recordedState = vimState.recordedState; if (!recordedState.operator) { - logger.warn('recordedState.operator: ' + recordedState.operator); + this._logger.warn('recordedState.operator: ' + recordedState.operator); throw new Error("what in god's name. recordedState.operator is falsy."); } @@ -849,7 +850,7 @@ export class ModeHandler implements vscode.Disposable { case 'moveCursor': break; default: - logger.warn(`modeHandler: Unhandled text transformation type: ${command.type}.`); + this._logger.warn(`Unhandled text transformation type: ${command.type}.`); break; } @@ -868,7 +869,7 @@ export class ModeHandler implements vscode.Disposable { if (textTransformations.length > 0) { if (areAnyTransformationsOverlapping(textTransformations)) { - logger.debug( + this._logger.debug( `Text transformations are overlapping. Falling back to serial transformations. This is generally a very bad sign. Try to make your text transformations operate on non-overlapping ranges.` @@ -991,7 +992,7 @@ export class ModeHandler implements vscode.Disposable { } break; default: - logger.warn(`modeHandler: Unhandled text transformation type: ${command.type}.`); + this._logger.warn(`Unhandled text transformation type: ${command.type}.`); break; } } @@ -1258,7 +1259,7 @@ export class ModeHandler implements vscode.Disposable { break; } default: { - logger.error(`ModeHandler: unexpected selection mode. selectionMode=${selectionMode}`); + this._logger.error(`unexpected selection mode. selectionMode=${selectionMode}`); break; } } diff --git a/src/mode/modes.ts b/src/mode/modes.ts index 4b92e718629..8725b687eba 100644 --- a/src/mode/modes.ts +++ b/src/mode/modes.ts @@ -1,9 +1,9 @@ -import { Position } from './../common/motion/position'; +import { Logger } from '../util/logger'; import { Mode, ModeName } from './mode'; +import { Position } from './../common/motion/position'; +import { SearchDirection } from '../state/searchState'; import { VSCodeVimCursorType } from './mode'; import { VimState } from '../state/vimState'; -import { SearchDirection } from '../state/searchState'; -import { logger } from '../util/logger'; export enum VisualBlockInsertionType { /** @@ -60,13 +60,15 @@ export class VisualLineMode extends Mode { } export class SearchInProgressMode extends Mode { + private readonly _logger = Logger.get('SearchInProgressMode'); + constructor() { super(ModeName.SearchInProgressMode, '', VSCodeVimCursorType.Block); } getStatusBarText(vimState: VimState): string { if (vimState.globalState.searchState === undefined) { - logger.warn(`SearchInProgressMode: vimState.globalState.searchState is undefined.`); + this._logger.warn(`vimState.globalState.searchState is undefined.`); return ''; } const leadingChar = diff --git a/src/neovim/neovim.ts b/src/neovim/neovim.ts index 4f00f4acbb2..130f2512b8c 100644 --- a/src/neovim/neovim.ts +++ b/src/neovim/neovim.ts @@ -1,5 +1,6 @@ import * as util from 'util'; import * as vscode from 'vscode'; +import { Logger } from '../util/logger'; import { Position } from './../common/motion/position'; import { Register, RegisterMode } from '../register/register'; import { TextEditor } from '../textEditor'; @@ -7,13 +8,13 @@ import { VimState } from './../state/vimState'; import { configuration } from '../configuration/configuration'; import { dirname } from 'path'; import { exists } from 'fs'; -import { logger } from '../util/logger'; import { spawn, ChildProcess } from 'child_process'; import { attach, Neovim } from 'neovim'; export class NeovimWrapper implements vscode.Disposable { private process: ChildProcess; private nvim: Neovim; + private readonly logger = Logger.get('Neovim'); async run(vimState: VimState, command: string) { if (!this.nvim) { @@ -29,7 +30,7 @@ export class NeovimWrapper implements vscode.Disposable { const apiInfo = await this.nvim.apiInfo; const version = apiInfo[1].version; - logger.debug(`Neovim Version: ${version.major}.${version.minor}.${version.patch}`); + this.logger.debug(`version: ${version.major}.${version.minor}.${version.patch}`); } await this.syncVSToVim(vimState); @@ -40,7 +41,7 @@ export class NeovimWrapper implements vscode.Disposable { await this.nvim.command('let v:errmsg="" | let v:statusmsg=""'); // Execute the command - logger.debug(`Neovim: Running ${command}.`); + this.logger.debug(`Running ${command}.`); await this.nvim.input(command); const mode = await this.nvim.mode; if (mode.blocking) { @@ -67,7 +68,7 @@ export class NeovimWrapper implements vscode.Disposable { } private async startNeovim() { - logger.debug('Neovim: Spawning Neovim process...'); + this.logger.debug('Spawning Neovim process...'); let dir = dirname(vscode.window.activeTextEditor!.document.uri.fsPath); if (!(await util.promisify(exists)(dir))) { dir = __dirname; @@ -77,7 +78,7 @@ export class NeovimWrapper implements vscode.Disposable { }); this.process.on('error', err => { - logger.error(`Neovim: Error spawning neovim. Error=${err.message}.`); + this.logger.error(`Error spawning neovim. Error=${err.message}.`); configuration.enableNeovim = false; }); return attach({ proc: this.process }); @@ -150,7 +151,7 @@ export class NeovimWrapper implements vscode.Disposable { fixedLines.join('\n') ); - logger.debug(`Neovim: ${lines.length} lines in nvim. ${TextEditor.getLineCount()} in editor.`); + this.logger.debug(`${lines.length} lines in nvim. ${TextEditor.getLineCount()} in editor.`); let [row, character] = ((await this.nvim.callFunction('getpos', ['.'])) as Array).slice( 1, diff --git a/src/state/vimState.ts b/src/state/vimState.ts index 6f11bc890a8..06fe1c23e1b 100644 --- a/src/state/vimState.ts +++ b/src/state/vimState.ts @@ -5,6 +5,7 @@ import { EasyMotion } from './../actions/plugins/easymotion/easymotion'; import { EditorIdentity } from './../editorIdentity'; import { HistoryTracker } from './../history/historyTracker'; import { InputMethodSwitcher } from '../actions/plugins/imswitcher'; +import { Logger } from '../util/logger'; import { ModeName } from '../mode/mode'; import { NeovimWrapper } from '../neovim/neovim'; import { Position } from './../common/motion/position'; @@ -13,7 +14,6 @@ import { RecordedState } from './recordedState'; import { RegisterMode } from './../register/register'; import { ReplaceState } from './../state/replaceState'; import { globalState } from './../state/globalState'; -import { logger } from '../util/logger'; /** * The VimState class holds permanent state that carries over from action @@ -23,6 +23,8 @@ import { logger } from '../util/logger'; * indicate what they want to do. */ export class VimState implements vscode.Disposable { + private readonly logger = Logger.get('VimState'); + /** * The column the cursor wants to be at, or Number.POSITIVE_INFINITY if it should always * be the rightmost column. @@ -151,7 +153,7 @@ export class VimState implements vscode.Disposable { public set allCursors(value: Range[]) { for (const cursor of value) { if (!cursor.start.isValid(this.editor) || !cursor.stop.isValid(this.editor)) { - logger.debug('VimState: invalid value for set cursor position. This is probably bad?'); + this.logger.debug('invalid value for set cursor position.'); } } diff --git a/src/taskQueue.ts b/src/taskQueue.ts index 6f2acda9ea5..8a1eac0e856 100644 --- a/src/taskQueue.ts +++ b/src/taskQueue.ts @@ -1,4 +1,5 @@ import * as _ from 'lodash'; +import { Logger } from './util/logger'; interface IEnqueuedTask { promise: () => Promise; @@ -8,7 +9,8 @@ interface IEnqueuedTask { } class TaskQueue { - private _taskQueue: { + private readonly _logger = Logger.get('TaskQueue'); + private readonly _taskQueue: { [key: string]: { tasks: IEnqueuedTask[]; }; @@ -37,7 +39,7 @@ class TaskQueue { await task.promise(); task.isRunning = false; } catch (e) { - console.error(`TaskQueue: error running task. err=${e}.`); + this._logger.error(`Error running task. err=${e}.`); } finally { this.dequeueTask(task); } diff --git a/src/util/clipboard.ts b/src/util/clipboard.ts index 377a0654dcf..cdff7a16192 100644 --- a/src/util/clipboard.ts +++ b/src/util/clipboard.ts @@ -1,12 +1,14 @@ -import { logger } from './logger'; import * as vscode from 'vscode'; +import { Logger } from './logger'; export class Clipboard { + private static readonly logger = Logger.get('Clipboard'); + public static async Copy(text: string): Promise { try { await vscode.env.clipboard.writeText(text); } catch (e) { - logger.error(e, `Clipboard: Error copying to clipboard. err=${e}`); + this.logger.error(e, `Error copying to clipboard. err=${e}`); } } diff --git a/src/util/logger.ts b/src/util/logger.ts index f1b6462d08e..e13156650af 100644 --- a/src/util/logger.ts +++ b/src/util/logger.ts @@ -4,18 +4,28 @@ import * as winston from 'winston'; import { ConsoleForElectron } from 'winston-console-for-electron'; import { configuration } from '../configuration/configuration'; +interface VsCodeMessageOptions extends TransportStream.TransportStreamOptions { + section?: string; +} + /** * Implementation of Winston transport * Displays VS Code message to user */ class VsCodeMessage extends TransportStream { + section?: string; actionMessages = ['Dismiss', 'Suppress Errors']; + constructor(options: VsCodeMessageOptions) { + super(options); + + this.section = options.section; + } + public async log(info: { level: string; message: string }, callback: Function) { if (configuration.debug.silent) { return; } - let showMessage: (message: string, ...items: string[]) => Thenable; switch (info.level) { case 'error': @@ -33,7 +43,12 @@ class VsCodeMessage extends TransportStream { throw 'Unsupported ' + info.level; } - let selectedAction = await showMessage(info.message, ...this.actionMessages); + let message = info.message; + if (this.section) { + message = this.section + ': ' + message; + } + + let selectedAction = await showMessage(message, ...this.actionMessages); if (selectedAction === 'Suppress Errors') { vscode.window.showInformationMessage( 'Ignorance is bliss; temporarily suppressing log messages. For more permanence, please configure `vim.debug.suppress`.' @@ -47,15 +62,20 @@ class VsCodeMessage extends TransportStream { } } -export const logger = winston.createLogger({ - format: winston.format.simple(), - transports: [ - new ConsoleForElectron({ - level: configuration.debug.loggingLevelForConsole, - silent: configuration.debug.silent, - }), - new VsCodeMessage({ - level: configuration.debug.loggingLevelForAlert, - }), - ], -}); +export class Logger { + static get(section?: string): winston.Logger { + return winston.createLogger({ + format: winston.format.simple(), + transports: [ + new ConsoleForElectron({ + level: configuration.debug.loggingLevelForConsole, + silent: configuration.debug.silent, + }), + new VsCodeMessage({ + level: configuration.debug.loggingLevelForAlert, + section: section, + }), + ], + }); + } +} diff --git a/src/util/util.ts b/src/util/util.ts index a14ac3bc6c2..9da0842ef1c 100644 --- a/src/util/util.ts +++ b/src/util/util.ts @@ -1,11 +1,10 @@ import * as vscode from 'vscode'; +import AppDirectory = require('appdirectory'); +import { Logger } from './logger'; import { Position } from '../common/motion/position'; import { Range } from '../common/motion/range'; -import { logger } from './logger'; import { exec } from 'child_process'; -const AppDirectory = require('appdirectory'); - /** * This is certainly quite janky! The problem we're trying to solve * is that writing editor.selection = new Position() won't immediately @@ -27,6 +26,7 @@ export async function waitForCursorSync( } export async function getCursorsAfterSync(timeoutInMilliseconds: number = 0): Promise { + const logger = Logger.get('getCursorsAfterSync'); try { await waitForCursorSync(timeoutInMilliseconds, true); } catch (e) { @@ -39,7 +39,9 @@ export async function getCursorsAfterSync(timeoutInMilliseconds: number = 0): Pr } export function getExtensionDirPath(): string { + const logger = Logger.get('getExtensionDirPath'); const dirs = new AppDirectory('VSCodeVim'); + logger.debug('VSCodeVim Cache Directory: ' + dirs.userCache()); return dirs.userCache(); diff --git a/test/testUtils.ts b/test/testUtils.ts index 820e5047db4..36bb4f0aa3c 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -135,7 +135,7 @@ export async function cleanUpWorkspace(): Promise { } export function reloadConfiguration() { - require('../src/configuration/configuration').configuration.reload(); + require('../src/configuration/configuration').configuration.load(); } /** From 821e158b47397cf1f7938b2fc780863bd243f5df Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Mon, 28 Jan 2019 23:43:52 -0800 Subject: [PATCH 2/7] winston-console-for-electron 0.0.5->0.0.6 --- package-lock.json | 32 ++++++++++++++++---------------- package.json | 2 +- src/util/logger.ts | 15 ++++++++------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index c27b4006921..1c0f8c51d56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1068,7 +1068,7 @@ }, "deep-assign": { "version": "1.0.0", - "resolved": "https://registry.npmjs.org/deep-assign/-/deep-assign-1.0.0.tgz", + "resolved": "http://registry.npmjs.org/deep-assign/-/deep-assign-1.0.0.tgz", "integrity": "sha1-sJJ0O+hCfcYh6gBnzex+cN0Z83s=", "dev": true, "requires": { @@ -1190,7 +1190,7 @@ }, "duplexer": { "version": "0.1.1", - "resolved": "https://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", + "resolved": "http://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", "integrity": "sha1-rOb/gIwc5mtX0ev5eXessCM0z8E=", "dev": true }, @@ -1367,7 +1367,7 @@ }, "event-stream": { "version": "3.3.4", - "resolved": "https://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", + "resolved": "http://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", "integrity": "sha1-SrTJoPWlTbkzi0w02Gv86PSzVXE=", "dev": true, "requires": { @@ -2548,7 +2548,7 @@ }, "kind-of": { "version": "1.1.0", - "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-1.1.0.tgz", + "resolved": "http://registry.npmjs.org/kind-of/-/kind-of-1.1.0.tgz", "integrity": "sha1-FAo9LUGjbS78+pN3tiwk+ElaXEQ=", "dev": true }, @@ -2601,7 +2601,7 @@ }, "readable-stream": { "version": "1.0.34", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", "integrity": "sha1-Elgg40vIQtLyqq+v5MKRbuMsFXw=", "dev": true, "requires": { @@ -2613,13 +2613,13 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=", "dev": true }, "through2": { "version": "0.6.5", - "resolved": "https://registry.npmjs.org/through2/-/through2-0.6.5.tgz", + "resolved": "http://registry.npmjs.org/through2/-/through2-0.6.5.tgz", "integrity": "sha1-QaucZ7KdVyCQcUEOHXp6lozTrUg=", "dev": true, "requires": { @@ -3188,7 +3188,7 @@ }, "is-obj": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", "integrity": "sha1-PkcprB9f3gJc19g6iW2rn09n2w8=", "dev": true }, @@ -4206,7 +4206,7 @@ }, "pause-stream": { "version": "0.0.11", - "resolved": "https://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", + "resolved": "http://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", "integrity": "sha1-/lo0sMvOErWqaitAPuLnO2AvFEU=", "dev": true, "requires": { @@ -4951,7 +4951,7 @@ }, "split": { "version": "0.3.3", - "resolved": "https://registry.npmjs.org/split/-/split-0.3.3.tgz", + "resolved": "http://registry.npmjs.org/split/-/split-0.3.3.tgz", "integrity": "sha1-zQ7qXmOiEd//frDwkcQTPi0N0o8=", "dev": true, "requires": { @@ -5024,7 +5024,7 @@ }, "stream-combiner": { "version": "0.0.4", - "resolved": "https://registry.npmjs.org/stream-combiner/-/stream-combiner-0.0.4.tgz", + "resolved": "http://registry.npmjs.org/stream-combiner/-/stream-combiner-0.0.4.tgz", "integrity": "sha1-TV5DPBhSYd3mI8o/RMWGvPXErRQ=", "dev": true, "requires": { @@ -5138,7 +5138,7 @@ }, "tar": { "version": "2.2.1", - "resolved": "https://registry.npmjs.org/tar/-/tar-2.2.1.tgz", + "resolved": "http://registry.npmjs.org/tar/-/tar-2.2.1.tgz", "integrity": "sha1-jk0qJWwOIYXGsYrWlK7JaLg8sdE=", "dev": true, "requires": { @@ -5149,7 +5149,7 @@ }, "text-encoding": { "version": "0.6.4", - "resolved": "https://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", + "resolved": "http://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", "integrity": "sha1-45mpgiV6J22uQou5KEXLcb3CbRk=", "dev": true }, @@ -5872,9 +5872,9 @@ } }, "winston-console-for-electron": { - "version": "0.0.5", - "resolved": "https://registry.npmjs.org/winston-console-for-electron/-/winston-console-for-electron-0.0.5.tgz", - "integrity": "sha512-LsvVdbVwkLERVCIxSSbLo/9XJc3d6FM0MMvEFZBOhBlzEqCBSHL8edt7H9BSgYfIG6JStEvwcrQRoLa6dul8qA==", + "version": "0.0.6", + "resolved": "https://registry.npmjs.org/winston-console-for-electron/-/winston-console-for-electron-0.0.6.tgz", + "integrity": "sha512-4oc83FpZ04CC+Rne1ljh6ysl/fzUbE7fmIacjgh+1CDU3xBCpgqfNYRrcw6rL0yTOCJ8e4Fk2CYnOgBhWao1iA==", "requires": { "winston-transport": "4.1.0" } diff --git a/package.json b/package.json index 33b144fae91..445a3da5fd0 100644 --- a/package.json +++ b/package.json @@ -694,7 +694,7 @@ "neovim": "4.2.1", "untildify": "3.0.3", "winston": "3.1.0", - "winston-console-for-electron": "0.0.5" + "winston-console-for-electron": "0.0.6" }, "devDependencies": { "@types/diff": "3.5.2", diff --git a/src/util/logger.ts b/src/util/logger.ts index e13156650af..1232d6ebaca 100644 --- a/src/util/logger.ts +++ b/src/util/logger.ts @@ -5,7 +5,7 @@ import { ConsoleForElectron } from 'winston-console-for-electron'; import { configuration } from '../configuration/configuration'; interface VsCodeMessageOptions extends TransportStream.TransportStreamOptions { - section?: string; + prefix?: string; } /** @@ -13,13 +13,13 @@ interface VsCodeMessageOptions extends TransportStream.TransportStreamOptions { * Displays VS Code message to user */ class VsCodeMessage extends TransportStream { - section?: string; + prefix?: string; actionMessages = ['Dismiss', 'Suppress Errors']; constructor(options: VsCodeMessageOptions) { super(options); - this.section = options.section; + this.prefix = options.prefix; } public async log(info: { level: string; message: string }, callback: Function) { @@ -44,8 +44,8 @@ class VsCodeMessage extends TransportStream { } let message = info.message; - if (this.section) { - message = this.section + ': ' + message; + if (this.prefix) { + message = this.prefix + ': ' + message; } let selectedAction = await showMessage(message, ...this.actionMessages); @@ -63,17 +63,18 @@ class VsCodeMessage extends TransportStream { } export class Logger { - static get(section?: string): winston.Logger { + static get(prefix?: string): winston.Logger { return winston.createLogger({ format: winston.format.simple(), transports: [ new ConsoleForElectron({ level: configuration.debug.loggingLevelForConsole, silent: configuration.debug.silent, + prefix: prefix, }), new VsCodeMessage({ level: configuration.debug.loggingLevelForAlert, - section: section, + prefix: prefix, }), ], }); From 178da8794cfa7e1ad92434ec05dc30800c38424b Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Tue, 29 Jan 2019 00:51:20 -0800 Subject: [PATCH 3/7] refactor: create keybinding map once instead of on each key press --- src/configuration/configuration.ts | 39 ++++++++++++++++++--------- src/configuration/remapper.ts | 43 +++--------------------------- 2 files changed, 30 insertions(+), 52 deletions(-) diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts index 50fade31de3..dcd73e44600 100644 --- a/src/configuration/configuration.ts +++ b/src/configuration/configuration.ts @@ -87,21 +87,23 @@ class Configuration implements IConfiguration { this.leader = Notation.NormalizeKey(this.leader, this.leaderDefault); - // normalize remapped keys - const keybindingList: IKeyRemapping[][] = [ - this.insertModeKeyBindings, - this.insertModeKeyBindingsNonRecursive, - this.normalModeKeyBindings, - this.normalModeKeyBindingsNonRecursive, - this.visualModeKeyBindings, - this.visualModeKeyBindingsNonRecursive, + // remapped keys + const modeKeyBindingsKeys = [ + 'insertModeKeyBindings', + 'insertModeKeyBindingsNonRecursive', + 'normalModeKeyBindings', + 'normalModeKeyBindingsNonRecursive', + 'visualModeKeyBindings', + 'visualModeKeyBindingsNonRecursive', ]; - for (const keybindings of keybindingList) { - const keybindingMap: { [key: string]: {} } = Object.create(null); + for (const modeKeyBindingsKey of modeKeyBindingsKeys) { + let keybindings = configuration[modeKeyBindingsKey]; + const modeKeyBindingsMap = new Map(); for (let i = keybindings.length - 1; i >= 0; i--) { let remapping = keybindings[i]; + // validate let remappingErrors = await configurationValidator.isRemappingValid(remapping); configurationErrors = configurationErrors.concat(remappingErrors); @@ -111,6 +113,7 @@ class Configuration implements IConfiguration { continue; } + // normalize if (remapping.before) { remapping.before.forEach( (key, idx) => (remapping.before[idx] = Notation.NormalizeKey(key, this.leader)) @@ -124,16 +127,19 @@ class Configuration implements IConfiguration { } const keys = remapping.before.join(''); - if (keys in keybindingMap) { + if (keys in modeKeyBindingsMap) { configurationErrors.push({ level: 'error', message: `${remapping.before}. Duplicate remapped key for ${keys}.`, }); continue; - } else { - keybindingMap[keys] = {}; } + + // add to map + modeKeyBindingsMap[keys] = remapping; } + + configuration[modeKeyBindingsKey + 'Map'] = modeKeyBindingsMap; } // wrap keys @@ -359,6 +365,13 @@ class Configuration implements IConfiguration { visualModeKeyBindings: IKeyRemapping[] = []; visualModeKeyBindingsNonRecursive: IKeyRemapping[] = []; + insertModeKeyBindingsMap: Map; + insertModeKeyBindingsNonRecursiveMap: Map; + normalModeKeyBindingsMap: Map; + normalModeKeyBindingsNonRecursiveMap: Map; + visualModeKeyBindingsMap: Map; + visualModeKeyBindingsNonRecursiveMap: Map; + private static unproxify(obj: Object): Object { let result = {}; for (const key in obj) { diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index 0a052838801..ff905e59793 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -79,7 +79,7 @@ export class Remapper implements IRemapper { return false; } - const userDefinedRemappings = this._getRemappings(); + const userDefinedRemappings = configuration[this._configKey]; this._logger.debug( `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}.` @@ -179,41 +179,6 @@ export class Remapper implements IRemapper { } } - private _getRemappings(): { [key: string]: IKeyRemapping } { - // Create a null object so that there is no __proto__ - let remappings: { [key: string]: IKeyRemapping } = Object.create(null); - - for (let remapping of configuration[this._configKey] as IKeyRemapping[]) { - let debugMsg = `before=${remapping.before}. `; - - if (remapping.after) { - debugMsg += `after=${remapping.after}. `; - } - - if (remapping.commands) { - for (const command of remapping.commands) { - let cmd: string; - let args: any[]; - - if (typeof command === 'string') { - cmd = command; - args = []; - } else { - cmd = command.command; - args = command.args; - } - - debugMsg += `command=${cmd}. args=${args}.`; - } - } - - this._logger.debug(`${this._configKey}. ${debugMsg}`); - remappings[remapping.before.join('')] = remapping; - } - - return remappings; - } - protected static findMatchingRemap( userDefinedRemappings: { [key: string]: IKeyRemapping }, inputtedKeys: string[], @@ -266,7 +231,7 @@ export class Remapper implements IRemapper { class InsertModeRemapper extends Remapper { constructor(recursive: boolean) { super( - 'insertModeKeyBindings' + (recursive ? '' : 'NonRecursive'), + 'insertModeKeyBindings' + (recursive ? '' : 'NonRecursive') + 'Map', [ModeName.Insert, ModeName.Replace], recursive ); @@ -276,7 +241,7 @@ class InsertModeRemapper extends Remapper { class NormalModeRemapper extends Remapper { constructor(recursive: boolean) { super( - 'normalModeKeyBindings' + (recursive ? '' : 'NonRecursive'), + 'normalModeKeyBindings' + (recursive ? '' : 'NonRecursive') + 'Map', [ModeName.Normal], recursive ); @@ -286,7 +251,7 @@ class NormalModeRemapper extends Remapper { class VisualModeRemapper extends Remapper { constructor(recursive: boolean) { super( - 'visualModeKeyBindings' + (recursive ? '' : 'NonRecursive'), + 'visualModeKeyBindings' + (recursive ? '' : 'NonRecursive') + 'Map', [ModeName.Visual, ModeName.VisualLine, ModeName.VisualBlock], recursive ); From 3ab82a0b75b26b6a4a380fe88d57e0745bd4cfa7 Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Tue, 29 Jan 2019 01:51:23 -0800 Subject: [PATCH 4/7] fix: if no userdefined remappings, exit early --- src/actions/motion.ts | 35 ++++++----------------------- src/actions/plugins/sneak.ts | 10 ++------- src/configuration/remapper.ts | 32 +++++++++++++------------- test/configuration/remapper.test.ts | 20 ++++++++--------- 4 files changed, 35 insertions(+), 62 deletions(-) diff --git a/src/actions/motion.ts b/src/actions/motion.ts index 8126b9297f5..e165f18a67e 100644 --- a/src/actions/motion.ts +++ b/src/actions/motion.ts @@ -663,14 +663,8 @@ class MoveFindForward extends BaseMovement { !this.isRepeat && (!vimState.recordedState.operator || !(isIMovement(result) && result.failed)) ) { - vimState.lastSemicolonRepeatableMovement = new MoveFindForward( - this.keysPressed, - true - ); - vimState.lastCommaRepeatableMovement = new MoveFindBackward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new MoveFindForward(this.keysPressed, true); + vimState.lastCommaRepeatableMovement = new MoveFindBackward(this.keysPressed, true); } return result; @@ -698,14 +692,8 @@ class MoveFindBackward extends BaseMovement { !this.isRepeat && (!vimState.recordedState.operator || !(isIMovement(result) && result.failed)) ) { - vimState.lastSemicolonRepeatableMovement = new MoveFindBackward( - this.keysPressed, - true - ); - vimState.lastCommaRepeatableMovement = new MoveFindForward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new MoveFindBackward(this.keysPressed, true); + vimState.lastCommaRepeatableMovement = new MoveFindForward(this.keysPressed, true); } return result; @@ -742,14 +730,8 @@ class MoveTilForward extends BaseMovement { !this.isRepeat && (!vimState.recordedState.operator || !(isIMovement(result) && result.failed)) ) { - vimState.lastSemicolonRepeatableMovement = new MoveTilForward( - this.keysPressed, - true - ); - vimState.lastCommaRepeatableMovement = new MoveTilBackward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new MoveTilForward(this.keysPressed, true); + vimState.lastCommaRepeatableMovement = new MoveTilBackward(this.keysPressed, true); } return result; @@ -782,10 +764,7 @@ class MoveTilBackward extends BaseMovement { !this.isRepeat && (!vimState.recordedState.operator || !(isIMovement(result) && result.failed)) ) { - vimState.lastSemicolonRepeatableMovement = new MoveTilBackward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new MoveTilBackward(this.keysPressed, true); vimState.lastCommaRepeatableMovement = new MoveTilForward(this.keysPressed, true); } diff --git a/src/actions/plugins/sneak.ts b/src/actions/plugins/sneak.ts index 48e6d09e873..c14fb0300e6 100644 --- a/src/actions/plugins/sneak.ts +++ b/src/actions/plugins/sneak.ts @@ -21,10 +21,7 @@ class SneakForward extends BaseMovement { public async execAction(position: Position, vimState: VimState): Promise { if (!this.isRepeat) { - vimState.lastSemicolonRepeatableMovement = new SneakForward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new SneakForward(this.keysPressed, true); vimState.lastCommaRepeatableMovement = new SneakBackward(this.keysPressed, true); } @@ -79,10 +76,7 @@ class SneakBackward extends BaseMovement { public async execAction(position: Position, vimState: VimState): Promise { if (!this.isRepeat) { - vimState.lastSemicolonRepeatableMovement = new SneakBackward( - this.keysPressed, - true - ); + vimState.lastSemicolonRepeatableMovement = new SneakBackward(this.keysPressed, true); vimState.lastCommaRepeatableMovement = new SneakForward(this.keysPressed, true); } diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index ff905e59793..dd4cdf5456c 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -79,7 +79,7 @@ export class Remapper implements IRemapper { return false; } - const userDefinedRemappings = configuration[this._configKey]; + const userDefinedRemappings = configuration[this._configKey] as Map; this._logger.debug( `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}.` @@ -180,13 +180,15 @@ export class Remapper implements IRemapper { } protected static findMatchingRemap( - userDefinedRemappings: { [key: string]: IKeyRemapping }, + userDefinedRemappings: Map, inputtedKeys: string[], currentMode: ModeName - ) { - let remapping: IKeyRemapping | undefined; + ): IKeyRemapping | undefined { + if (userDefinedRemappings.size === 0) { + return; + } - let range = Remapper.getRemappedKeysLengthRange(userDefinedRemappings); + const range = Remapper.getRemappedKeysLengthRange(userDefinedRemappings); const startingSliceLength = Math.max(range[1], inputtedKeys.length); for (let sliceLength = startingSliceLength; sliceLength >= range[0]; sliceLength--) { const keySlice = inputtedKeys.slice(-sliceLength).join(''); @@ -205,26 +207,24 @@ export class Remapper implements IRemapper { } } - remapping = userDefinedRemappings[keySlice]; - break; + return userDefinedRemappings[keySlice]; } } - return remapping; + return; } /** * Given list of remappings, returns the length of the shortest and longest remapped keys * @param remappings */ - protected static getRemappedKeysLengthRange(remappings: { - [key: string]: IKeyRemapping; - }): [number, number] { - const keys = Object.keys(remappings); - if (keys.length === 0) { - return [0, 0]; - } - return [_.minBy(keys, m => m.length)!.length, _.maxBy(keys, m => m.length)!.length]; + protected static getRemappedKeysLengthRange( + remappings: Map + ): [number, number] { + return [ + _.minBy(Array.from(remappings.keys()), m => m.length)!.length, + _.maxBy(Array.from(remappings.keys()), m => m.length)!.length, + ]; } } diff --git a/test/configuration/remapper.test.ts b/test/configuration/remapper.test.ts index 94e962c4d23..b63302c51f5 100644 --- a/test/configuration/remapper.test.ts +++ b/test/configuration/remapper.test.ts @@ -73,16 +73,16 @@ suite('Remapper', () => { } public findMatchingRemap( - userDefinedRemappings: { [key: string]: IKeyRemapping }, + userDefinedRemappings: Map, inputtedKeys: string[], currentMode: ModeName ) { return TestRemapper.findMatchingRemap(userDefinedRemappings, inputtedKeys, currentMode); } - public getRemappedKeySequenceLengthRange(remappings: { - [key: string]: IKeyRemapping; - }): [number, number] { + public getRemappedKeySequenceLengthRange( + remappings: Map + ): [number, number] { return TestRemapper.getRemappedKeysLengthRange(remappings); } } @@ -117,11 +117,11 @@ suite('Remapper', () => { visualModeKeyBindings: defaultVisualModeKeyBindings, }); - let remappings: { [key: string]: IKeyRemapping } = { - abc: { before: ['a', 'b', 'c'] }, - de: { before: ['d', 'e'] }, - f: { before: ['f'] }, - }; + let remappings: Map = new Map([ + ['abc', { before: ['a', 'b', 'c'] }], + ['de', { before: ['d', 'e'] }], + ['f', { before: ['f'] }], + ]); // act const testRemapper = new TestRemapper(); @@ -202,7 +202,7 @@ suite('Remapper', () => { for (const testCase of testCases) { // setup - let remappings: { [key: string]: IKeyRemapping } = {}; + let remappings: Map = new Map(); remappings[testCase.before] = { before: testCase.before.split(''), after: testCase.after.split(''), From 04de9eaca75bf5c2f6ecb9878d6e5d4b5e033204 Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Tue, 29 Jan 2019 02:37:08 -0800 Subject: [PATCH 5/7] fix: i need to learn how to use a map --- src/configuration/configuration.ts | 4 ++-- src/configuration/configurationValidator.ts | 2 +- src/configuration/remapper.ts | 18 +++++++++++++----- test/configuration/remapper.test.ts | 6 +++--- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts index dcd73e44600..73250fb3adf 100644 --- a/src/configuration/configuration.ts +++ b/src/configuration/configuration.ts @@ -127,7 +127,7 @@ class Configuration implements IConfiguration { } const keys = remapping.before.join(''); - if (keys in modeKeyBindingsMap) { + if (modeKeyBindingsMap.has(keys)) { configurationErrors.push({ level: 'error', message: `${remapping.before}. Duplicate remapped key for ${keys}.`, @@ -136,7 +136,7 @@ class Configuration implements IConfiguration { } // add to map - modeKeyBindingsMap[keys] = remapping; + modeKeyBindingsMap.set(keys, remapping); } configuration[modeKeyBindingsKey + 'Map'] = modeKeyBindingsMap; diff --git a/src/configuration/configurationValidator.ts b/src/configuration/configurationValidator.ts index ed94292241e..dd2a3879e1c 100644 --- a/src/configuration/configurationValidator.ts +++ b/src/configuration/configurationValidator.ts @@ -10,7 +10,7 @@ class ConfigurationValidator { return true; } - return (await this.getCommandMap()).get(command) || false; + return (await this.getCommandMap()).has(command); } public async isRemappingValid(remapping: IKeyRemapping): Promise { diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index dd4cdf5456c..73c2e90e4f6 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -82,7 +82,9 @@ export class Remapper implements IRemapper { const userDefinedRemappings = configuration[this._configKey] as Map; this._logger.debug( - `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}.` + `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}. keybindings=${ + this._configKey + }.` ); let remapping: IKeyRemapping | undefined = Remapper.findMatchingRemap( userDefinedRemappings, @@ -184,8 +186,10 @@ export class Remapper implements IRemapper { inputtedKeys: string[], currentMode: ModeName ): IKeyRemapping | undefined { + let remapping: IKeyRemapping | undefined; + if (userDefinedRemappings.size === 0) { - return; + return remapping; } const range = Remapper.getRemappedKeysLengthRange(userDefinedRemappings); @@ -193,7 +197,7 @@ export class Remapper implements IRemapper { for (let sliceLength = startingSliceLength; sliceLength >= range[0]; sliceLength--) { const keySlice = inputtedKeys.slice(-sliceLength).join(''); - if (keySlice in userDefinedRemappings) { + if (userDefinedRemappings.has(keySlice)) { // In Insert mode, we allow users to precede remapped commands // with extraneous keystrokes (eg. "hello world jj") // In other modes, we have to precisely match the keysequence @@ -207,11 +211,12 @@ export class Remapper implements IRemapper { } } - return userDefinedRemappings[keySlice]; + remapping = userDefinedRemappings.get(keySlice); + break; } } - return; + return remapping; } /** @@ -221,6 +226,9 @@ export class Remapper implements IRemapper { protected static getRemappedKeysLengthRange( remappings: Map ): [number, number] { + if (remappings.size === 0) { + return [0, 0]; + } return [ _.minBy(Array.from(remappings.keys()), m => m.length)!.length, _.maxBy(Array.from(remappings.keys()), m => m.length)!.length, diff --git a/test/configuration/remapper.test.ts b/test/configuration/remapper.test.ts index b63302c51f5..87c2f75bb9d 100644 --- a/test/configuration/remapper.test.ts +++ b/test/configuration/remapper.test.ts @@ -203,10 +203,10 @@ suite('Remapper', () => { for (const testCase of testCases) { // setup let remappings: Map = new Map(); - remappings[testCase.before] = { + remappings.set(testCase.before, { before: testCase.before.split(''), after: testCase.after.split(''), - }; + }); // act const testRemapper = new TestRemapper(); @@ -221,7 +221,7 @@ suite('Remapper', () => { assert( actual, `Expected remap for before=${testCase.before}. input=${testCase.input}. mode=${ - testCase.mode + ModeName[testCase.mode] }.` ); assert.deepEqual(actual!.after, testCase.expectedAfter.split('')); From f5855bade9b8d63ecb52aeb11c7f2c19e89b98cf Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Tue, 29 Jan 2019 03:16:22 -0800 Subject: [PATCH 6/7] fix: add configuration tests --- src/configuration/remapper.ts | 8 ++--- src/mode/modeHandler.ts | 7 ++-- test/cmd_line/substitute.test.ts | 4 +-- test/configuration/configuration.test.ts | 44 ++++++++++++++++++++---- test/mode/modeVisual.test.ts | 4 +-- test/plugins/sneak.test.ts | 2 +- test/testUtils.ts | 6 ++-- 7 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index 73c2e90e4f6..e9ef7452c86 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -82,9 +82,9 @@ export class Remapper implements IRemapper { const userDefinedRemappings = configuration[this._configKey] as Map; this._logger.debug( - `find matching remap. keys=${keys}. mode=${ModeName[vimState.currentMode]}. keybindings=${ - this._configKey - }.` + `trying to find matching remap. keys=${keys}. mode=${ + ModeName[vimState.currentMode] + }. keybindings=${this._configKey}.` ); let remapping: IKeyRemapping | undefined = Remapper.findMatchingRemap( userDefinedRemappings, @@ -113,7 +113,7 @@ export class Remapper implements IRemapper { } // Check to see if a remapping could potentially be applied when more keys are received - for (let remap of Object.keys(userDefinedRemappings)) { + for (let remap of userDefinedRemappings.keys()) { if (keys.join('') === remap.slice(0, keys.length)) { this._isPotentialRemap = true; break; diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index 2f19007ba9b..41935dfec76 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -313,20 +313,19 @@ export class ModeHandler implements vscode.Disposable { } private async handleKeyEventHelper(key: string, vimState: VimState): Promise { - // Just nope right out of here. if (vscode.window.activeTextEditor !== this.vimState.editor) { + this._logger.warn('Current window is not active'); return this.vimState; } // Catch any text change not triggered by us (example: tab completion). vimState.historyTracker.addChange(this.vimState.cursorPositionJustBeforeAnythingHappened); - let recordedState = vimState.recordedState; + vimState.keyHistory.push(key); + let recordedState = vimState.recordedState; recordedState.actionKeys.push(key); - vimState.keyHistory.push(key); - let result = Actions.getRelevantAction(recordedState.actionKeys, vimState); switch (result) { case KeypressState.NoPossibleMatch: diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 2f9dbda91ec..97b520410d8 100644 --- a/test/cmd_line/substitute.test.ts +++ b/test/cmd_line/substitute.test.ts @@ -220,9 +220,9 @@ suite('Basic substitute', () => { }); suite('Effects of substituteGlobalFlag=true', () => { - setup(() => { + setup(async () => { Globals.mockConfiguration.substituteGlobalFlag = true; - reloadConfiguration(); + await reloadConfiguration(); }); test('Replace all matches in the line', async () => { diff --git a/test/configuration/configuration.test.ts b/test/configuration/configuration.test.ts index f64d799f3d2..4cc4fae89bf 100644 --- a/test/configuration/configuration.test.ts +++ b/test/configuration/configuration.test.ts @@ -1,5 +1,4 @@ import * as assert from 'assert'; - import * as srcConfiguration from '../../src/configuration/configuration'; import * as testConfiguration from '../testConfiguration'; import { cleanUpWorkspace, setupWorkspace } from './../testUtils'; @@ -20,6 +19,23 @@ suite('Configuration', () => { after: ['v'], }, ]; + (configuration.visualModeKeyBindingsNonRecursive = [ + // duplicate keybindings + { + before: ['c', 'o', 'p', 'y'], + after: ['c', 'o', 'p', 'y'], + }, + { + before: ['c', 'o', 'p', 'y'], + after: ['c', 'o', 'p', 'y'], + }, + ]), + (configuration.insertModeKeyBindingsNonRecursive = [ + { + // missing after and command + before: ['a'], + }, + ]); configuration.whichwrap = 'h,l'; setup(async () => { @@ -30,19 +46,24 @@ suite('Configuration', () => { test('remappings are normalized', async () => { const normalizedKeybinds = srcConfiguration.configuration.normalModeKeyBindingsNonRecursive; + const normalizedKeybindsMap = + srcConfiguration.configuration.normalModeKeyBindingsNonRecursiveMap; const testingKeybinds = configuration.normalModeKeyBindingsNonRecursive; assert.equal(normalizedKeybinds.length, testingKeybinds.length); + assert.equal(normalizedKeybinds.length, normalizedKeybindsMap.size); assert.deepEqual(normalizedKeybinds[0].before, [' ', 'o']); assert.deepEqual(normalizedKeybinds[0].after, ['o', '', 'k']); }); - newTest({ - title: 'Can handle long key chords', - start: ['|'], - keysPressed: ' fes', - end: ['|'], - endMode: ModeName.Visual, + test('remappings are de-duped', async () => { + const keybindings = srcConfiguration.configuration.visualModeKeyBindingsNonRecursiveMap; + assert.equal(keybindings.size, 1); + }); + + test('invalid remappings are ignored', async () => { + const keybindings = srcConfiguration.configuration.insertModeKeyBindingsNonRecursiveMap; + assert.equal(keybindings.size, 0); }); test('whichwrap is parsed into wrapKeys', async () => { @@ -54,4 +75,13 @@ suite('Configuration', () => { assert.equal(wrapKeys[h], true); assert.equal(wrapKeys[j], undefined); }); + + newTest({ + title: 'Can handle long key chords', + start: ['|'], + // fes + keysPressed: ' fes', + end: ['|'], + endMode: ModeName.Visual, + }); }); diff --git a/test/mode/modeVisual.test.ts b/test/mode/modeVisual.test.ts index c65a457f74b..f4c69d62c33 100644 --- a/test/mode/modeVisual.test.ts +++ b/test/mode/modeVisual.test.ts @@ -827,9 +827,9 @@ suite('Mode Visual', () => { }); suite('visualstar', () => { - setup(() => { + setup(async () => { Globals.mockConfiguration.visualstar = true; - reloadConfiguration(); + await reloadConfiguration(); }); newTest({ diff --git a/test/plugins/sneak.test.ts b/test/plugins/sneak.test.ts index bc2feeebd75..ce6a8372c90 100644 --- a/test/plugins/sneak.test.ts +++ b/test/plugins/sneak.test.ts @@ -8,7 +8,7 @@ suite('sneak plugin', () => { setup(async () => { await setupWorkspace(); Globals.mockConfiguration.sneak = true; - reloadConfiguration(); + await reloadConfiguration(); }); teardown(cleanUpWorkspace); diff --git a/test/testUtils.ts b/test/testUtils.ts index 36bb4f0aa3c..47ec84bd823 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -85,7 +85,7 @@ export async function setupWorkspace( await vscode.window.showTextDocument(doc); Globals.mockConfiguration = config; - reloadConfiguration(); + await reloadConfiguration(); let activeTextEditor = vscode.window.activeTextEditor; assert.ok(activeTextEditor); @@ -134,8 +134,8 @@ export async function cleanUpWorkspace(): Promise { }); } -export function reloadConfiguration() { - require('../src/configuration/configuration').configuration.load(); +export async function reloadConfiguration() { + await require('../src/configuration/configuration').configuration.load(); } /** From 415852fc9d76e442f6dd0509f4833a1480cfff26 Mon Sep 17 00:00:00 2001 From: Jason Poon Date: Wed, 30 Jan 2019 02:14:30 -0800 Subject: [PATCH 7/7] fix: adjust comments --- extension.ts | 2 +- src/configuration/configuration.ts | 11 ++++++----- src/configuration/iconfiguration.ts | 2 -- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/extension.ts b/extension.ts index 6c31ad5ec95..fa4bcfb348e 100644 --- a/extension.ts +++ b/extension.ts @@ -72,7 +72,7 @@ async function loadConfiguration() { const logger = Logger.get('Configuration'); const numErrors = configurationErrors.filter(e => e.level === 'error').length; - logger.debug(`${numErrors} found with vim configuration`); + logger.debug(`${numErrors} errors found with vim configuration`); if (numErrors > 0) { for (let configurationError of configurationErrors) { diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts index 73250fb3adf..1fdf7254ce0 100644 --- a/src/configuration/configuration.ts +++ b/src/configuration/configuration.ts @@ -126,17 +126,18 @@ class Configuration implements IConfiguration { ); } - const keys = remapping.before.join(''); - if (modeKeyBindingsMap.has(keys)) { + // check for duplicates + const beforeKeys = remapping.before.join(''); + if (modeKeyBindingsMap.has(beforeKeys)) { configurationErrors.push({ level: 'error', - message: `${remapping.before}. Duplicate remapped key for ${keys}.`, + message: `${remapping.before}. Duplicate remapped key for ${beforeKeys}.`, }); continue; } // add to map - modeKeyBindingsMap.set(keys, remapping); + modeKeyBindingsMap.set(beforeKeys, remapping); } configuration[modeKeyBindingsKey + 'Map'] = modeKeyBindingsMap; @@ -149,6 +150,7 @@ class Configuration implements IConfiguration { } // read package.json for bound keys + // enable/disable certain key combinations this.boundKeyCombinations = []; for (let keybinding of packagejson.contributes.keybindings) { if (keybinding.when.indexOf('listFocus') !== -1) { @@ -168,7 +170,6 @@ class Configuration implements IConfiguration { }); } - // enable/disable certain key combinations for (const boundKey of this.boundKeyCombinations) { // By default, all key combinations are used let useKey = true; diff --git a/src/configuration/iconfiguration.ts b/src/configuration/iconfiguration.ts index 40560296577..1aa1acf6b50 100644 --- a/src/configuration/iconfiguration.ts +++ b/src/configuration/iconfiguration.ts @@ -30,13 +30,11 @@ export interface IDebugConfiguration { /** * Maximum level of messages to show as VS Code information message - * Supported values: ['error', 'warn', 'info', 'verbose', 'debug'] */ loggingLevelForAlert: 'error' | 'warn' | 'info' | 'verbose' | 'debug'; /** * Maximum level of messages to log to console. - * Supported values: ['error', 'warn', 'info', 'verbose', 'debug'] */ loggingLevelForConsole: 'error' | 'warn' | 'info' | 'verbose' | 'debug'; }