Skip to content

WIP: redesign "remote plugins", eliminate "host" concept #344

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justinmk
Copy link
Member

@justinmk justinmk commented Mar 26, 2024

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.
  • 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".

TODO

  • setup() (placeholder name) attaches and calls nvim_set_client_info() with the methods defined by setHandler().
  • node scripts can't import globally-installed modules. So the rplugin script will need to be run with NODE_PATH=$(npm root --quiet -g) , or would need to npm install in the local plugin directory.
    • most reliabe: do npm install neovim at the root of stdpath('cache') (or wherever plugins are stored)
    • yarn, pnpm, etc? 💩 ref

TEST CASE / DEMO:

Invocation

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']]);

Result

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' ] ]

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' ] ]
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant