Subject: bug#58790: Eglot URI parsing bug when using clojure-lsp server

From: João Távora <[email-masked]>
Date: Thu, 27 Oct 2022 16:09:18 +0100

Danny Freeman <[email-masked]> writes:

> Danny: The patches would still be necessary. The patches simply make
> eglot Danny: correctly parse the "jar:" scheme URIs from lsp servers
> that use them.  Danny: It doesn't have much to do with how emacs is
> handling the file paths Danny: once it is parsed.

Indeed, I am in agreement with the last sentence.  And this means the
result of this parsing is arbitrary as long as it's a bijection.

>> In my mind, project.el should support adding jars as collections of
>> source files just like it supports adding directories as collections
>> of source files. Many years ago, Eclipse did this. You could add a jar
>> as a library or a file as a library: it's just a different
>> implementation detail.
>
> Danny: Maybe so, it's unclear in my mind how that would work with lsp
> Danny: servers.

Here's how I envision it working, from reading some of the source
involved, but with no testing:

1. In buffer B1, the user uses M-. (xref-find-definition).  Eglot
   requests :textDocument/definition from the LSP server, which returns
   a number of definitions.  Some of these look like

     jar:file:///home/user/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar!clojure/core.clj

   Call this value X.

2. Eglot converts the URI X into a some file designator Y, using
   eglot--uri-to-path.  Y may or may not be === X, as long as there is
   exactly one X for every Y.  Eglot makes xref-item objects from it
   using xref-make-match and xref-make-file-location.  The designator Y
   goes into the 'file' slot of the xref-file-location object.

   We'll see later if and how eglot--uri-to-path needs to be changed to
   do this conversion in a way that doesn't hurt all the other
   conversions it already does there.  One of the possibilities is just
   to leave X completely untouched and pass it onwards.

   The word "path" in eglot--uri-to-path should be understood as "file
   name" or "file designator" (depending on whether you are into Elisp
   or CL parlance).  It does not necessarily represent a filesystem
   path.  eglot--uri-to-path should probably be renamed, but that is a
   cosmetic tweak.

3. Using xref--show-location on one of these matches will eventually
   call xref-location-marker and invoke invoke find-file-noselect on Y.
   
   Here, an entry of file-name-handler-alist should know how to handle the
   format Y.  This is what Danny is working on as a separate package.

   If that entry exists, the process should land the user in some buffer
   B2 where buffer-file-name is set to Y.

4. B2 can be setup in a way so that project-current returns the same
   object which is returned in B.  If this is true,
   eglot--current-server discovers the same connection which also
   manages B1 and Eglot adds buffer B2 to the list of managed buffers in
   that connection.

   However, if eglot-extend-to-xref is non-nil, eglot--current-server
   should also discover the correct connection.  This is less ideal than
   making project-current know that the buffers of source code inside
   the jar are also in the same project, but it works.  I can explain
   the downsides, but it's not very relevant right now.

5. Upon findin the "file", Eglot transmits a :textDocument/didOpen
   notification to the server.  This requires eglot--path-to-uri, the
   reciprocal of eglot--uri-to-path to convert the path Y to URI X
   again.  Again, maybe this conversion is just #'identity.

   Eventually, the LSP server knows that the client is now working on a
   textDocument whose relationship to other opened documents in the
   workspace it understands (which may or may not be read-only).

6. xref-find-definition on any part of the B2 should now work similarly
   to this whole flow.

> Dmitry: Sounds like you want to advise project-vc-external-roots-function. Or
> Dmitry: change its whole value.

I don't understand that "vc" has to do with this.  The above
implementation should work with any project backend, not just VC.

To be clear, my suggestion was to add the ability to add a jar file to
project-external-roots.  Currently a root is a string representing a
directory, per its docstring.  But it could be generalized to a string
representing any container of files.

> Dmitry: Or create an Eglot-specific project backend.

I don't understand this suggestion either.  Normally Eglot is a client
of project information maintained by other project.el backends.  Very
commonly VC projects, but not always necessarily so.  That clashes with
the idea of making Eglot simultaneously a supplier of this information.

Please read the summary of the outlined above.  Maybe there's nothing to
be done in project.el if eglot-extend-to-xref is to be used.

Or maybe I'm misunderstanding the whole flow, or missing some detail, in
which case feel free to correct me.

> Danny: Without the files being extracted somewhere would they be able to
> Danny: perform any analysis on them?

Maybe Eglot just needs to be changed to _not_ strip the leading
"jar:file" and leave it completely unchanged.

So the server should be able to sort itself out, as long as you give
back the very same URI you got -- from the server itself -- to the
in-JAR source code.

João