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

From: João Távora <[email-masked]>
Date: Sat, 4 Jul 2020 10:21:43 +0100

Thanks for the feedback, Eli.

I'll get to each point later on. But I'm surprised the NEWS entry I had
written got lost. In the git rebasing, maybe.

João

On Sat, Jul 4, 2020, 08:45 Eli Zaretskii <[email-masked]> wrote:

> > From: João Távora <[email-masked]>
> > Date: Tue, 30 Jun 2020 12:31:10 +0100
> > Cc: Stefan Monnier <[email-masked]>, [email-masked],
> >  Dmitry Gutov <[email-masked]>
> >
> > The work that started with this discussion is now mostly complete.  It
> > has been sitting in the scratch/eldoc-async branch of the Savannah repo
> > for a while, but I've been very busy and didn't have time to annouce it.
>
> Thanks.
>
> > Anyway I think my efforts are ready to push to master.  I'll do so soon
> > unless someone raises a serious problem about it.
>
> Not a serious problem, but some comments:
>
> > -(defun eldoc-message (&optional string)
> > +(make-obsolete
> > + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
>
> Isn't the version part of the obsolete message supposed to tell the
> version of Emacs?
>
> The change to the number of arguments of the functions in the
> eldoc-documentation-functions hook is backward-incompatible, isn't it?
> I see you've changed the relevant functions in our sources, but what
> about 3rd-party packages?
>
> Also, the doc string of this hook needs clarification regarding the
> arguments: it first says that CALLBACK is the only mandatory argument
> to the hook functions, but then, out of the blue, appear additional
> arguments DOCSTRING and a list of key-value pairs.  Confusing.
>
> The doc strings have some words in UK English spelling "(e.g.,
> "honour"), please fix that.  Also, please make sure comments all start
> with a capital letter, end with a period, and comprise full English
> sentences.
>
> The doc string of eldoc-documentation-compose doesn't say a word about
> the function's argument.
>
> In the doc string of eldoc-documentation-strategy, you use the phrase
> "queries the special hook for all functions that produce doc strings"
> to mean, AFAIU, that the specified functions in the hook-variable list
> are called.  IMO, this wording could be confusing; suggest to use this
> instead:
>
>   `eldoc-documentation-compose': calls all the functions in the hook,
>   and displays all of the resulting doc strings ...
>
> This doc string doesn't explain the use of the timer, it explains the
> reason for its existence.  It should also describe the use:
>
> > +(defvar eldoc--enthusiasm-curbing-timer nil
> > +  "Timer used by `eldoc-documentation-enthusiast' to avoid blinking.")
>
> Last, but not least: the "async" part of the branch's name hints on
> some advanced and extremely useful functionality that these changes
> are supposed to allow, but I see no mention of that in NEWS and in the
> manual bits.  What did I miss?
>