emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: jsonrpc.el uses "Content-Length" headers which seem specific to LSP


From: Adam Porter
Subject: Re: jsonrpc.el uses "Content-Length" headers which seem specific to LSP
Date: Thu, 19 Oct 2023 15:59:59 -0500
User-agent: Mozilla Thunderbird

Hi João,

On 10/18/23 19:11, João Távora wrote:

Instead of jsonrpc.el, I think you mean jsonrpc-process-connection.  It's
a subclass of jsonrpc-connection.  If you want more genericity, you could
subclass jsonrpc-connection.

Yes, thanks.

To give you an example, in a private project I subclassed
jsonrpc-connection to make a websocket-based JSONRPC transport.
It was really easy (I can show the code if you're interested).

Sure, that could be helpful.

Presumably you're interested in using an Emacs pipe or network process
as the transport, so by doing that you'd find yourself reinventing lots of
stuff in the methods for jsonrpc-process-connection.  Let's label this idea A.

Yes, we're using a network process connecting to a local TCP server. Indeed, we didn't want to reinvent those things.

Now, the flaw I will concede about jsonrpc-process-connection is that it
describes itself as a "jsonrpc connection over an Emacs process", but it
would be more accurately described as "JSONRPC connection over HTTP-like
over Emacs process".  Presumably, you want the first and the last part
but not the middle.

Right, the code we wrote (shared below) basically removes those middle parts.

I guess we could express the existing
jsonrpc-process-connection as a mixin of HTTP-like behaviour and Emacs
process behaviour.  If we did that, you could more elegantly do your
own hyperdrive-jsonrpc-connection by mixing in just the classes you wanted.
So this is idea B.
>
Back to idea A, we could just bundle most of the functionality used by
the generics operating on jsonrpc-process-connection as free helper
functions. Then you could just easily build on top of jsonrpc-connection
the process-related stuff you want to do and the existing
jsonrpc-process-connection would also use these helpers.

Nevertheless, I think idea B is cleaner.  From reading your email,
you seem to agree with me.

Right, we wanted to avoid the refactoring that would require.

But I have to see your code first.  You seem to have already come up
with some kind of idea C. You say the code is "pleasantly  simple",
yet you still think it could be easier, so I don't know.  I haven't
seen your pleasantly-simple-but-not-quite-optimal code.

Of course.  Attaching that code below.  :)

I do know subclassing and removing behaviour violates the "is a"
of inheritance and sometimes brings you into trouble
later on.  Or maybe not.

Yes, that is an ugly compromise. For the minor changes we had to make, it seemed reasonable within our project, but we'd be glad if it weren't necessary.

So, to summarize, changes to the transport implementor's API in
of jsonrpc.el are possible, so long as the current API isn't
broken and the new things, be it helpers of idea A or new classes of
idea B are documented in the manual.  So show me your code and propose
some patch to jsonrpc.el showing how much it would simplify your
existing code.

So, here's our code. As you can see, it just copies some forms from jsonrpc and removes the parts related to the HTTP-like headers. It seems to work so far, but I don't know if it's good or close to correct. :)

Of course, one of the considerations is backward compatibility with existing users of jsonrpc. There don't appear to be very many yet, other than Eglot, so some kind of transition might be feasible, especially since jsonrpc.el is on GNU ELPA.

Thanks for your work and your help with this.

Adam

Attachment: jsonrpc-hacks.el
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]