From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.devel Subject: Re: New jrpc.el JSONRPC library Date: Thu, 24 May 2018 12:02:33 +0200 Message-ID: References: <87lgcrqgvz.fsf@gmail.com> <831seipvdb.fsf@gnu.org> <871se9lyid.fsf_-_@gmail.com> <87d0xqs56o.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000dfd19a056cf0c0d8" X-Trace: blaine.gmane.org 1527156134 1813 195.159.176.226 (24 May 2018 10:02:14 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 24 May 2018 10:02:14 +0000 (UTC) Cc: Eli Zaretskii , monnier@iro.umontreal.ca, vibhavp@gmail.com, emacs-devel@gnu.org To: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu May 24 12:02:09 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fLn4S-0000LF-W6 for ged-emacs-devel@m.gmane.org; Thu, 24 May 2018 12:02:09 +0200 Original-Received: from localhost ([::1]:37531 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fLn6a-0005LD-7g for ged-emacs-devel@m.gmane.org; Thu, 24 May 2018 06:04:20 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fLn5L-0004fT-Es for emacs-devel@gnu.org; Thu, 24 May 2018 06:03:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fLn5F-0007Lq-4p for emacs-devel@gnu.org; Thu, 24 May 2018 06:03:03 -0400 Original-Received: from mail-ot0-x233.google.com ([2607:f8b0:4003:c0f::233]:34837) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fLn56-0007Js-Qe; Thu, 24 May 2018 06:02:49 -0400 Original-Received: by mail-ot0-x233.google.com with SMTP id h8-v6so1230546otb.2; Thu, 24 May 2018 03:02:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=I3+zyqao0fkXTDGik3B5XzC+7dk8Cmk7PPpYy4Jl11A=; b=fd3xGdfqnW6i1tmalM1+LtIndm3ZsOC83W1yc07xZnNMOj3vB2e+H7IlgIl8QGFjX7 pLI+TUypmWxE6mCTuD8g1S/gW8rheeA2GuvpjBVl1BHuY3PP/UqHTB8ZS7Cf+SqNMvWn cPcWKFqNInKrqRWbb6293QyzpeEDxy622Fcxa1OQxSVNOxtkO8fhA9gNDVqPIK3rVU2b 8J407M2q261XlPXxwzvJg09JAi/No0hEBvXPKFAw1TJphzCwaF+WvVrVNjM7tXp3m8Rh zMZrtrSD6xaFGCvuBMiry/IjMMkdxGLe/y/GPMKN29EoQI9ssnjBXvhPAbpsJAB23cEX ZcdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=I3+zyqao0fkXTDGik3B5XzC+7dk8Cmk7PPpYy4Jl11A=; b=AMJsMPdHl3SjgeDmYlVpomWoDQQUeLBL0S+rf+YkuhAG02JUYtJewwDaUHfXEaSUBz PdUrCZhDb9WmiAM5XnEg78F7HjZR+UfIFy9Ybda+RR6epHUepOAdoyXu0bgCXjkG+8pz 86VFQBkWFb5wxT+PsY7cnzkjXoX+fJ0EVFjT9d/Fh0AEum4LjXnEERm/pIVAGS4tLS18 NDFyTMdgyVm9Ope01jh1E5iufHHBsvFgG/fueISMTXD8v3KvtlPXPSsqeEApswANvClQ rzAJPtqODEM2mMGBIq9xX8K94R6c46i6shH/kPzf0lgdOLbCZhXjrT/Yh2pMMgfgobLw djXA== X-Gm-Message-State: ALKqPwcQFhcMyw81k/CHALxf/UmyzmrZzWONcFtDyJ/bUN86NYO75Rdy j/DuDTKSIIEMEiGYuSv5AXLtLLDz6JwTdKFeqO0= X-Google-Smtp-Source: AB8JxZpqVLURjCVP/Fey4O07BY0BrU7sLz6uv2RYQSoKb0mw74gEnIxf8XGbaZK0shn28NgDkMSLHiwbUg5eZemyS4M= X-Received: by 2002:a9d:255b:: with SMTP id j27-v6mr4420191otd.316.1527156167396; Thu, 24 May 2018 03:02:47 -0700 (PDT) In-Reply-To: <87d0xqs56o.fsf@gmail.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4003:c0f::233 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:225647 Archived-At: --000000000000dfd19a056cf0c0d8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks! Jo=C3=A3o T=C3=A1vora schrieb am So., 20. Mai 2018 u= m 17:44 Uhr: > Philipp Stephani writes: > > > Jo=C3=A3o T=C3=A1vora schrieb am Fr., 18. Mai 20= 18 um > 18:28 Uhr: > > > >> I propose this library, jrpc.el, for Emacs core (or GNU ELPA, or > > > Thanks. I think that separating out JSONRPC from LSP and having a > > JSONRPC library in core are highly valuable additions that I'd > > strongly support. Some review comments follow; sorry that there are > > so many, but I think as a foundational library jrpc.el should be as > > high-quality as possible: > > Apology *not accepted* :-). Having reviewers provide so many insightful > comments is a previlige and a pleasure. The high standards of the Emacs > project are one of the reasons I like to contribute to it. > > I'll triaged your comments into "No", "Maybe", "Ok". The former require > some more convincing on your part :-) > > I've already applied some of your suggestions (and some that Stefan sent > off-list, too). See the jsonrpc-refactor branch of Eglot's github > repository. The latest version of jsonrpc.el (I changed the name) lives > in: > > > https://raw.githubusercontent.com/joaotavora/eglot/jsonrpc-refactor/jsonr= pc.el Thanks! Could you maybe use GitHub PRs for further changes? I find it much easier to comment on them instead of via mail. > > > > - Please add extensive unit and integration tests. There are lots of > > corner cases (what happens when the server times out, sends an invalid > > or incomplete response, sends responses in the wrong order, etc.) that > > should all be tested. > > OK. It's in the plans, if not in the stars. Help appreciated. :-) We can > convert some of those examples to tests. Eglot's jsonrpc-refactor > branch, where I'm currently developing jsonrpc.el, already has > integration tests, so at least a part of jsonrpc.el is already covered. > It's OK to add the tests once the API stabilizes. It would be great to have tests e.g. for - server process crashes - server takes too long to respond - invalid responses - mismatched IDs - very large responses etc., just the usual failure modes. > > > - Please remove all defcustoms and defvars. jrpc.el is a low-level > > library, it doesn't make sense for it to maintain global state or be > > globally customizable. Rather, the timeout could either be a mandatory > > argument, or a reasonable constant default could be selected, and the > > other variables should be per-process. > > OK. Makes sense. Done. > > > - Also please remove `jrpc-current-process'. There is no general > > notion of a "current" process; > > Maybe. Not convinced. It's opininated, sure. It supposes that just most > JSONRPC applications would have the notion of a "session". So jsonrpc.el > makes this suggestion to its users, allowing them to configure what a > session means for them. They would also get access to M-x > jsonrpc-events-buffer. This is entirely optional on the client's part, > but I see no harm in JSONRPC providing this convenience. Perhaps the > name is off and should mention "session". > When reading the code, I was a bit confused to see the notion of a "current process". Later, I realized that it's only used for some debug helpers and not for the core API. I think it would make sense to separate the debug helpers from the core API (connect, send, receive) more cleanly. I'm even wondering whether the debug helpers are needed at all. Probably you saw a need for them, otherwise you wouldn't have added them. But maybe at least in the beginning Emacs's builtin debug functionality (edebug etc.) is already good enough? If at all, please separate the debug helpers from the main API (with a comment block or similar) and move them to the end, as they are not essential. > > > - Please don't use the `error' symbol directly. Instead, define your > > own error symbols for each error category using `define-error'. Only > > then clients have the chance to catch and handle errors. > > OK. Done. Though technically they can already catch and handle > them. What they can't do is catch *only* JSONRPC errors and let the > others thru. I happen to think that's a bad idea because a JSONRPC error > is an error in your program. But there's no reason to deny this, too. > When designing a general-purpose API, it's often unknown how it will be used; often it's used in unpredicted and surprising way. Even if we don't see a good use case to catch JSON-RPC errors now, it's quite likely that such a use case will show up in the future once the library is more widely used, and then the very minor cost of defining specific error symbols pays off. Please also use specific error symbols in `jsonrpc-error'. > > > - `jrpc-define-process-var' isn't JSONRPC-specific, it should be moved > > somewhere else (subr.el)? > > Yes, but if we do this it'll be harder for clients like eglot.el to keep > supporting Emacs 26.1 (assuming jsonrpc.el would be in GNU ELPA and > master simultaneously). > > > - Instead of defining process variables, consider using a structure or > > EIEIO class to encapsulate all needed state for the process client. > > > Maybe. Structures is a bad idea, but defclass isn't at all. > I'm not so much opposed to structures, it's more of an implementation detail and a matter of taste whether you prefer structures or classes. However, there's a pretty strong argument that I forgot about: for robustness you need a way to restart process and even have them restarted automatically. Let's say a server process crashes or the user kills it; you definitely don't want LSP/JSON-RPC to be unusable until the user restarts Emacs. Therefore, instead of having the 'connect' function create the process, the send and receive functions should ensure that it's running; i.e. shift the paradigm from "edge-triggered" (process is created once the user calls a function) to "level-triggered" (all functions that need a process make sure the process is available). That is at the same time more robust and easier to understand. However, it practically requires using a structure or class as main data type, because the process object needs to be replaced after creating the client. > > > - Please make it possible to run servers remotely. Unfortunately this > > isn't possible with `make-process', so you have to use > > `start-file-process' instead if `default-directory' is remote. As an > > alternative to using `default-directory', consider passing a > > REMOTE-DIRECTORY argument to `jrpc-connect' so that the behavior > > doesn't depend on ambient context. > > Maybe. I don't fully understand the example. But jsonrpc-connect allows > you to > pass any pre-connected process object. It will replace its buffer, > sentinel and filter. In the "autostart-server" branch of eglot.el, for > example, there is a function to I start a local process with special > arguments to that it open a server, then connect to it, then return that > connected process. This is for Windows LSP servers that can't to stdio > comms. > It's good to have such an extensibility mechanism, and I indeed overlooked that you can pass a process object to the connect function. I'm wondering whether that should indeed be the *only* option: leave the process creation business entirely to the client, and only accept a process object (or rather, process factory function, because you want to restart processes as necessary, see above). That is, have the following interface instead of the 'connect': (defun jsonrpc-make-client (process-factory) "PROCESS-FACTORY gets called an unspecified number of times and has to return a new live process object each time it's called." ...) Optionally you could provide convenience functions to return such factories for `make-process' etc., but you could also leave that to the users entirely. > > > - I think many of the names can be internal (i.e. contain a double > > dash) unless they are explicitly part of the public API. > > Maybe. What names? The names that I want to make public are already > public, and all the others aren't. > I'm thinking about things like jsonrpc-warn, jsonrpc-outstanding-request-ids, etc. These seem like implementation details that clients shouldn't care about. > > > - Probably jrpc-async-request should just return nil. Or is there a > > situation where callers have to know about IDs or timers? I think > > those should be treated as implementation details and not be exposed > > to clients. > > OK. Makes sense. > > > - Please use hashtables or alists instead of plists. There are many > > more functions to operate on hashtables and alists, > > Maybe. Hmm, I really really don't know about this. Have you tried > cl-destructuring-bind with plists? It really is the SH&T for JSON. > Not really, how would it work? Typically I use plain gethash, assq, etc. Or `let-alist'. > > > and json.c doesn't support plists. > > That's the superior argument. I didn't know about json.c until very > recently. I'll play along, but can you help me rewrite jsonrpc-lambda to > something less consing perhaps? > > (cl-defmacro jsonrpc-lambda (cl-lambda-list &body body) > (declare (indent 1) (debug (sexp &rest form))) > (let ((jsonobj (gensym "jsonrpc-lambda-elem"))) > `(lambda (,jsonobj) > (apply (cl-function (lambda ,cl-lambda-list ,@body)) > (let (plist) > (maphash (lambda (k v) > (push v plist) > (push (if (keywordp k) k > (intern (format ":%s" k))) > plist)) > ,jsonobj) > plist))))) > Why do we need jsonrpc-lambda? Writing a normal lambda and some gethash or assq calls doesn't sound overly bad to me. I'd just remove jsonrpc-lambda entirely; it's the kind of syntactic sugar that I think doesn't make code significantly more readable. > > > - The HTTP-style enveloping (using HTTP headers to separate responses) > > is technically not part of JSONRPC. You could either add support for > > other enveloping methods (though I'm not aware of any), or at least > > call this out in the documentation. > > OK. Yes, it's most definetly not JSONRPC. It's an LSP thing. This is the > reason I need to rework the "process" object into a proper defclass and > make generic functions that only apply this enveloping to the proper > LSPlike objects (which could/should be the default object types returned > by jsonrpc-connect). > > > - I think `string-bytes' isn't guaranteed to return the number of > > UTF-8 bytes. Rather you should encode the string explicitly to a > > unibyte string using `encode-coding-string', and then use > > `no-conversion' for the process coding systems. > > > I think Eli has already answered this authoritatively. > > > - Consider also sending (and parsing) a Content-Type header: > > Content-Type: application/vscode-jsonrpc; charset=3Dutf-8. > > OK. > > > - I don't quite understand how `jrpc-ready-predicates' is supposed to > > be used. Are clients supposed to have some form of server-specific > > sidechannel to detect when the server is ready? > > Yes that's precisely it (though it's entirely optional on the client > part). > > > It might be simpler to solve this problem in Emacs core, e.g. by > > adding a timeout to `process-send-string'. > > No, the "ready" here is for the application, not the transport. > > I understand it's obscure for a reader, so let me explain and then this > might go into the doc. Obviously, this came out of necessity in > eglot.el, but I think its good functionality to keep in jsonrpc.el > refactor (and it'd be really hard to implement outside it.) > If it should stay in jsonrpc.el, then it should be clear without referring to eglot.el. > > The problem arises in the ordering of sending multiple asynchronous > JSONRPC requests. So, for example in LSP, it doesn't make sense to send > most requests if there are outstanding changes in the buffer that you > haven't told the server about. But because the latter can come in > through idle timers, it's useful to have this generic :deferred > mechanism: you just put in `jsonrpc-ready-predicates' a function that > returns nil if there are outstanding changes and then write code without > worrying about whether the server is ready in that particular > situation. This is better than sprinkling the code with > "send-outstanding-changes-just-to-be-sure" before you make requests. > Hmm. I need to think a bit more about this. I guess it makes sense, but please make it a property of the client, not a global variable for now. --000000000000dfd19a056cf0c0d8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks!

Jo=C3=A3o T=C3=A1vora <joaotavo= ra@gmail.com> schrieb am So., 20. Mai 2018 um 17:44=C2=A0Uhr:
Philipp Stephani <p.stephani2@gmail.com> writes= :

> Jo=C3=A3o T=C3=A1vora <joaotavora@gmail.com> schrieb am Fr., 18. Mai 2018 um = 18:28 Uhr:
>
>>=C2=A0 I propose this library, jrpc.el, for Emacs core (or GNU ELPA= , or

> Thanks. I think that separating out JSONRPC from LSP and having a
> JSONRPC library in core are highly valuable additions that I'd
> strongly support.=C2=A0 Some review comments follow; sorry that there = are
> so many, but I think as a foundational library jrpc.el should be as > high-quality as possible:

Apology *not accepted* :-). Having reviewers provide so many insightful
comments is a previlige and a pleasure. The high standards of the Emacs
project are one of the reasons I like to contribute to it.

I'll triaged your comments into "No", "Maybe", &quo= t;Ok". The former require
some more convincing on your part :-)

I've already applied some of your suggestions (and some that Stefan sen= t
off-list, too). See the jsonrpc-refactor branch of Eglot's github
repository. The latest version of jsonrpc.el (I changed the name) lives
in:

https://raw.githubuser= content.com/joaotavora/eglot/jsonrpc-refactor/jsonrpc.el

Thanks! Could you maybe use GitHub PRs for further change= s? I find it much easier to comment on them instead of via mail.
= =C2=A0


> - Please add extensive unit and integration tests. There are lots of > corner cases (what happens when the server times out, sends an invalid=
> or incomplete response, sends responses in the wrong order, etc.) that=
> should all be tested.

OK. It's in the plans, if not in the stars. Help appreciated. :-) We ca= n
convert some of those examples to tests.=C2=A0 Eglot's jsonrpc-refactor=
branch, where I'm currently developing jsonrpc.el, already has
integration tests, so at least a part of jsonrpc.el is already covered.
=

It's OK to add the tests once the API = stabilizes. It would be great to have tests e.g. for
- server pro= cess crashes
- server takes too long to respond
- inval= id responses
- mismatched IDs
- very large responses
etc., just the usual failure modes.
=C2=A0

> - Please remove all defcustoms and defvars. jrpc.el is a low-level
> library, it doesn't make sense for it to maintain global state or = be
> globally customizable. Rather, the timeout could either be a mandatory=
> argument, or a reasonable constant default could be selected, and the<= br> > other variables should be per-process.

OK. Makes sense. Done.

> - Also please remove `jrpc-current-process'. There is no general > notion of a "current" process;

Maybe. Not convinced. It's opininated, sure. It supposes that just most=
JSONRPC applications would have the notion of a "session". So jso= nrpc.el
makes this suggestion to its users, allowing them to configure what a
session means for them. They would also get access to M-x
jsonrpc-events-buffer. This is entirely optional on the client's part,<= br> but I see no harm in JSONRPC providing this convenience. Perhaps the
name is off and should mention "session".
When reading the code, I was a bit confused to see the notion = of a "current process". Later, I realized that it's only used= for some debug helpers and not for the core API.
I think it woul= d make sense to separate the debug helpers from the core API (connect, send= , receive) more cleanly. I'm even wondering whether the debug helpers a= re needed at all. Probably you saw a need for them, otherwise you wouldn= 9;t have added them. But maybe at least in the beginning Emacs's builti= n debug functionality (edebug etc.) is already good enough? If at all, plea= se separate the debug helpers from the main API (with a comment block or si= milar) and move them to the end, as they are not essential.
=C2= =A0

> - Please don't use the `error' symbol directly. Instead, defin= e your
> own error symbols for each error category using `define-error'. On= ly
> then clients have the chance to catch and handle errors.

OK. Done. Though technically they can already catch and handle
them. What they can't do is catch *only* JSONRPC errors and let the
others thru. I happen to think that's a bad idea because a JSONRPC erro= r
is an error in your program. But there's no reason to deny this, too.

When designing a general-purpose API, it= 's often unknown how it will be used; often it's used in unpredicte= d and surprising way. Even if we don't see a good use case to catch JSO= N-RPC errors now, it's quite likely that such a use case will show up i= n the future once the library is more widely used, and then the very minor = cost of defining specific error symbols pays off.
Please also use= specific error symbols in `jsonrpc-error'.
=C2=A0

> - `jrpc-define-process-var' isn't JSONRPC-specific, it should = be moved
> somewhere else (subr.el)?

Yes, but if we do this it'll be harder for clients like eglot.el to kee= p
supporting Emacs 26.1 (assuming jsonrpc.el would be in GNU ELPA and
master simultaneously).

> - Instead of defining process variables, consider using a structure or=
> EIEIO class to encapsulate all needed state for the process client. >
Maybe. Structures is a bad idea, but defclass isn't at all.

I'm not so much opposed to structures, it'= s more of an implementation detail and a matter of taste whether you prefer= structures or classes.
However, there's a pretty strong argu= ment that I forgot about: for robustness you need a way to restart process = and even have them restarted automatically. Let's say a server process = crashes or the user kills it; you definitely don't want LSP/JSON-RPC to= be unusable until the user restarts Emacs. Therefore, instead of having th= e 'connect' function create the process, the send and receive funct= ions should ensure that it's running; i.e. shift the paradigm from &quo= t;edge-triggered" (process is created once the user calls a function) = to "level-triggered" (all functions that need a process make sure= the process is available). That is at the same time more robust and easier= to understand. However, it practically requires using a structure or class= as main data type, because the process object needs to be replaced after c= reating the client.
=C2=A0

> - Please make it possible to run servers remotely. Unfortunately this<= br> > isn't possible with `make-process', so you have to use
> `start-file-process' instead if `default-directory' is remote.= As an
> alternative to using `default-directory', consider passing a
> REMOTE-DIRECTORY argument to `jrpc-connect' so that the behavior > doesn't depend on ambient context.

Maybe. I don't fully understand the example. But jsonrpc-connect allows= you to
pass any pre-connected process object. It will replace its buffer,
sentinel and filter. In the "autostart-server" branch of eglot.el= , for
example, there is a function to I start a local process with special
arguments to that it open a server, then connect to it, then return that connected process. This is for Windows LSP servers that can't to stdio<= br> comms.

It's good to have such an ex= tensibility mechanism, and I indeed overlooked that you can pass a process = object to the connect function.
I'm wondering whether that sh= ould indeed be the *only* option: leave the process creation business entir= ely to the client, and only accept a process object (or rather, process fac= tory function, because you want to restart processes as necessary, see abov= e). That is, have the following interface instead of the 'connect':=

(defun jsonrpc-make-client (process-factory)
=C2=A0 "PROCESS-FACTORY gets called an unspecified number of tim= es and has to return a new live process object each time it's called.&q= uot;
=C2=A0 ...)

Optionally you could pr= ovide convenience functions to return such factories for `make-process'= etc., but you could also leave that to the users entirely.
=C2= =A0

> - I think many of the names can be internal (i.e. contain a double
> dash) unless they are explicitly part of the public API.

Maybe. What names? The names that I want to make public are already
public, and all the others aren't.

= I'm thinking about things like jsonrpc-warn,=C2=A0jsonrpc-outstanding-r= equest-ids, etc. These seem like implementation details that clients should= n't care about.
=C2=A0

> - Probably jrpc-async-request should just return nil. Or is there a > situation where callers have to know about IDs or timers? I think
> those should be treated as implementation details and not be exposed > to clients.

OK. Makes sense.

> - Please use hashtables or alists instead of plists. There are many > more functions to operate on hashtables and alists,

Maybe. Hmm, I really really don't know about this. Have you tried
cl-destructuring-bind with plists? It really is the SH&T for JSON.
<= /blockquote>

Not really, how would it work? Typically I = use plain gethash, assq, etc. Or `let-alist'.
=C2=A0

> and json.c doesn't support plists.

That's the superior argument. I didn't know about json.c until very=
recently. I'll play along, but can you help me rewrite jsonrpc-lambda t= o
something less consing perhaps?

(cl-defmacro jsonrpc-lambda (cl-lambda-list &body body)
=C2=A0 (declare (indent 1) (debug (sexp &rest form)))
=C2=A0 (let ((jsonobj (gensym "jsonrpc-lambda-elem")))
=C2=A0 =C2=A0 `(lambda (,jsonobj)
=C2=A0 =C2=A0 =C2=A0 =C2=A0(apply (cl-function (lambda ,cl-lambda-list ,@bo= dy))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let (plist)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (maphash (lambda (k= v)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0(push v plist)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0(push (if (keywordp k) k
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(intern (format ":= %s" k)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0plist))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0,jsonobj)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 plist)))))

Why do we need jsonrpc-lambda? Writing a normal= lambda and some gethash or assq calls doesn't sound overly bad to me. = I'd just remove jsonrpc-lambda entirely; it's the kind of syntactic= sugar that I think doesn't make code significantly more readable.
=C2=A0

> - The HTTP-style enveloping (using HTTP headers to separate responses)=
> is technically not part of JSONRPC. You could either add support for > other enveloping methods (though I'm not aware of any), or at leas= t
> call this out in the documentation.

OK. Yes, it's most definetly not JSONRPC. It's an LSP thing. This i= s the
reason I need to rework the "process" object into a proper defcla= ss and
make generic functions that only apply this enveloping to the proper
LSPlike objects (which could/should be the default object types returned by jsonrpc-connect).

> - I think `string-bytes' isn't guaranteed to return the number= of
> UTF-8 bytes. Rather you should encode the string explicitly to a
> unibyte string using `encode-coding-string', and then use
> `no-conversion' for the process coding systems.
>
I think Eli has already answered this authoritatively.

> - Consider also sending (and parsing) a Content-Type header:
> Content-Type: application/vscode-jsonrpc; charset=3Dutf-8.

OK.

> - I don't quite understand how `jrpc-ready-predicates' is supp= osed to
> be used. Are clients supposed to have some form of server-specific
> sidechannel to detect when the server is ready?

Yes that's precisely it (though it's entirely optional on the clien= t
part).

> It might be simpler to solve this problem in Emacs core, e.g. by
> adding a timeout to `process-send-string'.

No, the "ready" here is for the application, not the transport.
I understand it's obscure for a reader, so let me explain and then this=
might go into the doc.=C2=A0 Obviously, this came out of necessity in
eglot.el, but I think its good functionality to keep in jsonrpc.el
refactor (and it'd be really hard to implement outside it.)

If it should stay in jsonrpc.el, then it should be= clear without referring to eglot.el.
=C2=A0

The problem arises in the ordering of sending multiple asynchronous
JSONRPC requests. So, for example in LSP, it doesn't make sense to send=
most requests if there are outstanding changes in the buffer that you
haven't told the server about. But because the latter can come in
through idle timers, it's useful to have this generic :deferred
mechanism: you just put in `jsonrpc-ready-predicates' a function that returns nil if there are outstanding changes and then write code without worrying about whether the server is ready in that particular
situation. This is better than sprinkling the code with
"send-outstanding-changes-just-to-be-sure" before you make reques= ts.

Hmm. I need to think a bit more abo= ut this. I guess it makes sense, but please make it a property of the clien= t, not a global variable for now.=C2=A0
--000000000000dfd19a056cf0c0d8--