ocaml / ocaml-lsp

OCaml Language Server Protocol implementation
Other
749 stars 117 forks source link

Construct custom query #1348

Open xvw opened 1 month ago

xvw commented 1 month ago

At the moment, construct is a completion hook, and in some cases accessing it via a one-off query can be useful.

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 4485

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/custom_requests/req_construct.ml 31 44 70.45%
<!-- Total: 31 44 70.45% -->
Totals Coverage Status
Change from base Build 4483: 0.08%
Covered Lines: 5556
Relevant Lines: 25435

💛 - Coveralls
rgrinberg commented 1 month ago

When you say "can be useful", do you mean you have a concrete user in mind? Perhaps a client where you are planning to use this.

xvw commented 1 month ago

When you say "can be useful", do you mean you have a concrete user in mind? Perhaps a client where you are planning to use this.

Using Lsp through Emacs (that does not rely to much on hover and interactivy on top of mouse) can make the usage of construct cumbersome so I'll probably rely on a custom request to use it when I'll move to emacs + lsp (instead of emacs + merlin).

rgrinberg commented 1 month ago

I don't doubt the potential. What I'm saying is that until some client actually demonstrates real intention in using your custom request, you're just adding dead code. In the lsp standard itself, there's a requirement to demonstrate at least one client implementation for every additional protocol extension. We don't have to go that far, but how about we at least get some feedback from at least one actual client developer that is going to use this custom request before jump the gun.

xvw commented 1 month ago

@rgrinberg We expect different clients to use that query:

rgrinberg commented 1 month ago

Okay, so what I'm getting is that the only client developer we need to consider is from JS. So let's have somebody from there have a look at this request to do a sanity check.

rgrinberg commented 3 weeks ago

cc @awilliambauer