bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 25.05.2020 20:04, João Távora wrote:
> (add-hook 'eldoc-documentation-functions
>           #'test-eldoc-async 0 t)
>
> (defun test-eldoc-async ()
>   (cons :async
>         (lambda (cb)
>           (funcall cb "doc here!"))))

Thanks.  As I told you, it's not bad.  These aren't exactly "futures"
though, they're a way to simulate argument passing.  I prefer my
version, which matches what flymake.el, url.el, jsonrpc.el, sly.el and
others do.

> If you like, we could simplify the returned objects to be just FETCHER
> (as documented in the patch) rather than (:async . FETCHER). But the 
> latter seems more explicit.

Yes, if we do go with your version, I'd like that.  The latter is hard
to read and understand from the docstring.

Before I go any further I'd like Stefan and Andrey (or others) to weigh
in.  I don't have a lot of time to invest here, so if there is another
vote for your proposal, I'm not going to wrestle here.

To simplify, hopefully you agree that your proposal can be summarized
as:

  "return a function that receives a function that you
   should call with the docstring"

whereas my proposal can be summarized as:

  "receive a function that you should call with the docstring"

> There also exist a possible modification of this patch with a bit of
> functional programming where both calls to eldoc--handle-multiline 
> happen from inside of eldoc-documentation-default's definition.

Yes, that's independent of the shape of the callback we want to use.
I'll leave that for later.  Specifically, eldoc--handle-multiline has to
do quite a bit more handling (to satisfy Andrey's expectations).

Replying to parts from the other discussion in the Github tracker now.

> OK, another question: if the result still /valid/?
                        ^^ Assuming you mean "is".

Well, if the client discovers the result to be invalid, it can not call
the callback, or signal an error.  If it is valid, call the callback.

> No idea, a hypothetical one. It's an open API, not everybody is or
> will be using LSP until the end of time. And it doesn't really have to
> be huge. A saving is a saving.

There's no free lunch.  A very small saving in time for more complexity
is bad.  That's what overengineered means.

> You can certainly kill the external process outside of it. Saving on
> CPU expenses in general.

The future's creditor is the only one who could do that to any useful
effect.  Does it have access to the process?  Probably not.  You would
have to return a complex future with a kill switch.  That's possible,
but tricky, because you'd then have to be ready in the sentinel for
another source of unexpected kills.

> > For a poor man's async primitive, I prefer my version

> So even the code savings didn't convince you? Both in eldoc.el,

I do see minimal code savings in eldoc.  You do remove a special
variable (which is _not_ the same as a global variable, btw).

I do see a much more complicated docstring, where the reader has to wrap
his head around a 2-deep functional conundrum, whereas my version was
1-deep.  Nothing special, but a VERY common source of headaches.

Let's take your trivial example:

    (defun test-eldoc-async ()
       (cons :async
            (lambda (cb)
               (funcall cb "doc here!"))))

It isn't really representative of what a function that needs async would
have to do, is it?  Because, if you really wanted this very example,
then it's much better to do the one-liner:

   (defun test-eldoc-async () "doc here!")

Rather, presumably you would use this to fetch something from an HTTP
server or so:

    (defun test-eldoc-async ()
      (cons :async
        (lambda (cb)
           (url-retrieve-thingy "http://test-signature"; cb))))

Where url-retrieve-thingy is very similar to our url-retrieve. Right?
But why have that awkward :async there when a function is a first class
object that we can identify with the functionp predicate?  Let's just:

    (defun test-eldoc-async ()
       (lambda (cb)
          (url-retrieve-thingy "http://test-signature"; cb)))

And at this point one wonders why the heck not

  (defun test-eldoc-async (cb)
     (url-retrieve-thingy "http://test-signature"; cb))

?

> and likely in doc functions as well

No.  Unless I am completely mistaken (I might be), in the "doc function"
not only are there no savings, but complication.  This makes sense
because you just inverted the responsibility: the doc function now has
to "beg" for the argument that used to be given to it naturally.

So, it's just a functional gimmick.  As good as the next one, but a
gimmick all the same.  Until the "futures" are here, people will
potentially bang heads in an anguished "WHY??".

Why indeed?  Your other argument, that this makes the transition to
proper futures (such as the ones in https://github.com/skeeto/emacs-aio)
easier, is questionable, too.  There are two scenarios here:

- You want to keep backward compatibility to this API published in eldoc
  1.1.0 until the time of the Emacs 28 release:

  This is something that I -- and Stefan, if I'm not mistaken, -- don't
  think we should worry about.  Just because a package is :core GNU ELPA
  doesn't necessarily mean we guarantee stability of its API.

  But if we do, then we'll have to explain in the docstring that there
  is a fourth return value for the hook functions.  In my version we'll
  have to do exactly the same.

- You don't want to keep backward compatibility until Emacs 28:

  Then, when the super-futures are here, you can just kill the CALLBACK
  arg if we find it doesn't fit and rewrite the docstring without
  concerns.

João