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

[Adam Porter <adam@alphapapa.net>]

[-- Attachment #1: Type: text/plain, Size: 3511 bytes --]

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 #2: jsonrpc-hacks.el --]
[-- Type: text/x-emacs-lisp, Size: 1862 bytes --]

;; This code basically copies directly from `jsonrpc' while removing
;; some parts related to sending the HTTP-like headers.

(defclass hyperdrive-jsonrpc-process-connection (jsonrpc-process-connection)
  nil)

(cl-defmethod initialize-instance ((conn hyperdrive-jsonrpc-process-connection) slots)
  (cl-call-next-method)
  (cl-destructuring-bind (&key ((:process proc)) name &allow-other-keys) slots
    (set-process-filter proc #'hyperdrive-jsonrpc--process-filter)))

(defun hyperdrive-jsonrpc--process-filter (proc string)
  (when (buffer-live-p (process-buffer proc))
    (with-current-buffer (process-buffer proc)
      (let* ((inhibit-read-only t)
             (connection (process-get proc 'jsonrpc-connection)))
        ;; Insert the text, advancing the process marker.
        (save-excursion
          (goto-char (process-mark proc))
          (insert string)
          (set-marker (process-mark proc) (point)))
        (condition-case err
            (when-let ((json-message (jsonrpc--json-read)))
              (with-temp-buffer
                (jsonrpc-connection-receive connection json-message)))
          (error (message "hyperdrive-jsonrpc--process-filter ERROR:%S  PROC:%S  STRING:%S"
                          err proc string)))))))

(cl-defmethod jsonrpc-connection-send
  ((connection hyperdrive-jsonrpc-process-connection) &rest args
   &key _id method _params _result _error _partial)
  "Send MESSAGE, a JSON object, to CONNECTION."
  (when method
    (plist-put args :method
               (cond ((keywordp method) (substring (symbol-name method) 1))
                     ((and method (symbolp method)) (symbol-name method)))))
  (let* ((message `(:jsonrpc "2.0" ,@args))
         (json (jsonrpc--json-encode message)))
    (process-send-string (jsonrpc--process connection) json)
    (jsonrpc--log-event connection message 'client)))