bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 26.05.2020 04:21, João Távora wrote:
>> Dmitry Gutov <dgutov@yandex.ru> writes:
> They also use *special* variables? Flymake's API looks fairly clean
> (you call back to REPORTER-FN, that's very reasonable), but I haven't
> looked under the covers yet.

REPORTER-FN, is just another name for CALLBACK.  Flymake doesn't need
these special variables because its API doesn't go through an old,
argless shim like eldoc-documentation-function, singular.  If it did, it
would need a special variable, which is not such a ghoulish thing
either.

>> 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"
>
> To clarify, you actually included two proposals (behavior with patch
> 1, and with both patch 1 and patch 2 applied). The first one is the
> one I really didn't like. The second one is better, API-wise, but will 
> conflict with the futures-based approach.

It's not a conflict at all.  When you do the futures-based approach
you're going to rewrite the docstring to augment the API.  You do the
same here: just add this to the docstring:

   "If you prefer, ignore CALBACK and return a `future-future' object from
    this function".

How is that different from:

   "If you prefer, instead of returning a (:ASYNC . FUNCTION) cons,
   return a `future-future' object from this function""

???

>> I'll leave that for later.  Specifically, eldoc--handle-multiline has to
>> do quite a bit more handling (to satisfy Andrey's expectations).
> I'm not so such it's independent. The complexity of implementing that
> would certainly vary.

It is quite independent.  Just look at the last patch I sent Stefan.
There's only one call to eldoc--handle-multiline (in fact we could ditch
it entirely).

> BTW, maybe eldoc-message should handle this after all? Since it's the
> last link in the chain. Or even do that in the default value of 
> eldoc-message-function (create a wrapper for minibuffer-message).

Doesn't work well becasue eldoc-message is called by both the produced
_and_ the backend, as I showed you earlier.  And it certainly doesn't
work with eldoc-documentation-compose.

>> 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, ...
>
> So the client will have to save and then compare the current buffer,
> the value of point and buffer-chars-modification-tick now?

Ah, _that_ validity.  No, no, never do that.  The client should have no
idea of it.  In the framework you either make the callback a noop, or
you set it aborted for the client to save some work.  Or both.

>>> 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.
>
> Futures are not particularly more complex to use.

Sure, but they are _more_ complex.  And you're mixing up two things:
futures _aren't_ the only way -- or even a particularly easy way -- to
signal to the clients that thety can give up their efforts.  All the
client needs to have is access to an object that's shared between it and
the framework.  That object _needn't_ be a first-class CLOS object or
struct.  It can be a function.

>>> 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.
>
> It can (barring any complex abstractions). It created the process,
> after all.

Not really, it asked a client to solve a problem, doesn't know how the
client if the client is doing by async process or cointoss.

>> 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.
>
> That would be none of ElDoc's business, though. But the implementation
> will get to be as complex as it needs.

_That's_ why an abort switch whose contract is "kill this immediately"
is a bad idea.  An abort switch whose contract is "just letting you know
I don't need this anymore" is better.  But again, in eldoc, not so
useful.  And again, nothing to do with futures here.

>> 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:
>
> The one thing I want to avoid doing is changing the callsig of every
> documentation function, and then changing them back when we switch to 
> futures.

So _don't_ change the "callsig".  If you implement futures you _are_
going to change the protocol anyway, i.e. write new stuff in the
docstring.

>>    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.
>
> Um, I'm pretty sure we guarantee a fair amount of stability.

That's not what Stefan said here:

   https://github.com/joaotavora/eglot/pull/459#issuecomment-633634034

And that's not what the Dmitry from 2016 wrote in xref.el

   +;; NOTE: The xref API is still experimental and can change in major,
   +;; backward-incompatible ways.  Everyone is encouraged to try it, and
   +;; report to us any problems or use cases we hadn't anticiated, by
   +;; sending an email to emacs-devel, or `M-x report-emacs-bug'.

That has been sitting there for almost three Emacs major versions (in
fact you added it after the initial 25 release).  All I'm asking is for
a little such flexibility with eldoc.

>>    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.
>
> Kill the arg in all built-in functions, as well as in external
> consumers?

Yes, if we discover there aren't so many.  Currently there are 5 users
in Emacs proper, 0 in GNU ELPA and quite sure 0 elsewhere in the world.
So 0 external consumers.  Do you really think if I push my patch there
will be hordes of eldoc-documentation-functions users out there using
this horrible futureless API that I'm proposing?  There won't, and I'm
happy to add a big disclaimer to the top of the eldoc.el file.

But if you really think we wouldn't be able to bring ourselves to kill
the arg, then just reintroduce eldoc-documentation-callback, and kill
_that_ later on.  Or just don't kill the arg, period.

It just looks like you're holding this problem hostage to introducing
some kind of rushed futures solution.  I don't agree with either of
these things.  I think this particular problem shouldn't be held hostage
to rearchitecting async in Emacs, laudable and useful a goal as that
might be.  And I think a futures library should be well thought out: I'd
like to discuss this in emacs-devel, at least get some opinions on
Christopher Wellon's solution, which seems very elegant.

In the meantime, I have a working patch that follows current coding
practice, solves this problem and in no way prevents or interferes with
future work.

Anyway, see the patch I sent to Stefan.

João