Re: Eglot "inlay hints" landed

[João Távora]

On Thu, Feb 23, 2023 at 2:49 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Cc: dalal.chinmay.0101@gmail.com,  emacs-devel@gnu.org,
> >   dimitri@belopopsky.com,  luangruo@yahoo.com
> > Date: Thu, 23 Feb 2023 12:57:26 +0000
> >
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> > > We don't need to make the overlays until the buffer is shown in some
> > > window, right?
> >
> > Yes, but two buffers A and B might already be showing in some window.
> > If you do the change in buffer A and it affects B, then in the current
> > version, the parts of A being show in windows will be updated, but B the
> > parts of B being shown in some other windows will not.
>
> If there's a change in A that affects B, jit-lock will call
> fontification-functions in both A and B, each one when it's about to
> display the corresponding window.

Sure, but you're in charge of coding up the "affection" by asking
the LSP server.  In other words, only the LSP server knows that the
change in A affects B. You must assume that it does and ask it "Hey LSP
server, given that I've just changed document A, in your document B from
42 to 420 is are there any new or different inlay hints you'd like to
give me?" jit-lock cannot foresee that upfront, it will only act on B's
display if B's buffer is changed.

Regardless of that separate issue, I started experimenting with
jit-lock-register.  It's promising, but has problems. Here's a
small experiment.  Use this foo.cpp file somewhere and have say,
the clangd server handy.

void foo(int bar){}

int main() {
  foo(42);
  .... repeats about ~250 times ....
  foo(42);
}

If you start eglot and eglot-inlay-hints-mode in this buffer,
you should see

void foo(int bar){}

int main() {
  foo(bar: 42);  // 'bar: ' is the untangible hint overlay
  foo(bar: 42);
  foo(bar: 42);
  ...
  < end of window >

I also traced the current eglot--update-hints-1 file which is a
binary function of two buffer positions, just like the jit
function.

This is the function that contacts the LSP server via JSONRPC
and some time in the future, after it returns, the 0-length
overlays will be created.

Here's how it is called after enabling eglot and scrolling 4
screenfuls forward (C-v) and 4 back (M-v).

======================================================================
1 -> (eglot--update-hints-1 1 476)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 443 927)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 894 1378)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1345 1829)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1796 2280)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1345 1829)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 894 1378)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 443 927)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1 476)
1 <- eglot--update-hints-1: nil

As you can see, it only requests inlay hints for the regions actually
displayed in the window.  It re-requests some stuff when going
back, since it has no memory of what it already requested or if
things were invalidated.

Now if I put eglot--update-hints-1 in jit-lock-function and
disable my window-scroll-functions, then do the same:

======================================================================
1 -> (eglot--update-hints-1 1 1501)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1501 2678)
1 <- eglot--update-hints-1: nil

As you can see, it did much larger, heavier requests upfront, even before
it knew I was going to scroll forward.  But in general it worked and you
can argue that doing only two requests for larger chunks of inlay hints
is better than more requests for smaller chunks.

Now, if I change the line void foo(int bar){} to void foo(int baz){}
the point is to get updated hints in the window:

int main() {
  foo(baz: 42);  // 'bar: ' is the untangible hint overlay
  foo(baz: 42);
  foo(baz: 42);
  ...
  < end of window >

The jit-lock implementation will do this:

======================================================================
1 -> (eglot--update-hints-1 1 19)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 1 20)
1 <- eglot--update-hints-1: nil
======================================================================
1 -> (eglot--update-hints-1 20 1520)
1 <- eglot--update-hints-1: nil

The first two calls are useless and so is most of the third one.
For the third one you can argue, as above, that it's a good
thing to predict that the user is going to scroll down.  But
the first two, which also caused two LSP requests, are definitely
useless.

OT1H, I think I can make this work, by finding some means to
coalesce those three calls into one.  Suggestions welcome.
Even after that complexity, it seems it will be a much neater
implementation.

OTOH it sort of negates my original intention of requesting
only as many inlay hints as strictly necessary.  THough that
might have been premature optimization anyway.

> > PS: On an unrelated note, I pushed this to emacs-29.  If you wish me to
> > revert the inlay hints implementation and do all this work in master,
> > it's fine by me.
>
> emacs-29 is fine, thanks.

OK!

João