bug#81514: 32.0.50; Eglot messages progress reporter shows errors

On Sat, Aug 1, 2026 at 7:59 AM João Távora <[email protected]> wrote:

> Stéphane Marks <[email protected]> writes:
>
> I've pushed my simpler patch
>
>    commit cf98d9d725f034b3dd89f6f92a1c8205e778f98c
>    Author: João Távora <[email protected]>
>    Date:   Sat Aug 1 10:54:10 2026 +0100
>
>      Eglot: do more orthodox use of progress reporters (bug#81514)
>
>      The progress reporters on recent Emacs has backward incompatible
>      changes, but our use of them wasn't too correct either.

but please read on.
>
> >  The previous reporter or the fixed one?  And before or
> >  after your changes?
> > It is good the system-taskbar progress bar displays but it is bad that
> > it pulses for a known 0-100 progress reporter
>
> What do you mean by "pulsing"?  It's been a while since I've cared about
> these OSX/Windows/GUI paraphernalia.  You seem to mean "pulses when it
> should be xxxing".  What is xxx?
>

Pulsing is the terminology used in progress-reporter:

(defvar progress-reporter-update-functions (list
#'progress-reporter-echo-area)
...
- A monotonically increasing integer for a pulsing reporter.

A pulsing reporter is one that has no bounds and knows only that an
activity is under way so it "pulses" using a spinner or whatever in the
echo area and system-taskbar will show a progress bar that simply cycles
rather than indicates percentage completion.

> and very bad that the stateful system-taskbar progress bars need to
> > disappear when percent gets to 100 (or times out or whatever) but
> > don't disappear, sitting at the last remaining reported percentage.
>
> Understood.
>
>  Anyway, can you try the simpler patch after my sig?  It should create
> >  the reporter with 0-100 and also call 'progress-reporter-done' at the
> >  appropriate time (and be a bit less intrusive).
> >
> > The only issue with the simpler patch is that if the LSP server sends
> "end" without first reporting 100, any stateful
> > progress reporter back ends like system-taskbar's are never cleared.
>
> If the server sends 'end' and no percentage my patch (which btw I've just
> pushed to emacs-31) throws a 100 in there and you should get a call to
> progress-reporter-done.  What else is missing there?
>

Good.

> Same if the LSP server dies.  Are those conditions we should account
> > for?
>
> The first condition you mention I've tried to address.  As to the
> "server death", I don't think either of our patches handles it.  What is
> an appropriate cleanup function for a progress reporter, just a
> 'progress-reporter-done' or something else?
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 0f0791d37ab..285041653d1 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -1491,6 +1491,11 @@ eglot--on-shutdown
>           (lambda (x) (eq server
>                           (get-text-property 0 'eglot--server (car x))))
>           flymake-list-only-diagnostics))
> +  ;; Cleanup progress reporters
> +  (maphash (lambda (_ r)
> +             (unless (eq (car r) 'eglot--mode-line-reporter )
> +               (progress-reporter-done r)))
> +           (eglot--progress-reporters server))
>    (cond ((eglot--shutdown-requested server)
>           t)
>          ((not (eglot--inhibit-autoreconnect server))
>

Yes, progress-reporter-done should do the trick.

> Did you want to address Microsoft's suggestion to ignore erroneous
> > progress values?  Clearly, they know that LSP servers misbehave and/or
> > are misimplemented.
>
> I'll protect the code if I find actual evidence of misbehavior.  Up to
> now, no such reports existed.  In fact this bug was bisected to your
> changes to the progress reporters ;-) But feel please conjecture a
> hypothetical scenario where we both can clearly tell there would be
> (serious?) misbehavior.
>
> > For the sake of documentation posterity, I'll include the official LSP
> > documentation reference here.
> >
> >
> https://github.com/microsoft/language-server-protocol/blob/b7f5132c95261c0898ae5124e7a91707abc48fcd/_specifications/specification-3-16.md?plain=1#L1455
> >
> >     The value range is [0, 100]. The value should be steadily
> > rising. Clients are free to ignore values that are not following this
> > rule.
>
> Why do you keep highlighting that clients are free to ignore this
> monotony.  Is it of particular relevance to this bug or proposed patch?
>

I've mentioned it twice if that counts as "keep highlighting" merely to see
if this is worth defending against and clearly you think not yet, so I
won't highlight this again.

>  Finally, can you spare a sentence or two about what the new progress
> >  reporter enables?  Alternatively, point me a relevant NEWS entry?
> >
> > See
> >
> https://github.com/emacs-mirror/emacs/blob/99a9307235ca7e833744adb19e58473b86fc76e8/etc/NEWS#L4089
>
> Thanks.
>
> João
>