-
Notifications
You must be signed in to change notification settings - Fork 401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add optional support for class files with file scheme #2322
Comments
I just heard about this issue from someone and wanted to chime in. Clojure-lsp allows for navigating to decompiled class files without any fancy language extensions. When handling a call to Either way, decompiling classes to a temp directory would be a great thing to support in jdtls in my opinion, as it would remove the need for language extensions and special implementation in clients. |
We don't use The We call this in quite a few places. The good news is that although, we return this URI to clients, we also provide the methods to handle its content. Since each client must write some code to deal with this, let me provide an example from vscode-java.
Altogether, the interaction looks like this :
If you want to use your own dissassembler on the classfile content, we support this. We have an extension point for content providers : We use it to contribute a basic dissasembler : You'd need to create a separate bundle that gets contributed via. https://github.com/eclipse/eclipse.jdt.ls/wiki/Contribute-an-extension-bundle#use-the-jdtls-extension but would be possible. |
Thanks for the response, that paints a pretty clear picture of what is going on now.
I understand why Is that something that might be possible? |
If we ended up creating a separate file for every Is there some uri scheme, we could return from 'textDocument/definition` that most clients will support ? Doesn't look like it. We use the query string portion (the part of after the
That's definitely custom enough that nothing is going to support this, which is why the client is forced to query the server once again, in order to get the proper content to display to the user. I've marked this as an enhancement since it could benefit other clients, but the work to support |
I could see it getting complicated. I've never had an underlying library change from underneath a project though. The versions in the filepaths to the jars themselves typically take care of that. Maybe that's more common in java projects than the clojure projects where I normally work.
I think that would be a fantastic way to handle this.
I wouldn't be so sure about this degrading performance in a meaningful way. My experience with both clojure-lsp and poking at the scala laguage server has been that this strategy is fast enough for me not to notice a delay between sending a textDocument/definition request, and my editor landing in the middle of a decompiled class file.
Nope :) The closest thing is a zipfile or jar uri, but this doesn't really make sense with class files. They have to decompiled, and the server needs to know how it is being decompiled to be able communicate the resulting To me this issue is a little deeper than finding a URI scheme to support this. The complexity of the JDT URI scheme speaks to that as well, with what looks to be a lot of information encoded in the URI. There are a number of LSP servers I have seen that need some way to tell a client "I know where the location of this type, definition, whatever is, but it isn't a plain file on your filesystem, some extra work needs to be done to retrieve it". Maybe that work is decompiling something, unzipping something, or downloading it from the internet. Either way, this is a gap in the LSP spec that different language servers and clients have filled in different ways. The current Location response isn't enough.
Thank you, I hope it gets some serious consideration. The work to support these JDT URIs is more than "just" implementing it. The original issue is about eglot, an Emacs lsp client that is set to be distributed with Emacs next major release. I do not foresee handling jdt URLs being added to that client, as the maintainers are very adamant about not implementing anything outside the LSP spec in Emacs core. There are simply too many small exceptions to the standard like this for a small team to implement and stay on top of. In the meantime, I may try to add support in a separate Emacs package for these JDT URIs. |
Does emacs-lsp/lsp-java#22 help ? Can any of that be re-used ? |
Not really. Eglot and lsp-mode are different lsp clients with different design philosophies. But that is beside the point. The code itself wouldn't be that hard to write. Support for this bespoke URL format for this one specific server probably won't make it into Eglot and Emacs core, it will end up in a separate package that someone has to write and maintain. So out of the box, this lsp server won't play very nicely with Emacs and will require the user to install extra packages for it to work correctly. That's not the end of the world. Maybe other lsp-clients require users to download extra packages to support servers like jdtls and users are fine with that. I don't know though. |
FWIW, @dannyfreeman , we could support this in emacs using something like: (defun jdt-file-name-handler (operation &rest args)
"Support Eclipse jdtls `jdt://' uri scheme."
(let* ((uri (car args))
(cache-dir "/tmp/.eglot")
(source-file
(expand-file-name
(file-name-concat
cache-dir
(save-match-data
(when (string-match "jdt://contents/\\(.*?\\)/\\(.*\\)\.class\\?" uri)
(format "%s.java" (replace-regexp-in-string "/" "." (match-string 2 uri) t t))))))))
(unless (file-readable-p source-file)
(let ((content (jsonrpc-request (eglot-current-server) :java/classFileContents (list :uri uri)))
(metadata-file (format "%s.%s.metadata"
(file-name-directory source-file)
(file-name-base source-file))))
(unless (file-directory-p cache-dir) (make-directory cache-dir t))
(with-temp-file source-file (insert content))
(with-temp-file metadata-file (insert uri))))
source-file))
(add-to-list 'file-name-handler-alist '("\\`jdt://" . jdt-file-name-handler)) Seeing how eglot finally is in core, I can open a bug report and see if we can include some similar code to this, either in eglot or somewhere a file-handler goes. In the meantime you can steal this for your own config :-) |
Hello,
I use emacs with eglot. I noticed that sometimes, jdt.ls returns URIs on the form
jdt://
when trying to find a definition. Eglot does not handle these URIs.Is it possible to configure jdt.ls to use
file://
protocol instead? I noticed that other packages parsejdt://
and convert them to something useful (I see that emacs's lsp-java and nvim nvim-jdtls do that. I suspect vscode does as well).Excuse my naivity as I don't now the design and purpose of return
jdt://
uris (not sure what that protocol is meant for).The text was updated successfully, but these errors were encountered: