tliron / glsp Goto Github PK

First of all thank you very much for lib @tliron
Just a heads up that #24 hasn't been released yet so a raw go get still fetches 0.2.1 with incorrect signature

workaround for other users: go get github.com/tliron/glsp@main

Hello,
First of all, thanks for the library.

It is very good, and I'm currently experimenting the creation of a language server based on it (and your puccini-language-server as a reference) 🙏

The spec just got upgraded a few days ago to 3.17.0 (05/10/2022), we should upgrade the protocol accordingly (if it's not too much effort).

I'm ready to help, still working around your code atm, but totally available 😇

Thanks for the great package!

The version currently posted on pkg.go.dev still references kutil/logging rather than commonlog.
Would you mind publishing an update so that we (and others) can use it via go get rather than maintaining a local copy?

After a panic the first time I tried to do anything basic, I read through issue #2 and see that we need to call logging.Configure() somewhere, but when I try that I get a different panic:

func main() {
	logging.Configure(1, nil)
...

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1098a03]

goroutine 1 [running]:
github.com/tliron/kutil/logging.Configure(...)
	vendor/github.com/tliron/kutil/logging/api.go:15
main.main()
	main.go:22 +0x83

Looking at the logging implementation, it seems like SetBackend() needs to be called somewhere as well? I'm not seeing where it is called in puccini.

Thanks!

In order to parse either a TextDocumentContentChangeEvent or TextDocumentContentChangeEventWhole from the ContentChanges, the code uses the assumption that the marshaling fails if there are missing fields. This is not true.

Example code where this assumption is shown.

for _, change := range parameters.ContentChanges {
	switch cast := change.(type) {
	case protocol.TextDocumentContentChangeEvent:
		// This will always occur, with zeroed ranges if sync kind is TextDocumentSyncKindFull
		return fmt.Errorf("incremental changes are not supported")
	case protocol.TextDocumentContentChangeEventWhole:
		// This never occurs
		document.Content = cast.Text
	}
}

A better approach for detecting would be to first have a common, private, struct much like TextDocumentContentChangeEvent, but where Range is a pointer. If the pointer is nil after marshaling, then return a TextDocumentContentChangeEventWhole, else return a TextDcoumentContentChangeEvent. That mechanism uses the documented behavior for the changes - range is never set for whole updates.

Thank you for creating, sharing, and supporting your library.

Using it was a real pleasure, and it only took me a couple of hours to get completion and info-on-hover working for my toy lisp implementation.

Firstly, thanks for this library. It makes implementing a LSP in Go a pleasure and the API is delightful!

Forgive me if I'm wrong, but I think the current implementation is purely re-active. Eg, responding to client requests on the server.

If I look at for example window/ShowMessage or $/progress these are notifications sent to the client from the server.
However they seem to be implemented in the same way as textDocument/didOpen for example.
Which is a request sent from the client.

From this, I believe it is currently impossible to send arbitrary messages from the server to a client, and only to respond to Requests/Act on notifications from the client.

I would be happy to work towards fixing this if this is something you agree with.

Thanks

i expected callbacks to have this shape:

TextDocumentCodeAction: func(ctx context.Context, context *glsp.Context, params *protocol.CodeActionParams) (any, error) {
...

or even just

TextDocumentCodeAction: func(ctx context.Context, params *protocol.CodeActionParams) (any, error) {
   gctx := glsp.FromContext(ctx)
   ...

or even just (you can generate concrete types with go generate ./... and not have loosely-typed lspctx.Notify(protocol.ServerTextDocumentPublishDiagnostics, &protocol.PublishDiagnosticsParams{

TextDocumentCodeAction: func(ctx context.Context, params *protocol.CodeActionParams) (any, error) {
   /// ..
   protocol.NotifyTextDocumentPublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
	URI:         uri,
	Diagnostics: res.Diagnostics,
   })

....

but at the moment it's quite difficult to work with this library

Hi, first of all, thanks for the tremendous amount of work you put into this library.

I'm implementing an LSP for a doctest library which I wrote (https://github.com/apitoolkit/doctests), and the way it should work is such that a user can add golang expressions into their comments, and they can execute those expressions like an inline repl. But the documentation of LSPs has been tough so far. Including on the official lsp spec website. So maybe you can help me better.

I've been able to implement this functionality in 2 ways already, but each way doesn't completely give me what I would like or expect.

1. Via codelens

I implement the TextDocumentCodeLens which triggers code commands. But the problem is that in the WorkspaceExecuteCommand handler, the return type is just an interface, so I'm completely clueless about how to update the document i'm in like with code actions. So I end up editing the file on disk in the execute command function, but then the user has to reload the file from disk to see the changes i made, or risk making more changes and overwritting the files on disk.

Is there a type/data that i can return from executeCommand, to make it edit the current text document instead of by passing the editor to directly edit the file on disk?

2. I've also been able to implement the functionality via code actions, and with code actions I can return the protocol.Edit values, where i define a range in the document and it's new text content. This works very smoothly with no issues. The problem is that, unlike with code lenses, I don't get any visual cue telling me that the comment line can be evaluated or refreshed. So I figured maybe we need to manually publish the diagnostics, but I have been unable to figure out how to trigger publishing diagnostics with this library as well. Could you help? Or just point me to resources?

This is just what I was looking for just now! I thought I'd let you know instead of staying silent!

Albeit in early stage, your project is looking very promising!

It would be helpful to have a list of supported features to know if glsp could be a good fit for a given project. I'm trying to figure out if I should go with glsp or https://github.com/sourcegraph/go-lsp.

I would be willing to contribute as well if something I need is missing.

The ContentChanges of DidChangeTextDocumentParams are of any type (interface{}). The documentation for the field does not specify what types it may hold, but the source code does. By moving the comment into the documentation for the field, users of the library may easily find what types the field may hold.

What's the reasoning behind some CompletionItem members like Kind, Details, Deprecated, Preselect, etc... are defined as pointers?

It seems like glsp does not implement this method?

I used the minimal example, added textDocument sync methods (like open close etc), and then wanted to do simple color highlighting of some keywords in my language.
So I implemented the and added the functions TextDocumentColor and TextDocumentColorPresentation to the protocol.Handler, but vscode gives me the error message

Request textDocument/colorPresentation failed.
    Message: method not supported: textDocument/documentColor
    Code: -32061

I looked into the source code of this package and infact it implements method textDocument/color and not textDocument/documentColor as stated in the specification

It is likely that this is an Issue in my code, but maybe it is in the package.
Can you look into it? Maybe it is just a typo in textDocument/color which should have been textDocument/documentColor.

Fix me if I'm wrong but on a client I can do client.sendRequest("foobar", {}) which mean it should be possible to defined custom handler. But looking at Handler interface implementation in the lib it looks like there's a switch that simply do return in case there is no match with predefined paths. Or Am I wrong?

Maybe it would make sense to someone who's done a lot of other language parsers using a lot of other libraries. It's completely useless to a beginner. "All you need to do, then, is provide the features for the language you want to support", it says, in one of the ten lines of documentation. It doesn't hint at how.

I would give my back teeth for a simple demo that put red wiggly lines under the word foo in any file with extension .bar, I could take it from there. I'm going to use the first library I can find which has such an example, except I'm starting to doubt there is one. It seems like everyone who's ever spent months of effort writing an LSP library in Go has then decided to ensure that all their effort will be wasted and useless by not spending another half-an-hour on writing any docs.