bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 05.07.2020 02:07, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>
> > [that is the] "transition" I meant here.
> >
> > If you have other questions, please ask.

I meant questions about futures as pertaining to eldoc, of course. I 
have other uses in mind, but redoing existing features is a less urgent 
endeavor. E.g., Flymake is stable, and I don't have any particular bugs 
in mind that need solving. So I'd be fine with keeping it as is. Unless 
someone wanted to take advantage of extra features that futures provide.

> If it wasn't clear, my suggestion is that you send your earlier
> dimtry-futures.el (as I am temporarily dubbing it, for clarity) to
> emacs-devel, along with a rationale for why it is needed and where it
> can be useful, not only in eldoc.el but in other places in Emacs that
> use callbacks like like url.el, flymake.el, jsonrpc.rl, timers,
> completion, processes, etc.  You may want to include a short comparison
> to Christopher Wellon's aio.el, if you have studied it.

Yes, will do.

But note that when I argue for futures, it's for basically any 
implementation that has a container for such values, and a set of common 
functions for manipulating them. Christopher's version would serve that 
just fine (with copyright assignment signed). Which exact version to 
choose, however, can indeed be discussed separately.

> > I think I have described at length the very simple idea of what it is
> > supposed to improve: provide a standard primitive and a library of
> > composition/manipulation functions that would make these operations
> > simpler. Which can be used in Emacs core, and in clients of the
> > various facilities and expose future-based interfaces.
>
> Maybe I missed something, but I don't consider our discussion an
> at-length discussion.

It also wasn't a discussion really. Too one-sided.

> > You seem to be in a hurry to get this in for some reason, but I'm fine
> > with waiting a little more, if we get a better result.
>
> I want to fix longstanding problems in Eglot.  I have limited resources,
> mainly time.  It is you who seem intent on not letting this go in, as if
> you won't be able to prove that "better result" after it does.  This is
> absurd: if you do show it to be better, then we should migrate eldoc.el
> etc to futures.  ...as I've said a trillion times already at this point.

Please don't make it sound as if I'm the only one here having a strong 
opinion about proposed code. You disregarded the simple solution in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531#8, and then went on 
with your own vision of of "simple" function based async, full on with 
extra features and additional incompatible API changes.

> > Please recall how you rejected my proposal along the same lines.
>
> I don't remember your proposal fixing anything in eldoc.el, you merely
> suggested I experimented with a technique.  Which I actually did, and I
> didn't see any advantage in it.

It certainly did away with clients having to call eldoc-message. And if 
shorter code and better backward compatibility are not advantages, we 
might disagree hard on the meaning of the word.

> > But the problem you seem to be urgent to fix (having eldoc-message
> > called from client code) is not so terrible that we can't wait until
> > the future-related discussion at least has had a chance to happen.
>
> I've explained repeatedly: this is holding up multiple things in Eglot.
> I want Eglot to serve diagnostic messages, and various kinds of
> automatically gathered information about the "things at point" all
> through eldoc.el.  Then move on to better stuff, of which there is a lot
> in LSP, as you well know.  Also, this fixes SLY and SLIME too, both
> hacking their way through eldoc.el.  Possibly the same for CIDER and
> other such extensions.  This also opens up eldoc.el in non-async
> situations as well, such as elisp-mode.el.  Just read the patch.

Aside: given that this discussion has user interface in mind, it needs 
more consideration of actual user experiences we'd want to allow. Ones 
not provided by eldoc itself as well. For instance, I think we sooner or 
later should have a callsig floating popup (in the vein of MS's editors) 
as an alternative to showing the signature in the echo area only.

> But very well then, let's
> have those non-futures objections in this bug report, the sooner the
> better.

To be sure, they will also contain the "how we could do this better" 
kind of arguments.

Let's start with the basics:

The new API is incompatible with the previous one, it requires changing 
a lot of functions (though admittedly in a minor way). Which is easy to 
miss, as evidenced by describe-char-eldoc which still doesn't accept any 
arguments. Whereas we could limit ourselves to the change which would 
only make the clients use the different hook (or keep using the old one, 
for compatibility with older Flymake/Emacs versions).

The choice to require a return of a non-nil value if the callback is 
going to be used is also kinda odd (+1 control flow to test). What's the 
problem with using the callback synchronously if the doc is available 
right away?

The decision to double down on having different 
eldoc-documentation-strategy values seems odd to me in several respects.

First of all, if I understand the main motivation behind it, it's to 
delegate the implementation of asynchronous primitives to Eldoc. We 
don't want to bother with that in Eglot, etc. But you mentioned "a type 
of docstring" to be returned in the discussion (and the callbacks have 
the plist argument as a future proofing). If the type is a buffer, its 
contents might as well be created from several async calls, which would 
require the same async primitives (though rather in general purpose 
form) available on the client anyway. Though it doesn't seem to be 
necessary for LSP in common operations, it's unlikely to be the only 
such protocol we'd ever want to support.

The strategies themselves:

- eldoc-documentation-enthusiast: What's the difference compared to the 
'default' one? Sacrificing CPU load for lower latency? It's an odd 
choice to force the user to make. The only people who can make an ideal 
decision regarding this are probably the authors of 
eldoc-documentation-functions, but it wouldn't help if 
eldoc-documentation-functions has functions coming from different 
authors, facilities, etc.

- eldoc-documentation-compose: Okay, this is kinda interesting (though 
not essential), and the implementation is not working as well as I'd 
hoped it would. It makes the echo area blink up and down as I move the 
cursor around. There's no expected truncation of individual docstrings 
when they don't fit in the window's width. And I don't understand what 
you expect it to do if several docstrings are multiline, and as an 
aggregate they don't fit the target height. I think the only reasonably 
predictable behavior would be to truncate each of them to one line, always.

- eldoc-documentation-compose-eagerly: Ooh, now I think I see why 
documentation functions have to return these non-nil, non-string values. 
Is it that this particular strategy wouldn't have to wait too long for 
documentation functions that never intended to call back? Returning 
futures instead (when async is needed) would provide this kind of 
guarantee without the seeming duplication of signals.

On a related note, I believe some facilities might want to use only 
certain "kinds" of documentation functions (as indicated by the plist 
arguments). If the plist is only provided together with the response, 
there is no way to avoid unnecessary computations (e.g. making HTTP 
requests that return some other kinds of values). If the plist was 
returned together with a future value, however, it would be easy to do, 
as long as we document that the futures should start the computation 
until a callback is provided (if possible, at least).

And in a different high-level argument: you stated that you intend to 
have Eglot use a non-default value of eldoc-documentation-strategy. 
Which would basically have you use Eldoc as a library, controlling both 
the list of documentation functions and how they are composed.

Whereas having the eldoc-documentation-strategy defcustom at all makes 
it seem like the user's choice, and that all Eldoc clients must be able 
to perform well under any strategy chosen by the user. We might want to 
think twice before introducing such responsibility.

Next, eldoc-print-current-symbol-info is a mess, very hard to follow. I 
am frankly offended that you argued that both ad-hoc futures I showed, 
or any potential "proper" ones would make backtraces hard to read and 
debug, and then introduced this kind of control flow with callbacks 
calling dynamically generated callbacks, retrieved from a variable 
dynamically set in a caller function up the stack (to be clear: I think 
having eldoc--make-callback variable at all is a bad idea). This should 
very well be possible to untangle in the future, but I'd rather not have 
code like this in Emacs if we can help it.

Further, having all strategies basically delegate to hardcoded options 
inside eldoc-print-current-symbol-info seems to confirm that the set of 
available strategies is a closed one. Which is not what I would expect 
from a variable called eldoc-documentation-strategy.

These are my objections, for now. I'll have to study eldoc--handle-docs 
a bit later, but any problems it has are probably orthogonal to the rest 
of the list.