bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

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.