Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

simplify remote plugins #167

Open
1 of 5 tasks
justinmk opened this issue May 8, 2024 · 8 comments
Open
1 of 5 tasks

simplify remote plugins #167

justinmk opened this issue May 8, 2024 · 8 comments

Comments

@justinmk
Copy link
Member

justinmk commented May 8, 2024

Problem

The design, and usage, of remote plugins can be simplified, as described in neovim/neovim#27949

tl;dr:

  • remove the concept of "remote plugins"
  • instead, any Go module that imports the go-client can call RegisterHandler to handle requests from Nvim
  • "remote plugins" become plain old Lua plugins which call RPC methods on the Go module

Solution

Reference

@zchee
Copy link
Member

zchee commented May 12, 2024

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

@garyburd
Copy link
Contributor

garyburd commented May 13, 2024

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
Copy link
Member Author

@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
Copy link
Contributor

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

  • Prepare for plugin deprecation.

  • Add new directory examples/remote containing the following:

    • ./plugin/goexample.lua: remote module Lua code.
    • ./goexample/{main.go, go.mod}: remote module Go code
    • ./README.md: instructions for building and installing the remote module.

    The immediate goal is to post minimal working code using the package as is. Let's wait for progress on neovim/issues/27949 before adding the proposed helper function, writing complete documentation and adding tests.

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

@zchee
Copy link
Member

zchee commented May 14, 2024

@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
Copy link
Member Author

justinmk commented May 14, 2024

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 (#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
Copy link
Contributor

@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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants