Subject: bug#41531: 27.0.91; Better handle asynchronous eldoc backends

From: Dmitry Gutov <[email-masked]>
Date: Tue, 26 May 2020 22:14:31 +0300

On 26.05.2020 19:03, João Távora wrote:

>>> 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""
> 
> ???

We're not going to do either. The :async option is only for those tho 
need async _right this second_. It would be fully replaced by the 
"futures" option. Or we can implement it right now.

And hopefully we won't have to maintain the eldoc-specific futures 
"forever" either.

>> 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.

We could split it apart, I guess. Or make two versions.

>>> 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.

So the abort thing. In pre-command-hook?

>>>> 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.

It's good to have a well-documented contract. Functions do _everything_. 
They can't be optimal for everything.

And this way we don't add extra arguments, so whoever doesn't need 
async, doesn't need to change a thing.

>>>> 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.

Seems like we're miscommunicating.

>>> 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.

Here as well.

>>> 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.

See above about not having to change anything.

>>>     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.

I wrote that for xref.el because that was the state of affairs, and xref 
was relatively new. Especially compared to eldoc.

But we're probably miscommunicating here as well.

>>>     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.

OK, I see your point: eldoc-documentation-functions is new. And 
apparently you don't intend to add this feature to the variable without "s".

> 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.

Who's holding what hostage? I showed a smoother approach, you didn't 
like it. No big surprise about that.

> 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.

We should certainly take a look at it. But the built-in futures don't 
have to provide all the options. Just be functionally compatible. 
Christopher could pick up from that.