From 9cf55275b8e0ae76e31fe1c29c71656e3e3d4eef Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 11 Sep 2026 17:10:06 +0200 Subject: [PATCH] fix(findNvim): skip default dirs if `cmds` is given Problem: No way for caller to skip the default dir search, which makes it awkward to check if the found result was part of the caller's specified `cmds` or a "default" location. Solution: - Skip searching default locations if `cmds` is given. Callers are expected to call `findNvim()` a 2nd time if they want to also search default locations. - Throw an error if both `cmds` and `dirs` are given. --- CHANGELOG.md | 5 +++-- packages/neovim/src/utils/findNvim.test.ts | 17 +++++++++++------ packages/neovim/src/utils/findNvim.ts | 19 +++++++++++-------- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fc24f12..6071b17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ - fix: "Right-hand side of 'instanceof' is not an object" if `api/Buffer` is required before the rest of the package. https://github.com/vscode-neovim/vscode-neovim/issues/2671 -- feat: `findNvim(…, { cmds: … })` accepts multi-part commands. - Example: `['wsl.exe', '-d', 'Ubuntu', 'nvim']`. Results have `cmd`; `path` is deprecated. #432 +- feat: `findNvim(…, { cmds: … })` checks multi-part commands (and skips + searching the default locations). Example: `['wsl.exe', '-d', 'Ubuntu', 'nvim']`. + Results have `cmd`; `path` is deprecated. #432 ## [5.4.0](https://github.com/neovim/node-client/compare/v5.3.0...v5.4.0) diff --git a/packages/neovim/src/utils/findNvim.test.ts b/packages/neovim/src/utils/findNvim.test.ts index f46aff00..59c5f9cb 100644 --- a/packages/neovim/src/utils/findNvim.test.ts +++ b/packages/neovim/src/utils/findNvim.test.ts @@ -130,7 +130,7 @@ describe('findNvim', () => { }); }); - it('tries commands given by `cmds`', () => { + it('tries only the commands given by `cmds`', () => { const nvim = findNvim({ firstMatch: true }).matches[0].cmd[0]; const cmds = [ [nvim, '--clean'], @@ -139,13 +139,18 @@ describe('findNvim', () => { ['/custom/path/to/nvim'], ['/another/custom/path'], ].map(([arg0, ...args]) => [normalizePath(arg0), ...args]); - const nvimRes = findNvim({ cmds, orderBy: 'none' }); + const nvimRes = findNvim({ cmds }); - // Args are passed to the command: `--clean` works, `--bogus` fails. - expect(nvimRes.matches[0].cmd).toEqual(cmds[0]); + // No search. Args are passed to the command: `--clean` works, `--bogus` fails. + expect(nvimRes.matches.map(m => m.cmd)).toEqual([cmds[0]]); expect(nvimRes.invalid.map(i => i.cmd)).toEqual(cmds.slice(1)); - // Deprecated `paths`. - expect(findNvim({ paths: cmds[2] }).invalid.map(i => i.cmd)).toEqual([cmds[2]]); + // Deprecated `paths` also searches the default locations. + const pathsRes = findNvim({ paths: cmds[2] }); + expect(pathsRes.matches.length).toBeGreaterThan(0); + expect(pathsRes.invalid.map(i => i.cmd)).toEqual([cmds[2]]); + // Empty `cmds` searches. `cmds` + `dirs` is an error. + expect(findNvim({ cmds: [] }).matches.length).toBeGreaterThan(0); + expect(() => findNvim({ cmds, dirs: [testDir] })).toThrow('cannot combine `cmds` and `dirs`'); }); it('searches in additional custom dirs', function () { diff --git a/packages/neovim/src/utils/findNvim.ts b/packages/neovim/src/utils/findNvim.ts index 23dfbfc9..cd3143b4 100644 --- a/packages/neovim/src/utils/findNvim.ts +++ b/packages/neovim/src/utils/findNvim.ts @@ -38,8 +38,8 @@ export type FindNvimOptions = { */ readonly firstMatch?: boolean; /** - * (Optional) Other commands that (potentially) invoke Nvim and can receive arbitrary args. - * Checked before searching `dirs`. Useful for checking a user-configured Nvim location or + * (Optional) Other commands that (potentially) invoke Nvim. Not allowed with `dirs`; skips + * searching the default locations. Useful for checking a user-configured Nvim location or * arbitrary wrappers such as Windows WSL. * * Example: @@ -54,10 +54,8 @@ export type FindNvimOptions = { /** @deprecated */ readonly paths?: string[]; /** - * (Optional) Additional directories to search for Nvim executables. - * These directories will be searched after checking `cmds` - * but before searching `$PATH` and other default locations. - * Useful for including non-standard installation directories. + * (Optional) Additional directories to search for Nvim executables, before `$PATH` and other + * default locations. Not allowed with `cmds`. * * Example: ['/opt/neovim/bin', '/home/user/custom/bin'] */ @@ -198,21 +196,26 @@ function getPlatformSearchDirs(): Set { } /** - * Tries to find a usable `nvim` binary on the current system. + * Tries to find a usable `nvim` binary on the current system. Searches common locations by default. * * @param opt.minVersion See {@link FindNvimOptions.minVersion} * @param opt.orderBy See {@link FindNvimOptions.orderBy} * @param opt.firstMatch See {@link FindNvimOptions.firstMatch} * @param opt.cmds See {@link FindNvimOptions.cmds} * @param opt.dirs See {@link FindNvimOptions.dirs} + * @throws {TypeError} If `cmds` and `dirs` are both given. */ export function findNvim(opt: FindNvimOptions = {}): Readonly { + if (opt.cmds?.length && opt.dirs?.length) { + throw new TypeError('Invalid params: cannot combine `cmds` and `dirs`'); + } const nvimExecutable = windows ? 'nvim.exe' : 'nvim'; const userCmds = [...(opt.cmds ?? []), ...(opt.paths ?? []).map(p => [p])].map( ([arg0, ...args]) => [normalizePath(arg0), ...args] ); + const searchDirs = opt.cmds?.length ? [] : [...(opt.dirs ?? []), ...getPlatformSearchDirs()]; // Unlike `cmds` (always tried, so failures are reported in `invalid`), skip dirs without Nvim. - const dirCmds = [...(opt.dirs ?? []), ...getPlatformSearchDirs()] + const dirCmds = searchDirs .map(dir => [normalizePath(join(dir, nvimExecutable))]) .filter(([nvimPath]) => existsSync(nvimPath)); // Dedupe, e.g. if a dir is in both $PATH and the platform defaults.