Re: master 08c80c45dde: Don't use file-truepath in Eglot (bug#70036)

["João Távora" <joaotavora@gmail.com>]

On Thu, Apr 18, 2024 at 7:02 AM Theodor Thornhill <theo@thornhill.no> wrote:

> > But clangd doesn't do this, and in general servers _can't_ do
> > this because LSP models a virtual file system.
>
> From what I can see in other servers this is recognized as a server
> problem, and not a client problem [1], [2], [3].

From what I can see it was _reported_ as a server problem.  Whether
that actually means it is _recognized_ as a server problem would
need a much larger consensus between the numerous servers.  There
are very many LSP servers, and new ones pop up every day. It
won't be long until Eglot supports 100 servers.

So even a newfound LSP directive stating this is indeed the server's
responsibility, it wouldn't really be satisfactory, because we're dealing
with a "de facto" spec.

Also the LSP maintainers tend to err on the side of bothering the clients
to do these things.  Sesee the "DidChangeWatchedFiles" notification discussion
in the spec.

They argue that it's easier to do in the client partially because
LSP clients are less numerous than LSP servers.

Myself, I can't think how this can be a server problem.  It makes no
sense to me for a client such as Emacs which has a perfectly sane and
clear view of what is a symlink or not (it tells you explicitly in
the echo eare) to mention the same object twice via its alias.

As I wrote, it may be that some servers have the ability to understand
symlinks because they are close to the file system.  But that's not
necessarily the case.  LSP servers can function in completely
different hosts and be informed of files just via DidOpen, didChange,
DidSave.

Whatever the outcome of that discussion I think we should have
no inclination to expose Eglot users to a new problem.  Also note
that the reports you link to are about looping.  My report is not
about looping at all, it's about incorrect responses.

Taking all this in consideration I have reverted the patch.
I've added a much simpler fix to your performance problems that
doesn't endanger the correct behaviour.

> > And for symlinks to large enough files, I'd be surprised if this
> > doesn't slow down the performance of the server because it has to
> > analyse what it is told is a completely new file.
> >
> > So this seems like a pretty big flaw to me after just minimal
> > surface scratching.  Please reinstate the previous code.
> >
>
> I can absolutely do this, but I'd rather we consider it a bit more
> before I do. On my system there is a real, tangible performance increase
> with this change, and the symlink one seems more like a server edge case
> issue.

Maybe, but bugs about edge cases aren't uncommon or less important IMO.


> Maybe we could check for file-symlink-p rather than runthe whole
> file-truepath?

Maybe, but let's do that after fixing the regression.  Let's follow up
in the bug report, which I've hopefully catched up on.

João