-
Notifications
You must be signed in to change notification settings - Fork 3
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
Instruct clojure-lsp about how coc.nvim conveys URLs #8
Comments
Sure. I know you are already working on a fix on server side so maybe we don't need any changes on coc-clojure, but I'd like to bring a question: Why vim uses zipfile and not jar as default? I don't see how that change could affect user experience, and having all clients using the same scheme would help a lot avoiding bugs and code maintainability, WDYT @mainej @snoe @NoahTheDuke? Of course we would need to test and make sure jar scheme works seamless on coc/nvim client |
That's right. I think this is fixed on the server side, so we don't need a coc-clojure change. I'd still appreciate it if coc-clojure set
💯 I agree—supporting the zipfile scheme adds complexity to what is already a very complex part of clojure-lsp and it would be nice if we could remove it. I'm not sure we can, but we should try, IMO. (I accidentally forced coc.nvim to use the jar scheme today, and I think it broke definition navigation. Lots of other things were breaking navigation too, including clojure-lsp changes, so I'm not sure the jar scheme was to blame.) |
Vim uses I also discovered this 2019 coc.nvim issue that relates to clojure-lsp (opened by fellow clojurian @snoe) which relates to the discussion y'all had about url escaping. |
@NoahTheDuke thanks for doing some archeology. @snoe do you have any additional historical context? I just did an experiment. I'm using a build of So while it's true that nvim handles clojure-lsp's default scheme of |
@mainej Using zipfile allows changing the jar on disk. To me, this is an advantage, others might not like it. |
I suppose this should be pretty specific, not sure it's worth considering the tradeoff for only that feature |
omg this explains times I've broken things and fixed it by clearing my |
I have to revise what I said earlier... for me,
@snoe, suppose I can see both sides to this. If those questions were being posed to me, I might answer "Yes... and... at the time, it was easier to teach clojure-lsp to use the zipfile scheme." @NoahTheDuke I apologize—we've veered from the original topic, and I'm not sure coc-clojure has any role to play in this. If you'd like we can close this issue and continue the discussion elsewhere, perhaps in clojure-lsp. |
I'd be nice if one could create a issue on nvim side to track/understand that better |
@mainej |
@snoe I think you're talking about "clojure/dependencyContents". I don't know that part of the code well. I think it's true that at some point it was put in place for clients to be able to request the file content of a jar scheme file. The client is told "here's a jar URI" and it then turns around and asks the server for the content. The server unzips the jar, finds the file, and returns the file content. Because the server provided the content, the editor has no way to save edits. Is all of that right? @ericdallo is it also true that we're moving away from that approach, for example here, and instead asking the editors to take that responsibility? That is, we're moving toward a world where we tell clients "here's the URI for a jar file" and we expect them to know what to do... figure out where the jar is on disk, unzip it, show a buffer with one of the files in that archive, and if they so desire, support edits of that file by writing them back into the archive? If it is true, and I'm not sure it is, then I personally support it! In my opinion those responsibilities—finding archive files, opening them, and editing them—are all editor responsibilities. I understand why clojure-lsp might have put in place workarounds for editors that couldn't do all of that. But to me, the long term solution is to improve the clients, not provide server-side workarounds. This is especially true if those workarounds aren't seamless and require client-side changes anyway. That is, rather than teach each client about our special "clojure/dependencyContents" JSON-RPC message, everyone's time would be better spent teaching the editors to handle the jar scheme natively. That way, other JVM language servers can benefit too. If the issue gets solved at the editor level, then it improves the client ecosystem too. Competing clients can also use the same code. That's the approach being taken in joaotavora/eglot#661, which is filed for eglot but looks like it may be fixed in a way that benefits lsp-mode and even CIDER too. I'd be willing to be corrected, but I think that Calva already understands the "jar" scheme without needing "clojure/dependencyContents". Emacs appears to be moving in that direction. That leaves nvim. So, @NoahTheDuke perhaps this is an issue for coc-clojure, coc.nvim, or nvim after all. That is, if I understand correctly, @snoe you aren't attached to the "zipfile" scheme, but instead like the functionality it offers, functionality coc-clojure doesn't currently offer if you use the "jar" scheme. But you'd be happy if nvim could open "jar" scheme files and if those files were editable. Preferably the choice of whether archive files were read-only or editable would be a configuration option in coc-clojure or coc.nvim, but in the worst case it would be OK if nvim could be toggled from a read-only mode into an editable mode. I don't understand the vim ecosystem very well, but I think that vim can open jar files (if not edit them), so it's arguably a bug that nvim can't. So is that the place to start, with a request that nvim learns how to open "jar" scheme files? |
@mainej You are right, I agree entirely. The spec doesn't say server should support reading URI content because I do think they agree that should be client responsibility. |
@mainej that's all largely correct. One of the big problems is that neither zip or jar uris are officially specced. I used the zipfile scheme that vim and nvim both used and was also used by fireplace, in the hopes that emacs had or could add support for the same. Also, since jars ARE zipfiles, support for zip in general would naturally expand to cover jar. I did survey in-the-wild jar uris, and found about 4 or 5 formats. How to address a file within an archive is poorly described by the URI rfcs and the jar ones were a reflection of that - some used It would be interesting to know what the java-lsps do. |
One other point, vimscript is probably the worst client plugin language in use, so that also played into the decision to lift their scheme, knowing that it would be far easier for elisp or js clients to translate into their native archive-file schemes. |
This is a fuckin mess lol. I've tried to keep up but a lot of this is far outside of my knowledge base so I don't have much to offer. I'm down to do what needs to be done, but so far, it seems like the answer is "nothing"? |
They are all different. I have looked into 3, but there are probably more language servers out there
There is nothing in the LSP spec to really help here either. I am looking into submitting an issue over there to see if something could be added to the spec to help mediate these kinds of exchanges where the source file isn't readily available as a file on disk. |
coc.nvim is a bit different from other editors in terms of how it conveys URLs.
"zipfile://some.jar::xxx.clj"
. Other editors tend to follow a "jar" scheme:"jar:file:///some.jar!/xxx.clj"
"zipfile://some.jar%3a%3axxx.clj"
(I think. It may also encode the://
. Needs to be verified).clojure-lsp needs to know how each client handles URIs, to make sure that it stores data internally in a way that the client can access.
So, this is a proposal to add a few options to the client config, to inform clojure-lsp about how coc.nvim likes to handle URIs.
First, to address 1) above, it'd be useful if coc-clojure set
:dependency-scheme "zipfile"
. @ericdallo, clojure-lsp uses"zipfile"
internally, but mentions"zip"
and:zip
in the docs. Can we settle on"zipfile"
?Second, to address 2) above, it'd be useful if coc-clojure set
:uri-scheme "escaped-path"
, or something like that. This would have to be coordinated with @ericdallo, because clojure-lsp doesn't understand this setting yet. See clojure-lsp/clojure-lsp#1287 and clojure-lsp/clojure-lsp#1327 for more discussion.:uri-scheme "escaped-path"
is just a proposal. Other keys or values may be more appropriate depending on which components of the URI are escaped.The text was updated successfully, but these errors were encountered: