neovim / go-client

Nvim Go client
https://pkg.go.dev/github.com/neovim/go-client
Apache License 2.0
573 stars 36 forks source link

simplify remote plugins #167

Open justinmk opened 5 months ago

justinmk commented 5 months ago

Problem

The design, and usage, of remote plugins can be simplified, as described in https://github.com/neovim/neovim/issues/27949

tl;dr:

Solution

Reference

zchee commented 4 months ago

@justinmk I don't understand all the new "remote modules" behavior yet, but I decided to work on the migration process ASAP.

garyburd commented 4 months ago

remove the concept of "remote plugins"

Delete the plugin package and any references to that package in the documentation.

instead, any Go module that imports the go-client can call set_handler('foo', myFunction) to handle the "foo" request from Nvim

The package accomplishes this goal as is:

Step 1 The application creates a new Nvim client connected to stdio:

// Turn off timestamps in output.
log.SetFlags(0) 

// Create a client connected to stdout. Configure the client
// to use the standard log package for logging.
v, err := nvim.New(os.Stdin, os.Stdout, os.Stdout, log.Printf)
if err != nil {
    log.Fatal(err)
}

// Redirect the application's direct use of stdout to stderr.
// Direct writes by the application garble the RPC stream.
os.Stdout = os.Stderr

Step 2 The application registers handlers with the nvim client using the client's RegisterHandler method.

Step 3 The application runs the the RPC message loop.

if err := v.Serve(); err != nil {
    log.Fatal(err)
}

Optional enhancement Encapsulate the stdio related code in a helper function.

package nvim

import (
    "log"
    "os"
)

// NewStdio creates a client connected to the process stdio.
func NewStdio(logf func(string ...any)) (*Nvim, error) {
    v, err := nvim.New(os.Stdin, os.Stdout, os.Stdout, logf)
    if err != nil {
        return nil, err
    }
    os.Stdout = os.Stderr
    return v, nil
}

Example use:

package main

import  (
    "log"
    "github.com/neovim/go-client/nvim"
)

func clearCurrentLine(v *nvim.Nvim) {
     v.SetCurrentLine([]byte{})
}

func main() {
    v, err := nvim.NewStdio(log.Printf)
    if err != nil { log.Fatal(err) }
    v.RegisterHandler("clearCurrentLine", clearCurrentLine)
    if err := v.Serve(); err != nil { log.Fatal(err) }
}

Edit: I moved the call to v.Serve() from the helper function to the application's main. This gives the application more control over how Serve() is run.

Edit: I wrote the code in this comment directly in the issue tracker. The code has not been compiled, let alone tested.

justinmk commented 4 months ago

@garyburd thanks so much, that is very helpful. So this is basically already supported by go-client.

Optional enhancement Encapsulate the stdio related code in a helper function.

💯

garyburd commented 4 months ago

@justinmk Yes, it's already supported. I propose the following:

I have some time for this in the next couple of days. @zchee and @justinmk let me know your thoughts on the proposal.

zchee commented 4 months ago

@garyburd cc: @justinmk Totally understand and almost agreed to your proposal. Thanks for quickly improvement!

More comment later, because now JST is start working time.

justinmk commented 4 months ago

The immediate goal is to post minimal working code using the package as is.

Sounds good!

Let's wait for progress on neovim/issues/27949 before adding the proposed helper function, writing complete documentation and adding tests.

27949 won't be a "big bang", we'll have to do it incrementally.

Since go-client already has RegisterHandler, if we provide a full example (https://github.com/neovim/go-client/pull/172) that shows a Lua plugin starting a go-client process and calling RPC methods via vim.rpcrequest/vim.rpcnotify, there isn't really anything blocking us from officially deprecating the go-client rplugin support.

This will be doubly helpful, as an end-to-end demonstration of the logistics for 27949.

Of course, in the future Nvim may gain nice-to-have features that make it more convenient to spin up a long-lived client and call its RPC methods. But I don't see that as a requirement for this phase.

garyburd commented 4 months ago

@justinmk I wanted to wait on 27949 before adding an API in case the work on 27949 exposes additional requirements for the API. If you think things are baked enough, then I'll proceed ahead with the API and deprecation. I may not be able to get to it for a couple of weeks.

Here's a list of public Go plugins: https://pkg.go.dev/github.com/neovim/go-client/nvim/plugin?tab=importedby

justinmk commented 4 months ago

in case the work on 27949 exposes additional requirements for the API

We may weave-in more nice-to-have ideas later, but that will just upgrade the UX rather than be a hard requirement.

Currently, the (informal) requirements for a remote plugin are:

  1. responds to "poll" (I guess this is more for "providers" than rplugins)
    • seems potentially worth keeping as a basic "liveness check".
  2. returns a "spec"
    • can probably be dropped. remote plugins will be coupled with their Lua setup code, which inherently defines the "spec".