-
Notifications
You must be signed in to change notification settings - Fork 200
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
Find definition won't work for external dependencies in Clojure #661
Comments
Yeah, this isn't supported. Could it be? Maybe yes. But I'm unsure if it should be in the context of Eglot or simply Xref and Project, which are two packages that Eglot depends on. I lean towards Xref and Project. We can the maintainer of both those projects here, @dgutov, and allow him to give his opinion, or you can open a bug in Emacs' bug tracker, by sending mail to Once the Xref + jar opening is sorted (supposing it is doable) we can think about whether that file should be associated with the same project. Indeed, as you suspect, the situation isn't linear because the file could be also found by |
I don't think it's really Xref or Project's responsibility, but here are some suggestions.
Approach #2 could be conceivably added to xref.el itself. Approach #3 (which I personally like) doesn't have that option. Regarding how to track which dependencies belong where, I don't have much better to suggest than some ad-hoc options (perhaps track the list of files/buffers opened this way, associated with a project, or perhaps track lists of directories, if LSP servers report "library paths"). If you go the "virtual buffer" route (#3), it could simply have As you are both the provider and the consumer of the project information in this case, I'm not sure if following the project.el framework holds much value to you. E.g., you could set Here's what I've done in another tool (not LSP-related): for each open project, there is a corresponding connection buffer. That buffer has a buffer-local variable set to the "library path" for that project. Then I do a linear search across such buffers whenever the current file is seemingly not attached to any project directly. |
I said Xref becasue this is a general cross-referencing problem. It could conceivably exist without LSP, which is Eglot's main business, and be useful in other situations.
Translate LSP formats into Emacs formats is indeed Eglot's business. But where can I get hold of this "expected format"? If no expected format exists, we should make one. That's number 2 and it should live in Xref (or
This is the curious absurdist view of LSP as language-agnostic-but-not-really. These extensions belongs in
If the same problem is being solved inside and outside the scope of LSP, this again hints at the need of a shared library. But first I'd solve the file-finding problem. |
It explicitly encourages extensibility to allow for special cases. I'm not sure how "core" finding files inside archives is to its purpose, but if it's implemented sufficiently general, it could be added. If you go this route, do consider option #1 first. It should take ~ the same amount of code, but then you'd be able to address files inside archives anywhere, not just using Xref. I also wonder which of the approaches would work better with Tramp. Like, if the LS is on a remote host, and we add an addressing scheme for files inside archive files, would Tramp have to add special handling for files inside archives on remote hosts?
The view of current reality, shrug.
These handlers are per-server, not per-language (but good luck convincing
The implementation is trivial, the real difficulty is choosing which data to use, and where to get it from. |
That sounds fine. But you said there's a format that Eglot should translate to. Where is that format? Or should I invent it inside Emacs and distribute a package from it? Didn't you say there is code to find files inside jars?
My concrete reality as someone who actually uses LSP is using it without any such trash.
I don't use clojure, so I'm not going to be doing any convincing. Clearly doesn't belong in Eglot. You said there's a clojure server with some third-party fancy extensions, maybe clojure-mode maintainers or someone else wants to leverage that using Eglot as a library
I wasn't referring to the implementation, just pointing out that the problem seemingly reaches out of LSP. I don't fully understand the problem and its quirks yet, though. My naive understanding of the statement is "how to have a file somehow referenced by two projects, and what advantages and limitations does that bring?" |
Invent, yes. Create a file name handler and distribute it from somewhere. But first I would check why it's still not done yet: after all, archive files themselves already have file name handlers.
It certainly exists, since you can visit a
As long as there is a good extensibility story, I suppose interested parties could create an But it will require infrastructure, such as the use said url handlers in the eglot-specific location type's
It's the kind of problem that doesn't easily lend itself into being extracted into a library.
My solution is essentially a hack (just use the most recently used project that has this file in the "externals" set), with the upside of having most features seamlessly working, and the downside of sometimes not guessing the user's intent correctly (using the "wrong" project is more problematic for some commands than others, e.g. it's fine for "find definition" but not for "find references"). |
How to check that? If you have any suspicions, please share, else I think it's easier just to try it.
In this case, since there is no format and I'm supposed to invent one, I might as well just use the same format as the clojure LSP server is using here, if it's minimally useful. So no translation necessary. Else, if LSP comes up with an official format that's not gratutiously different, translation is Eglot's job. |
Try the code, search its history,
IIUC there are some considerations to use for choosing particular addressing scheme. Michael could advise on that as well.
Have you compared the formats used by clojure-lsp and jdt.ls? |
No, have you? This format looks fine. If someone else (jdt or hopefully LSP itslef) shows another format then Eglot will do some minimal translation. Same thing if Michael or the facts of code show that Clojurels format isn't suitable. |
I'd have to install both to really do that. This handler looks pretty involved, though: https://github.com/emacs-lsp/lsp-java/blob/master/lsp-java.el#L837-L877 |
yes, pretty involved. Better stay away from it |
OK, so to sum up, I think this
@Andre0991 since you seem to be interested in Lisp languages, you can try to develop this. By scratching your itch, you'll help Eglot, Emacs and many others. |
Sounds like one hell of a first issue: lots of investigation and organizational stuff, not so much tech. |
Yes, it's a bit ambitious as a first issue. One should definitely start just experimenting briefly with |
Olá. First, I need to amend my issue description. I found out that there is this For more details, please see https://clojure-lsp.github.io/clojure-lsp/settings/. On
I see that both Calva and lsp-mode explicitely set this to If I don't set it to jar,
Also, I have a question, though I'm not sure if it's part of the scope of this issue. I don't know if the points above impact anything you have discussed so far. Finally: I don't know much |
I decided to take a look at fixing this problem today and have come up with the following solution, which setups a handler for the The buffer gets associated with the current project. When used in conjunction with cider, I can evaluate expressions in the new buffer, modify it, and save it under the current project structure (does NOT update it in the jar itself, but that's fine). It seems like clojure-lsp/eglot continues to provide analysis on the unziped file in some cases. It currently expects This has been really useful for me so far. I'm going to use it at work for a couple days and see how I like it, maybe make some tweaks to how the buffer is configured. I'm posting this solution here for other people to use. Would this code be something that the maintainers like @joaotavora might like to have submitted to this project? If not, maybe it would be suited elsewhere, like clojure-mode, or an independent package. Asking here first since this is where I found the bug ticket. Really interested to hear your thoughts on that. (defvar clojure-jar-match
(rx (group "/" (* not-newline) ".jar") ;; match group 1, the jar file location
"::"
(group (* not-newline) "." (+ alphanumeric)) ;; match group 2, the file within the archive
line-end)
"A regex for matching jar files to be opened by eglot and clojure-lsp")
(defvar clojure-jar-file->buffer
(make-hash-table :test 'equal))
(defun existing-clojure-jar-buffer (file)
(when-let ((existing-buffer (gethash file clojure-jar-file->buffer)))
(if (buffer-live-p existing-buffer)
existing-buffer
(remhash file clojure-jar-file->buffer)
nil)))
(defvar project-source-root "src/")
(defun try-place-buffer-file-in-project-src (source-file fallback)
"Set `buffer-file-name' to SOURCE-FILE under a project's `project-source-root' directory.
If that can't be done then the buffer-file-name is set to FALLBACK."
(let* ((root (project-root (project-current)))
(project-src-dir (expand-file-name project-source-root root)))
(if (file-exists-p project-src-dir)
(setq buffer-file-name (expand-file-name source-file project-src-dir))
(setq buffer-file-name fallback))))
;; Implement save file (save into project root src folder if it exists)
(defun clojure-jar-handler (op &rest args)
"A `file-name-handler-alist' handler for opening clojure source files located in jars.
For this to work, the `:dependency-scheme' clojure-lsp setting should be set to `:jar',
although this could also work when set to `:zip'."
(cond
((eq op 'get-file-buffer)
(let* ((file (car args))
(match (string-match clojure-jar-match file))
(jar (substring file (match-beginning 1) (match-end 1)))
(source-file (substring file (match-beginning 2))))
(if-let ((existing-buffer (existing-clojure-jar-buffer file)))
existing-buffer
(message "Unzipping %s to show %s" jar source-file)
(with-current-buffer (create-file-buffer file)
(puthash file (current-buffer) clojure-jar-file->buffer)
;; unzip -p path/to/bidi-2.1.3.jar bidi/bidi.cljc
(call-process "unzip" nil (current-buffer) nil "-p" jar source-file)
(try-place-buffer-file-in-project-src source-file file)
(setq buffer-read-only nil)
(setq buffer-offer-save nil)
(set-auto-mode)
(set-buffer-modified-p nil)
(goto-char (point-min))
(current-buffer)))))
(t (let ((inhibit-file-name-handlers (cons 'clojure-jar-handler
(and (eq inhibit-file-name-operation op)
inhibit-file-name-handlers)))
(inhibit-file-name-operation op))
(apply op args)))))
(defun clojure-jar-init ()
(add-to-list 'file-name-handler-alist (cons clojure-jar-match #'clojure-jar-handler))
(remove-hook 'clojure-mode-hook #'clojure-jar-init))
(add-hook 'clojure-mode #'clojure-jar-init) I am also not very proficient with emacs-lisp, and would love some feedback if anyone has the time. Thank you! |
Well, then, great effort. I do spot a tiny bit of non-idiomatic Elisp code here and there, but nothing really serious. I guess you know Clojure, Elisp isn't so off. I don't understand the entry point. How does one find a file in a |
Thank you for the quick reply!
Not sure I understand this question. There isn't really any time I've clojure-lsp reports the file location like this The buffer itself I try to associate with a new file in the current project, and if saved would be found under Either way, I am happy to see what the clojure-mode.el maintainers think, and will look into publishing a new package if they don't want this. |
You ended up answering it. I think that, with your code loaded, if you give So it looks pretty good. Though on second thought, |
Jars are just zip files and can be used to package arbitrary source files. So java, kotlin, scala, clojure, etc. I believe this might also work for .java files as is, but would have to find a way to test that. I will look into other emacs utilities for handling zip files. shelling out to Edit:
|
@dannyfreeman Emacs already knows how to handle jars and their files. Try opening a jar file in your .m2 directory. When I do that, I end up in a file that looks like |
Or maybe emacs facilities for handling jars could just learn to grok the double |
To follow up on this … it appears that arc-mode.el provides archive-mode, which is a dired style view of an archive. Because jar files follow the zip file format, archive-find-type classifies the jar as a zip. This gets translated in the mode line into “Zip-Archive”. From the dired view, you can navigate to a file. So what I wrote above is slightly wrong... you can't just open a file within a jar. You have to open the jar first and then find a file within it. That’s as far as I’ve gotten. I’m not sure yet how archive-mode unzips the file, or whether that’s something tools like eglot could hook into.
@joaotavora that sounds ideal, although we're way over my head already. |
I was about to post a response, but you summed it up @mainej, we still can't navigate straight to files within jars with just one command like I can go through archive mode by replacing the Edit: I'm going to try to find some time this weekend to search through cider's source and see what they are doing. Perhaps they will have some hints for me. |
@dannyfreeman that's exactly what cider does in So I'd say all signs are pointing to using |
A note on clojure-lsp... As you've discussed there's a setting called There are two valid values Note that you might find documentation that suggests The URIs for
An example is The URIs for
Here uri-to-jar is itself a uri, usually with the format You may be dismayed to see a URI in a URI. I agree, but direct your complaints to Java who defined this style. 🙄 And actually, since this is Java's preferred way of referencing its own jar files, ideally Emacs would understand this style too. @dannyfreeman you're setting |
@mainej This is great information, thank you for sharing it. There is no reason this file-name-handler-alist strategy couldn't support both zipfile and jar strategies. I'll turn on the real I think the path forward for the time being is to get this working and published somewhere as an independent package. That will allow me to work through all the kinks, see if other language servers can benefit from it. From there we can look to see if emacs might be a better home for this code. I will post here, and in clojure lsp slack channel when I get a repo set up. |
The nested jar URIs that are provided when using the Relevant code: Line 1503 in e501275
When eglot gets a filepath like what the jar option provides Right now I have eglot locally parsing the uri twice, with the line linked above replaced with (retval (eglot--parse-uri uri)) and it's definition: (defun eglot--parse-uri (uri)
(url-unhex-string
(url-filename
(let ((url (url-generic-parse-url uri)))
(if (string= "jar" (url-type url))
;; jar: URIs can contain a nested URI, so we need to parse twice.
;; For example, `jar:file:///home/user/some.jar!/path/in/jar'
(url-generic-parse-url (url-filename url))
url))))) Once that is wired up I can make emacs work with the (defvar clojure-jar-match
(rx (group "/" (* not-newline) ".jar") ;; match group 1, the jar file location
(or "::" "!") "/" ;; this part is what's new
(group (* not-newline) "." (+ alphanumeric)) ;; match group 2, the file within the archive
line-end)) Parsing the URI twice might be something that should be placed in eglot. Would eglot maintainers be interested in a patch for double parsing the URIs with a Baring that, I think I could still make this work outside of eglot by adding some |
Either patch will not be enough. This only makes it so that extra code like what I've got above capable of dealing with the jar style paths. I'll start to prepare the patches and try to have them sent today. Congrats on getting this code merged into emacs! |
But hadn't we established earlier that there's already some code in Emacs itself which does more or less the same? Regardless of your answer to the previous question, and imagining one of those patches in in place in |
Fell free to answer the questions/continue discussion in the bug report where you propose the patches. |
No, there is still nothing in emacs to handle this exact situation. There is code in emacs, via I've started creating a package for this here: https://git.sr.ht/~dannyfreeman/jarchive
Assuming eglot has one of the patches in place, the user needs to do the following
|
@dannyfreeman Could you provide a link to the thread with patches, please? |
Yeah, when I get them sent out I will link them here. It's taken me a little longer than I had hoped, with life getting in the way. I'll try to do it later today though. Edit: here is the two patches zipped up for emacs. I still need to send it to the mailing list. I'm taking the time to read through the contribute guidelines, trying to get it right the first time. |
@dannyfreeman fyi, I tried your patch to eglot + jarchive and it works for me to |
That is because I'm setting the directory of the opened 'source in the jar' to the jar's directory. eglot starts a new lsp server there, and clojure-lsp doesn't really know what to do probably because it's not in a real project directory and the actual file does not exist. I don't know of a great way to solve this right now. One way is to trick clojure-lsp by placing the extracted file in the source path of the current project. I'm not very crazy about that approach though because it makes too many assumptions. Especially that source file will be managed in a project under the A work around that requires no code changes at all is using I'm not sure what will be a good solution. Going to have to give it some thought. |
@abcdw here is the thread started on gnu emacs bug list |
@ericdallo perhaps you can help @dannyfreeman with this. I know that lsp-mode is able to do the two-level navigation |
Are people here aware of eglot-extend-to-xref or eglot-xref-extend (can't
remember the name right now...)?
That's how "level 2" xref-find-definition is supposed to work in Eglot.
Whether just setting the flag to t works with the current patches is
another matter...
…On Wed, Oct 26, 2022, 17:51 Jacob Maine ***@***.***> wrote:
if I then try to xref-find-definitions for some fn defined in the 'source
in the jar', I don't find definitions
@ericdallo <https://github.com/ericdallo> perhaps you can help
@dannyfreeman <https://github.com/dannyfreeman> with this. I know that
lsp-mode is able to do the two-level navigation project file -> dep file
1-> dep file 2 so perhaps you have some insight about how the temp
buffers need to be managed to support this.
—
Reply to this email directly, view it on GitHub
<#661 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC6PQ7ZNCFUQZV7AFDEKO3WFFOS5ANCNFSM42L7SIJQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Setting |
https://git.sr.ht/~dannyfreeman/jarchive/commit/a5ab89db3f7bf248f3f0ff0ad97767f2c9fe01e5 This change makes the file managed by eglot by simply not setting the It's still not possible to navigate around again with lsp-mode seems to be doing something special with clojure by creating a temp directory, writing the extracted files to that temp directory, and adding those to the lsp workspace so that clojure-lsp can find them. Maybe I need to be doing something along those lines, I don't know. Does eglot (or maybe project.el) have a mechanism for adding an external directory to the current project? I think that would be what I need to do this automatically. Right now I get around this with a function I wrote called |
I think project.el does. But then again this seems like a completely different approach and seems to negate the need for the patches you sent to Eglot. 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. Please respond in Emacs's bug tracker. |
@dannyfreeman Thank you! BTW, I packaged jarchive for guix: |
Sounds like you want to advise Or create an Eglot-specific project backend. |
I'm going to answer this in Emacs's bug tracker https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-10/msg02118.html. It's akward and unpractical to keep commenting in both places. |
FWIW, I don't think anybody pinged me there, and I currently have 1939 unread messages in my Debbugs folder. |
I was just registering the reason for closing the issue. Don't worry, I'll CC you the next email. |
sorry, what's the status of this? I see this issue is closed as completed, but the mailing-list thread suggests that the feature is not yet supported? I'm considering migrating from lsp-mode to Eglot once this feature is implemented, but the status is a bit unclear. |
@andreyorst this issue is closed because discussion moved over to the mailing list. Work on it is still ongoing. I can post back here when its wrapped up for people who don't want to follow the mailing list discussion. |
Posting a final resolution here. Installing my jarchive package resolves this issue: https://git.sr.ht/~dannyfreeman/jarchive @abcdw you may want to update your guix recipe with a newer commit. Thank you for sharing that here. |
Hi @dannyfreeman. I see that there are no new messages in the Emacs bug tracker 1 – will Emacs get a patch too or is |
There is still discussion on that ticket in the 2022-11 archives. I think this is the latest message at the time of me writing this. The way the mailing list archives are displayed can be confusing. The way I see it, jarchive is the solution for now, and it is on ELPA: https://elpa.gnu.org/packages/jarchive.html which is the most available place for it to be next to emacs core IMO. Maybe down the line jarchive or something like it will make its way into Emacs. I think that would be nice and I'll keep discussing it. Until then ELPA is a nice proving ground for this functionality. |
Oh, indeed, I missed the newest messages. Thank you so much for tackling this – I just installed |
FWIW, I just tried jarchive, and it doesn't fix the issue for me. I get an error like this:
|
Hi. I'm experimenting with using
eglot
with Clojure.I've had no issues so far except that
xref-find-definitions
won't work for external dependencies.When I use it on some symbol that refers to an external lib, Emacs asks me if I want to create the directory:
The expected behaviour would be to open a buffer with the contents of the file in the jar (and manage it in the same project? Not sure).
Here's the an eglot log sample for this issue:
Steps to reproduce
Download the
clojure
binaryDownload the native clojure-lsp binary in https://github.com/clojure-lsp/clojure-lsp/releases
Configure
eglot
to use itclj-new
and install its dependencies.Emacs
, go toclj-new/src/clj/generate/def.clj
and usexref-find-definitions
on some external lib likeclojure.java.io
(line 5 in this file).Note: I'm using Doom Emacs. I think this does not affect anything in regards to this behaviour. Unfortunately, I don't have a clean Emacs for confirming this easily right now. If you suspect this is Doom specific, I can setup a barebones Emacs in a few days.
The text was updated successfully, but these errors were encountered: