Skip to content

Commit

Permalink
fix: check neovim configurations and timeout on nvim attach. mitigates
Browse files Browse the repository at this point in the history
  • Loading branch information
jpoon committed Feb 1, 2019
1 parent bfcb817 commit 05abc9c
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 16 deletions.
11 changes: 11 additions & 0 deletions src/configuration/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,17 @@ class Configuration implements IConfiguration {
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;
}

// wrap keys
this.wrapKeys = {};
for (const wrapKey of this.whichwrap.split(',')) {
Expand Down
24 changes: 24 additions & 0 deletions src/configuration/configurationValidator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as vscode from 'vscode';
import * as fs from 'fs';
import { IKeyRemapping } from './iconfiguration';
import { ConfigurationError } from './configurationError';
import { promisify } from 'util';

class ConfigurationValidator {
private _commandMap: Map<string, boolean>;
Expand All @@ -13,6 +15,28 @@ class ConfigurationValidator {
return (await this.getCommandMap()).has(command);
}

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'.` }];
Expand Down
3 changes: 1 addition & 2 deletions src/mode/modeHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,7 @@ export class ModeHandler implements vscode.Disposable {
this.vimState = await this.handleKeyEventHelper(key, this.vimState);
}
} catch (e) {
this._logger.error(`error handling key=${key}. err=${e}.`);
throw e;
throw new Error(`Failed to handle key=${key}. ${e.message}`);
}

this.vimState.lastKeyPressedTimestamp = now;
Expand Down
29 changes: 21 additions & 8 deletions src/neovim/neovim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,30 @@ export class NeovimWrapper implements vscode.Disposable {
private process: ChildProcess;
private nvim: Neovim;
private readonly logger = Logger.get('Neovim');
private readonly processTimeoutInSeconds = 3;

async run(vimState: VimState, command: string) {
if (!this.nvim) {
this.nvim = await this.startNeovim();

await this.nvim.uiAttach(80, 20, {
ext_cmdline: false,
ext_popupmenu: false,
ext_tabline: false,
ext_wildmenu: false,
rgb: false,
});
try {
const nvimAttach = this.nvim.uiAttach(80, 20, {
ext_cmdline: false,
ext_popupmenu: false,
ext_tabline: false,
ext_wildmenu: false,
rgb: false,
});

const timeout = new Promise((resolve, reject) => {
setTimeout(() => reject(new Error('Timeout')), this.processTimeoutInSeconds * 1000);
});

await Promise.race([nvimAttach, timeout]);
} catch (e) {
configuration.enableNeovim = false;
throw new Error(`Failed to attach to neovim process. ${e.message}`);
}

const apiInfo = await this.nvim.apiInfo;
const version = apiInfo[1].version;
Expand Down Expand Up @@ -78,9 +90,10 @@ export class NeovimWrapper implements vscode.Disposable {
});

this.process.on('error', err => {
this.logger.error(`Error spawning neovim. Error=${err.message}.`);
this.logger.error(`Error spawning neovim. ${err.message}.`);
configuration.enableNeovim = false;
});

return attach({ proc: this.process });
}

Expand Down
6 changes: 1 addition & 5 deletions src/state/vimState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,7 @@ export class VimState implements vscode.Disposable {
this.identity = new EditorIdentity(editor);
this.historyTracker = new HistoryTracker(this);
this.easyMotion = new EasyMotion();

if (enableNeovim) {
this.nvim = new NeovimWrapper();
}

this.nvim = new NeovimWrapper();
this._inputMethodSwitcher = new InputMethodSwitcher();
}

Expand Down
2 changes: 1 addition & 1 deletion src/taskQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class TaskQueue {
await task.promise();
task.isRunning = false;
} catch (e) {
this._logger.error(`Error running task. err=${e}.`);
this._logger.error(`Error running task. ${e.message}.`);
} finally {
this.dequeueTask(task);
}
Expand Down
5 changes: 5 additions & 0 deletions test/configuration/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ suite('Configuration', () => {
},
]);
configuration.whichwrap = 'h,l';
configuration.enableNeovim = true;

setup(async () => {
await setupWorkspace(configuration);
Expand Down Expand Up @@ -76,6 +77,10 @@ suite('Configuration', () => {
assert.equal(wrapKeys[j], undefined);
});

test('neovim disabled on missing path', async () => {
assert.equal(false, srcConfiguration.configuration.enableNeovim)
});

newTest({
title: 'Can handle long key chords',
start: ['|'],
Expand Down

0 comments on commit 05abc9c

Please sign in to comment.