fix(findNvim): skip default dirs if cmds is given

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)
 

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 () {

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<string> {
 }
 
 /**
- * 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<FindNvimResult> {
+  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.