Async jsonrpc #40
Async jsonrpc #40
Conversation
svaante
force-pushed
the
async-jsonrpc
branch
from
933b711
to
890e9ed
Compare
|
No need to be sorry :-) I'm fixing the bug you reported in |
svaante
force-pushed
the
async-jsonrpc
branch
2 times, most recently
from
4001b30
to
b03f7bb
Compare
| "Send request with COMMAND and ARGUMENTS to adapter CONN. | ||
| If callback function CB is supplied, it's called on timeout | ||
| and success. See `dape--callback' for signature." | ||
| (jsonrpc-async-request conn command arguments |
There was a problem hiding this comment.
I would consider using jsonrpc-request as my main workhorse. jsonrpc-async-request only seems to be necessary in the REPL, as far as I can tell, because you don't want to block the UI but you also don't want to cancel the request either.
| @@ -1186,12 +1098,12 @@ See `dape--callback' for expected CB signature." | |||
| :lines (apply 'vector lines)) | |||
| cb))) | |||
|
|
|||
| (defun dape--set-exception-breakpoints (process cb) | |||
| "Set the exception breakpoints in adapter PROCESS. | |||
| (defun dape--set-exception-breakpoints (conn cb) | |||
There was a problem hiding this comment.
These functions which take cb callback-passing style are IMO harder to read than imperative counterparts. I think you get the point from my other PR.
| `(lambda (&optional process body success msg) | ||
| (ignore process body success msg) | ||
| `(lambda (&optional conn body success-p msg) | ||
| (ignore conn body success-p msg) | ||
| ,@body)) | ||
|
|
||
| (defmacro dape--with (request-fn args &rest body) |
There was a problem hiding this comment.
dape--with and dape--callback are odd unidiomatic macros. dape--callback in particular is "anaphoric" meaning it establishes variables "magically". I don't absolutely hate anaphoric macros but I don't like them either (Paul Graham's https://letoverlambda.com/ chapther 6 introduces some). But I think it's better to keep them small and not problem-domain-specific, like here.
My understanding you try to use these macro tricks to hide the ugliness from the jsonrpc-async-request CB nature (which your previous parser also needed). But it doesn't really work in my opinion. I can't reorder code naturally like I would in an imperative approach.
Nor to mention it's very tough to understand the flow if one of these requests fails.
|
Hi @svaante I've left some very generic comments, which probably are not unexpected. My opinion is that in the longer run, async call-back passing jungles become hard to manage, but maybe you have some good reason to prefer this over #37 , which is how I think |
I am deeply sorry João
I know you think this an inferior solution... and I am butchering the transforms.
There is some more stuff that I need to do before I am happy.