Skip to content

Commit

Permalink
WIP: eliminate "host" concept
Browse files Browse the repository at this point in the history
Problem:
The "remote plugin" concept is too complicated. neovim/neovim#27949

Solution:
- Let the "client" also be the "host". Eliminate the separate "host"
  concept and related modules.
- Let any node module be a "host". Any node module that imports the
  "neovim" package and defines method handler(s) is a "remote module".
  It is loaded by Nvim same as any "node client".

Story:
- The value in rplugins is:
  1. it finds the interpreter on the system
  2. it figures out how to invoke the main script with the interpreter

Old architecture:

    nvim rplugin framework ->
      node: cli.js ->
        starts the "plugin Host"
          attaches itself to current node process
            searches for plugins and tries to load them in the node process (MULTI-TENANCY)
              -> plugin1
              -> plugin2
              -> ...

New architecture:

    nvim vim.rplugin('node', '…/plugin1.js') ->
      node: neovim.cli()
    nvim vim.rplugin('node', '…/plugin2.js') ->
      node: neovim.cli()

1. A Lua plugin calls `vim.rplugin('node', '/path/to/plugin.js')`.
2. Each call to `vim.rplugin()` starts a new node process (no "multi-tenancy").
3. plugin.js is just a normal javascript file that imports the `neovim` package.
4. plugin.js provides a "main" function. It can simply import the `neovim.cli()` util function, which handles attaching/setup.

TEST CASE / DEMO:

    const found = findNvim({ orderBy: 'desc', minVersion: '0.9.0' })
    const nvim_proc = child_process.spawn(found.matches[0].path, ['--clean', '--embed'], {});
    const nvim = attach({ proc: nvim_proc });
    nvim.setHandler('foo', (ev, args) => {
      nvim.logger.info('handled from remote module: "%s": args:%O', ev.name, args);
    });
    nvim.callFunction('rpcrequest', [(await nvim.channelId), 'foo', [42, true, 'bar']]);

    2024-03-26 16:47:35 INF handleRequest: foo
    2024-03-26 16:47:35 DBG request received: foo
    2024-03-26 16:47:35 INF handled from remote module: "foo": args:[ [ 42, true, 'bar' ] ]
  • Loading branch information
justinmk committed Apr 18, 2024
1 parent 0253672 commit 8982da5
Show file tree
Hide file tree
Showing 10 changed files with 300 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/example-plugin2/fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'you bet!';
30 changes: 30 additions & 0 deletions packages/example-plugin2/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const required = require('./fixture');
const neovim = require('neovim');

let nvim;

function hostTest(args, range) {
if (args[0] === 'canhazresponse?') {
throw new Error('no >:(');
}

nvim.setLine('A line, for your troubles');

return 'called hostTest';
}

function onBufEnter(filename) {
return new Promise((resolve, reject) => {
console.log('This is an annoying function ' + filename);
resolve(filename);
});
}

function main() {
nvim = neovim.cli();
// Now that we successfully started, we can remove the default listener.
//nvim.removeAllListeners('request');
nvim.setHandler('testMethod1', hostTest);
}

main();
9 changes: 9 additions & 0 deletions packages/example-plugin2/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@neovim/example-plugin2",
"private": true,
"version": "1.0.0",
"description": "Test fixture for new rplugin design",
"main": "index.js",
"license": "MIT",
"devDependencies": {}
}
9 changes: 8 additions & 1 deletion packages/integration-tests/__tests__/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,16 @@ import * as fs from 'fs';
import * as path from 'path';
import * as http from 'http';

//
//
// TODO: The old rplugin design is deprecated and NOT supported.
// This file will be deleted.
//
//

import { NeovimClient, attach, findNvim } from 'neovim';

describe('Node host', () => {
describe.skip('Node host (OLD, DELETE ME)', () => {
const testdir = process.cwd();
let proc: cp.ChildProcessWithoutNullStreams;
let args;
Expand Down
127 changes: 127 additions & 0 deletions packages/integration-tests/__tests__/rplugin2.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/* eslint-env jest */
import * as cp from 'child_process';
import * as path from 'path';

import { NeovimClient, attach, findNvim } from 'neovim';

/**
* Runs a program and returns its output.
*/
async function run(cmd: string, args: string[]) {
return new Promise<{ proc: ReturnType<typeof cp.spawn>, stdout: string, stderr: string}>((resolve, reject) => {
const proc = cp.spawn(cmd, args, { shell: false });
const rv = {
proc: proc,
stdout: '',
stderr: '',
}

proc.stdout.on('data', (data) => {
rv.stdout += data.toString();
});

proc.stderr.on('data', (data) => {
rv.stderr += data.toString();
});

proc.on('exit', (code_) => {
resolve(rv);
});

proc.on('error', (e) => {
reject(e);
});
});
}

describe('Node host2', () => {
const thisDir = path.resolve(__dirname);
const pluginDir = path.resolve(thisDir, '../../example-plugin2/');
const pluginMain = path.resolve(pluginDir, 'index.js').replace(/\\/g, '/');

const testdir = process.cwd();
let nvimProc: ReturnType<typeof cp.spawn>;
let nvim: NeovimClient;

beforeAll(async () => {
const minVersion = '0.9.5'
const nvimInfo = findNvim({ minVersion: minVersion });
const nvimPath = nvimInfo.matches[0]?.path;
if (!nvimPath) {
throw new Error(`nvim ${minVersion} not found`)
}

nvimProc = cp.spawn(nvimPath, ['--clean', '-n', '--headless', '--embed'], {});
nvim = attach({ proc: nvimProc });
});

afterAll(() => {
process.chdir(testdir);
nvim.quit();
if (nvimProc && nvimProc.connected) {
nvimProc.disconnect();
}
});

beforeEach(() => {});

afterEach(() => {});


/**
* From the Nvim process, starts a new "node …/plugin/index.js" RPC job (that
* is, a node "plugin host", aka an Nvim node client).
*/
async function newPluginChan() {
const nodePath = process.argv0.replace(/\\/g, '/');
const luacode = `
-- "node …/plugin/index.js"
local argv = { [[${nodePath}]], [[${pluginMain}]] }
local chan = vim.fn.jobstart(argv, { rpc = true, stderr_buffered = true })
return chan
`
return await nvim.lua(luacode);
}

it('`node plugin.js --version` prints node-client version', async () => {
//process.chdir(thisDir);
const proc = await run(process.argv0, [pluginMain, '--version']);
// "5.1.1-dev.0\n"
expect(proc.stdout).toMatch(/\d+\.\d+\.\d+/);

proc.proc.kill('SIGKILL');
});

it('responds to "poll" with "ok"', async () => {
// See also the old provider#Poll() function.

// From Nvim, start an "node …/plugin/index.js" RPC job.
// Then use that channel to call methods on the remote plugin.
const chan = await newPluginChan();
const rv = await nvim.lua(`return vim.rpcrequest(..., 'poll')`, [ chan ]);

expect(rv).toEqual('ok');
});

//it('responds to "nvim_xx" methods', async () => {
// // This is just a happy accident of the fact that Nvim plugin host === client.
// const chan = await newPluginChan();
// const rv = await nvim.lua(`return vim.rpcrequest(..., 'nvim_eval', '1 + 3')`, [ chan ]);
// expect(rv).toEqual(3);
//});

it('responds to custom, plugin-defined methods', async () => {
const chan = await newPluginChan();
// The "testMethod1" function is defined in …/example-plugin2/index.js.
const rv = await nvim.lua(`return vim.rpcrequest(..., 'testMethod1', {})`, [ chan ]);

expect(rv).toEqual('called hostTest');
});

// TODO
//it('Lua plugin can define autocmds/functions that call the remote plugin', async () => {
// // JSHostTestCmd
// // BufEnter
//});
});

22 changes: 20 additions & 2 deletions packages/neovim/src/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Handles attaching transport
*/
import { Logger } from '../utils/logger';
import { Transport } from '../utils/transport';
import { Response, Transport } from '../utils/transport';
import { VimValue } from '../types/VimValue';
import { Neovim } from './Neovim';
import { Buffer } from './Buffer';
Expand All @@ -12,12 +12,30 @@ const REGEX_BUF_EVENT = /nvim_buf_(.*)_event/;
export class NeovimClient extends Neovim {
protected requestQueue: any[];

/**
* Handlers for custom (non "nvim_") methods registered by the remote module.
* These handle requests from the Nvim peer.
*/
public handlers: {
[index: string]: (args: any[], event: { name: string }) => any;
} = {};

private transportAttached: boolean;

private _channelId?: number;

private attachedBuffers: Map<string, Map<string, Function[]>> = new Map();

/**
* Defines a handler for incoming RPC request method/notification.
*/
setHandler(
method: string,
fn: (args: any[], event: { name: string }) => any
) {
this.handlers[method] = fn;

Check warning on line 36 in packages/neovim/src/api/client.ts

View check run for this annotation

Codecov / codecov/patch

packages/neovim/src/api/client.ts#L35-L36

Added lines #L35 - L36 were not covered by tests
}

constructor(options: { transport?: Transport; logger?: Logger } = {}) {
// Neovim has no `data` or `metadata`
super({
Expand Down Expand Up @@ -66,7 +84,7 @@ export class NeovimClient extends Neovim {
handleRequest(
method: string,
args: VimValue[],
resp: any,
resp: Response,
...restArgs: any[]
) {
// If neovim API is not generated yet and we are not handle a 'specs' request
Expand Down
100 changes: 100 additions & 0 deletions packages/neovim/src/cli.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { spawnSync } from 'node:child_process';
import { attach } from './attach';

let nvim: ReturnType<typeof attach>;

// node <current script> <rest of args>
const [, , ...args] = process.argv;

if (args[0] === '--version') {
// eslint-disable-next-line global-require, @typescript-eslint/no-var-requires
const pkg = require('../package.json');
// eslint-disable-next-line no-console
console.log(pkg.version);
process.exit(0);
}

// "21.6.1" => "21"
const nodeMajorVersionStr = process.versions.node.replace(/\..*/, '');
const nodeMajorVersion = Number.parseInt(nodeMajorVersionStr ?? '0', 10);

if (
process.env.NVIM_NODE_HOST_DEBUG &&
nodeMajorVersion >= 8 &&
process.execArgv.every(token => token !== '--inspect-brk')
) {
const childHost = spawnSync(
process.execPath,
process.execArgv.concat(['--inspect-brk']).concat(process.argv.slice(1)),
{ stdio: 'inherit' }
);
process.exit(childHost.status ?? undefined);
}

export interface Response {
send(resp: any, isError?: boolean): void;
}

process.on('unhandledRejection', (reason, p) => {
process.stderr.write(`Unhandled Rejection at: ${p} reason: ${reason}\n`);
});

/**
* The "client" is also the "host". https://github.com/neovim/neovim/issues/27949
*/
async function handleRequest(method: string, args: any[], res: Response) {
nvim.logger.debug('request received: %s', method);
// 'poll' and 'specs' are requests from Nvim internals. Else we dispatch to registered remote module methods (if any).
if (method === 'poll') {
// Handshake for Nvim.
res.send('ok');
// } else if (method.startsWith('nvim_')) {
// // Let base class handle it.
// nvim.request(method, args);
} else {
const handler = nvim.handlers[method];
if (!handler) {
const msg = `node-client: missing handler for "${method}"`;
nvim.logger.error(msg);
res.send(msg, true);
}

try {
nvim.logger.debug('found handler: %s: %O', method, handler);
const plugResult = await handler(args, { name: method });
res.send(
!plugResult || typeof plugResult === 'undefined' ? null : plugResult
);
} catch (e) {
const err = e as Error;
const msg = `node-client: failed to handle request: "${method}": ${err.message}`;
nvim.logger.error(msg);
res.send(err.toString(), true);
}
}
}

// "The client *is* the host... The client *is* the host..."
//
// "Main" entrypoint for any Nvim remote plugin. It implements the Nvim remote
// plugin specification:
// - Attaches self to incoming RPC channel.
// - Responds to "poll" with "ok".
// - TODO: "specs"?
export function cli() {
try {
// Reverse stdio because it's from the perspective of Nvim.
nvim = attach({ reader: process.stdin, writer: process.stdout });
nvim.logger.debug('host.start');
nvim.on('request', handleRequest);

return nvim;
} catch (e) {
const err = e as Error;
process.stderr.write(
`failed to start Nvim plugin host: ${err.name}: ${err.message}\n`
);

return undefined;
}
}
3 changes: 3 additions & 0 deletions packages/neovim/src/host/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ export interface Response {
send(resp: any, isError?: boolean): void;
}

/**
* @deprecated Eliminate the "host" concept. https://github.com/neovim/neovim/issues/27949
*/
export class Host {
public loaded: { [index: string]: NvimPlugin };

Expand Down
1 change: 1 addition & 0 deletions packages/neovim/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { attach } from './attach';
export { cli } from './cli';
export { Neovim, NeovimClient, Buffer, Tabpage, Window } from './api/index';
export { Plugin, Function, Autocmd, Command } from './plugin';
export { NvimPlugin } from './host/NvimPlugin';
Expand Down
2 changes: 1 addition & 1 deletion packages/neovim/src/utils/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from '@msgpack/msgpack';
import { Metadata } from '../api/types';

class Response {
export class Response {
private requestId: number;

private sent!: boolean;
Expand Down

0 comments on commit 8982da5

Please sign in to comment.