Closed gavocanov closed 5 years ago
In the above new commit I have attempted to solve your problem by allowing the "CONTACT" cdr
in an element of eglot-server-programs
to be a function.
I also have tentatively added the command line arguments you mention as a default value for a new java-mode
element. Please test and report back if it works.
I know very little about Java these days. Can you tell me if the command-line arguments you have above are generic enough for a substantial amount of java users or are they very specific to your setup?
@gavocanov , notice that the new commit is not in master yet. You must checkout to it directly or to the scratch/attempt-at-63
branch.
I think the -data
program argument is path to your global eclipse workspace and you pass path to project root via workspaceFolders
key in initializationOptions
via LSP. To do that you can create your own subclass of eglot-lsp-server
and override eglot-initialization-options
. Look at cquery server for an example. There's also a connection example at eclipse.jdt.ls wiki
I know very little about Java these days. Can you tell me if the command-line arguments you have above are generic enough for a substantial amount of java users or are they very specific to your setup?
I think we'll also need a couple of defcustoms for path to the jar and config used (not just config_mac
)
Yes, path to the jar, jar version, and config should be defcustoms, the command line arguments are not generic. Also, it works only with java 1.8, 9 is experimental, 10/11 are not supported at all and the server will not start. I suppose we'd need a defcustom for java executable also.
@mkcms is right, -data
param is for the global eclipse workspace (don't ask...they tend to complicate stuff), and subclassing like it's done in cquery
seems to work ok, but it works ok even without subclassing anything by just defining the -data
param to be a constant.
So, no need for function in eglot-server-programs
, at least not for java.
Thnx for your help guys!
@mkcms, @gavocanov: off the bat: no server-specific defcustoms, sorry.
-data param is for the global eclipse workspace
In your other message you said it worked based on projectile-project-root
, so this is why I presented this solution.
Can you show the skeleton of a minimal java project and show the actual args used for a successful invocation of the server for that project?
off the bat: no server-specific defcustoms, sorry.
I thought you might say that and I agree that eglot should be minimal like that. I'll look into adding support for this server just by using JAVA_HOME
and CLASSPATH
environment variables.
I thought you might say that
;)
JAVA_HOME and CLASSPATH environment variables.
good idea
I came up with the following, although I don't know how correct it is. This server is badly documented. Also I found that you can actually build a single executable of it with maven... But it's name is just eclipse
.
From 33442448cfad3bd2bef26506b7564256d18d938c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20K?= <k.michal@zoho.com>
Date: Tue, 7 Aug 2018 00:13:45 +0200
Subject: [PATCH] Improve eclipse.jdt.ls server support
---
README.md | 4 ++--
eglot.el | 61 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/README.md b/README.md
index f836b7f..8d35443 100644
--- a/README.md
+++ b/README.md
@@ -32,7 +32,7 @@ for the language of your choice. Otherwise, it prompts you to enter one:
* PHP's [php-language-server][php-language-server]
* C/C++'s [cquery][cquery]
* Haskell's [IDE engine][haskell-ide-engine]
-* Java's [Eclipse JDT Language Server][eclipse-jdt]
+* Java's [Eclipse JDT Language Server][eclipse-jdt] (if you [download and extract it][jdt-langserver-download-link] and point `CLASSPATH` environment variable at the `org.eclipse.equinox.launcher` jar file)
I'll add to this list as I test more servers. In the meantime you can
customize `eglot-server-programs`:
@@ -279,4 +279,4 @@ Under the hood:
[windows-subprocess-hang]: https://www.gnu.org/software/emacs/manual/html_node/efaq-w32/Subprocess-hang.html
[haskell-ide-engine]: https://github.com/haskell/haskell-ide-engine
[eclipse-jdt]: https://github.com/eclipse/eclipse.jdt.ls
-
+[jdt-langserver-download-link]: http://download.eclipse.org/jdtls/snapshots/jdt-language-server-latest.tar.gz
diff --git a/eglot.el b/eglot.el
index d1e5c95..96a9dd2 100644
--- a/eglot.el
+++ b/eglot.el
@@ -94,19 +94,34 @@ language-server/bin/php-language-server.php"))
(haskell-mode . ("hie-wrapper"))
(java-mode
. ,(lambda ()
- `("java"
- "-Declipse.application=org.eclipse.jdt.ls.core.id1"
- "-Dosgi.bundles.defaultStartLevel=4"
- "-Declipse.product=org.eclipse.jdt.ls.core.product"
- "-Dlog.protocol=true"
- "-Dlog.level=ALL"
- "-noverify"
- "-Xmx1G"
- "-jar" "plugins/org.eclipse.equinox.launcher_1.5.100.v20180611-1436.jar"
- "-configuration" "config_mac"
- "-data" ,(if-let ((proj (project-current)))
- (car (project-roots proj))
- default-directory)))))
+ (when-let* ((classpath (getenv "CLASSPATH"))
+ (jar (cl-find-if
+ (lambda (path)
+ (string-match-p
+ "org\\.eclipse\\.equinox\\.launcher_.*.jar$"
+ (file-name-nondirectory path)))
+ (split-string classpath ":")))
+ (config
+ (expand-file-name
+ (cond
+ ((string= system-type "darwin") "config_mac")
+ ((string= system-type "windows-nt") "config_win")
+ (t "config_linux"))
+ (expand-file-name ".." (file-name-directory jar))))
+ (workspace (locate-user-emacs-file "eglot-workspace")))
+ (cons 'eglot-eclipse-jdt
+ (list (executable-find "java")
+ "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=1044"
+ "-Declipse.application=org.eclipse.jdt.ls.core.id1"
+ "-Dosgi.bundles.defaultStartLevel=4"
+ "-Declipse.product=org.eclipse.jdt.ls.core.product"
+ "-Dlog.protocol=true"
+ "-Dlog.level=ALL"
+ "-noverify"
+ "-Xmx1G"
+ "-jar" jar
+ "-configuration" config
+ "-data" workspace))))))
"How the command `eglot' guesses the server to start.
An association list of (MAJOR-MODE . CONTACT) pairs. MAJOR-MODE
is a mode symbol, or a list of mode symbols. The associated
@@ -1683,6 +1698,26 @@ If SKIP-SIGNATURE, don't try to send textDocument/signatureHelp."
:progressReportFrequencyMs -1)))
+;;; eclipse-jdt-specific
+;;;
+(defclass eglot-eclipse-jdt (eglot-lsp-server) ()
+ :documentation "eclipse's jdt langserver.")
+
+(cl-defmethod eglot-initialization-options ((server eglot-eclipse-jdt))
+ "Passes through required jdt initialization options"
+ `(:workspaceFolders
+ [,(eglot--path-to-uri (car (project-roots (eglot--project server))))]
+ ,@(if-let* ((home
+ (or (getenv "JAVA_HOME")
+ (ignore-errors
+ (expand-file-name
+ ".."
+ (file-name-directory
+ (file-chase-links (executable-find "javac"))))))))
+ `(:settings (:java (:home ,home)))
+ (ignore (eglot--warn "JAVA_HOME env var not set")))))
+
+
;; FIXME: A horrible hack of Flymake's insufficient API that must go
;; into Emacs master, or better, 26.2
(when (version< emacs-version "27.0")
--
2.11.0
@mkcms your code looks fine, should work and it's exactly what I came up to without dynamic vars and it works.
@joaotavora I agree and understand about defcustoms per lsp server impl, makes total sense.
Not sure how well JAVA_HOME will work for me personally, on OS level (MacOS here) it's set to latest version (10/11) but I need to use 1.8 for this server to work. These days as a Java dev I'm changing Java environment for almost every project.
I guess I'll be overriding it and it will be possible, right ?
I'm sure it will be a good starter and default to have in eglot anyway.
. ,(lambda ()
@mkcms, OK, but note we only need the lambda if the conditions likely to change between the moment that eglot.el
is initialized and invocations of M-x eglot
. If you suspect they don't then the lambda isn't needed.
+(cl-defmethod eglot-initialization-options ((server eglot-eclipse-jdt))
- "Passes through required jdt initialization options"
- `(:workspaceFolders
We might as well support :workspaceFolders
for all servers: notice that project-roots
returns a list (I remember the discussion in emacs-devel when that function was designed and it hinged partially on Java's multiple project roots). It is that list that should be fed to :workspaceFolders
. Nonwithstanding we probably still need the eglot-eclipse-jdt
specifics since I doubt that project-current
understands that it's being called for a multiple-root Java project (it could be made to tho, and that would be the correct way).
We might as well support :workspaceFolders for all servers
You mean default eglot-initialization-options
should have :workspaceFolders
key?
Yeah, or whatever it takes to officially support it (would have to see the spec). But a default specialization for that methods sounds good (perhaps conditional on the capability), with the remaining methods appending to call-next-method
whatever it takes to officially support it (would have to see the spec).
workspaceFolders
ininitializationOptions
is non-standard, maybe we should implement it directly in initialize
instead.
workspaceFolders ininitializationOptions is non-standard
Yes. I read this in the spec.
"If the client supports workspace folders and announces them via the corrsponding workspaceFolders client capability the InitializeParams contain an additional property workspaceFolders with the configured workspace folders when the server starts"
So, minimally we must annouce the capability and send workspaceFolders
in the InitializeParams
object. One thing I still don't understand is if we should send null here if there is no (cdr (project-roots <project>))
or if we should send something based on all (project-roots)
, even if there is only one.
I think we should start with this minimal implementation, and only then move on to support the workspaceFolders
server request and the didChangeWorkspaceFolders
notifications, in that order.
Now, this won't solve the java problem just yet, because project-roots
won't return the JAVA_HOME thing.
But I'd like to first understand if it works at all (perhaps by hardcoding JAVA_HOME in there, or defining a new project class).
Are you up for this or do you prefer I try it out? Obviously your advantage is that you have a working Java environment, and I have not,
Are you up for this or do you prefer I try it out?
You can go ahead.
Obviously your advantage is that you have a working Java environment, and I have not,
I just downloaded openjdk with my system package manager and eclipse + eclipse.jdt.ls from eclipse website. It's self contained, so if you want to temporarily install it just extract it and then delete this folder (I put it in ~/opt
).
I also wonder if we can have an "interactive" lambda as a contact? So that if we fail to find the required programs/jars we can ask the user for a path from it.
Are you up for this or do you prefer I try it out? Obviously your advantage is that you have a working Java environment, and I have not,
Never mind, I did it myself (the minimal bit). Can you try it with your java setup? Instead of defining a new server class I suggest you add a method to project-roots
.
I also wonder if we can have an "interactive" lambda as a contact? So that if we fail to find the required programs/jars we can ask the user for a path from it.
Good question. Try adding some read-from-minibuffer
to the lambda and see if it breaks. Thinking about eglot-ensure
, you might need to check called-interactively-p
, but that function is problematic. The lambda is called from two places, but theoretically shouldn't ever be called twice.
I just downloaded openjdk with my system package manager and eclipse + eclipse.jdt.ls from eclipse website. It's self contained, so if you want to temporarily install it just extract it and then delete this folder (I put it in ~/opt).
OK I'll try it.
By the way, the workspaceFolders
part of the spec is shady, if the minimal approach works I'm not sure It's worth implementing any more of it for now.
Thinking about eglot-ensure, you might need to check called-interactively-p, but that function is problematic.
Maybe eglot can call the contact with call-interactively
if it checks that it's an interactive lambda? Then one could just do (interactive (list t))
to know if the lambda is called interactively.
Maybe eglot can call the contact with call-interactively if it checks that it's an interactive lambda? Then one could just do (interactive (list t)) to know if the lambda is called interactively.
But what should we do when the call is non-interactive, like from eglot-ensure
?
eglot-ensure
calls eglot--guess-contact
with nil interactive
argument and eglot
calls it with t
. So if interactive
is nil, we can always call funcall
on the contact and otherwise call call-interactively
. Would this work?
Would this work?
No, because call-interactively
provides an arbitrary number of arguments to the function according to its interactive
spec. That's fine, but when you call it via funcall
you have to provide the same number of arguments.
To do what you want, it's better just to specify in eglot-server-programs
that the function is a function of one argument which is t
if the call is interactive. Then test that argument whenever a read-from-minibuffer
is necessary.
But can you first show an example of such a function?
Can you try it with your java setup?
You forgot to eval inner expr in initialize. I'm getting this:
client-request (id:1) Wed Aug 8 16:56:19 2018:
(:jsonrpc "2.0" :id 1 :method "initialize" :params
(:processId
(unless
(eq
(jsonrpc-process-type server)
'network)
(emacs-pid))
:rootPath
(expand-file-name default-directory)
:rootUri
(eglot--path-to-uri default-directory)
:initializationOptions
(eglot-initialization-options server)
:capabilities
(eglot-client-capabilities server)))
server-reply (id:1) ERROR Wed Aug 8 16:56:19 2018:
(:jsonrpc "2.0" :id 1 :error
(:code -32700 :message "Message could not be parsed." :data
(:message "java.lang.IllegalStateException: Expected an int but was BEGIN_ARRAY at line 1 column 70 path $.params.processId")))
Right. Silly me. If you fix that does it work?
On Wed, Aug 8, 2018, 15:59 mkcms notifications@github.com wrote:
Can you try it with your java setup?
You forgot to eval inner expr in initialize. I'm getting this:
client-request (id:1) Wed Aug 8 16:56:19 2018: (:jsonrpc "2.0" :id 1 :method "initialize" :params (:processId (unless (eq (jsonrpc-process-type server) 'network) (emacs-pid)) :rootPath (expand-file-name default-directory) :rootUri (eglot--path-to-uri default-directory) :initializationOptions (eglot-initialization-options server) :capabilities (eglot-client-capabilities server)))
server-reply (id:1) ERROR Wed Aug 8 16:56:19 2018: (:jsonrpc "2.0" :id 1 :error (:code -32700 :message "Message could not be parsed." :data (:message "java.lang.IllegalStateException: Expected an int but was BEGIN_ARRAY at line 1 column 70 path $.params.processId")))
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/joaotavora/eglot/issues/63#issuecomment-411436911, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXnw0i-eUYGhEhYCJPTCZKsjlqLtlUKks5uOvzRgaJpZM4Vwq9g .
Hmm, I don't see any difference in how the server behaves. In fact I don't see any difference with :workspaceFolders
in initializationOptions
neither (I thought the wiki said it was required, I was wrong). But I'm only testing this on a very simple project.
But can you first show an example of such a function?
Here's an example. I actually changed it to ask for root eclipse.jdt.ls directory instead of the jar, as it's easier. I'm not sure if the directory structure will never change though.
(lambda ()
(interactive)
(cl-labels
((is-the-jar
(path)
(string-match-p
"org\\.eclipse\\.equinox\\.launcher_.*.jar$"
(file-name-nondirectory path))))
(let* ((classpath (or (getenv "CLASSPATH") ":"))
(jar (cl-find-if #'is-the-jar (split-string classpath ":")))
(dir
(cond
(jar (expand-file-name ".." (file-name-directory jar)))
((called-interactively-p 'any)
(read-directory-name
(concat "Path to eclipse.jdt.ls directory (could not"
" find it in CLASSPATH): ")
nil nil t))
(t (error "Could not find eclipse.jdt.ls jar"))))
(repodir
(file-name-as-directory
(concat dir
"org.eclipse.jdt.ls.product/target/repository")))
(config
(concat
repodir
(cond
((string= system-type "darwin") "config_mac")
((string= system-type "windows-nt") "config_win")
(t "config_linux"))))
(workspace (make-temp-file "eglot-workspace" t)))
(unless jar
(setq jar
(cl-find-if #'is-the-jar
(directory-files (concat repodir "plugins") t))))
(unless (and jar (file-exists-p jar) (file-directory-p config))
(error "Could not find required eclipse.jdt.ls files ('mvn package' required?)"))
(cons 'eglot-eclipse-jdt
(list (executable-find "java")
"-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=1044"
"-Declipse.application=org.eclipse.jdt.ls.core.id1"
"-Dosgi.bundles.defaultStartLevel=4"
"-Declipse.product=org.eclipse.jdt.ls.core.product"
"-Dlog.protocol=true"
"-Dlog.level=ALL"
"-noverify"
"-Xmx1G"
"-jar" jar
"-configuration" config
"-data" workspace)))))
It actually kind of works for me.
It actually kind of works for me.
Nice. We should:
eglot--eclipse-java-contact
or something like that).INTERACTIVE
arg. Test that instead of `called-interactively-p.What are you doing vis-à-vis the project-roots
and workspaceFolders
thing? I see that you are passing a dummy workspace dir as data
. What is really happening there?
BTW I think that it wouldn't be unreasonable to ask for this LSP server to come bundled with a script (or something like that) that does all this. It's kind of ridiculous that the LSP client has to. I can make a pull request there.
What are you doing vis-à-vis the project-roots and workspaceFolders thing?
Currently nothing and it seems to work. This server is really undocumented so I don't even know what should go there. I assume that subdirs with pom.xml
(or gradle files) should be included there?
I see that you are passing a dummy workspace dir as data. What is really happening there?
I think it's just a cache directory for the server. I tried putting real eclipse workspaces there and the server works, but it gives the errors "classpath is incomplete, only syntax errors will be reported". Not providing this argument also gives the same errors.
BTW I think that it wouldn't be unreasonable to ask for this LSP server to come bundled with a script (or something like that) that does all this. It's kind of ridiculous that the LSP client has to. I can make a pull request there.
That would be great. It's even possible to build standalone executables of this server (I got mine in ./org.eclipse.jdt.ls.product/target/products/languageServer.product/linux/gtk/x86_64/eclipse
) although I don't even remember how I did it now (also it gave the same "classpath is incomplete" errors).
name this function (eglot--eclipse-java-contact or something like that). make it accept a single INTERACTIVE arg. Test that instead of `called-interactively-p.
Ok, I pushed these changes to a new branch. I also made a change to call the contact interactively if appropriate.
https://github.com/joaotavora/eglot/compare/master...mkcms:feature/eclipse.jdt.ls-server
Currently nothing and it seems to work. This server is really undocumented so I don't even know what should go there.
So are you saying it would work without the workspaceFolders
addition at all?
I assume that subdirs with pom.xml (or gradle files) should be included there?
I don't understand. Are subdirs of the root of a Java project also considered roots of a java project?
Ok, I pushed these changes to a new branch. I also made a change to call the contact interactively if appropriate.
It's time to make a pull request to better review these things. In particular I notice you're still using call-interactively
. You don't need to. Just use funcall
and pass the interactive
arg in eglot--guess-contact
.
So are you saying it would work without the workspaceFolders addition at all?
It seems to work for me, but I'm testing it in a simple project.
Are subdirs of the root of a Java project also considered roots of a java project?
I meant more generally subprojects of a project.
Is there anything in the spec that actually says what workspace folders are? Even the vscode website isn't helpful. Also you mentioned that the design decision to make project-roots
return a list was influenced by Java multiple project roots. Is there an example of such a setup? The documentation says
Most often it’s just one directory which contains the project build file and everything else in the project. But in more advanced configurations, a project can span multiple directories.
To me it's obvious that a project can span multiple directories, like src
in most projects - should it be considered a "root"?
Lsp-mode already have support for all of the custom code that you need to make jdt work via https://github.com/emacs-lsp/lsp-java you may use it as a guide where appropriate. In general without looking at vscode extension code it is impossible to make jdt work . There are couple of extension points that have to be added in eglot to make jdt work
of extension points that have to be added in eglot to make jdt work
Hi @yyoncho. Thanks for the pointer. I don't have vscode (@mkcms do you?). Can you clarify hat each of these is? I'm having trouble linking them to the spec.
- Ability to apply actions client side
Ins't this codeAction
or executeCommand
? I think Eglot already has resonable support for those. What's missing
- Ability to override buffer Uri
What do you mean?
- The code that resolves buffer content for xrefs should be able to load the content from the server .
I'm even more confused by this one.
I don't have vscode (@mkcms do you?).
I don't.
Ins't this codeAction or executeCommand? I think Eglot already has resonable support for those. What's missing
This server executes codeActions in a nonstandard way:
https://github.com/eclipse/eclipse.jdt.ls/issues/376. This is one reason why I implemented the eglot-execute-command
defgeneric in my PR (there are other reasons as well)
This server executes codeActions in a nonstandard way:
So much for "the spec"...
This is one reason why I implemented the eglot-execute-command defgeneric in my PR
Yes, now I understand this problem.
(there are other reasons as well)
Can I have one or two of those?
Can I have one or two of those?
Sure, I will make a writeup in the PR later.
Jdt expects uris not part of the to be with custom prefix (jdt:) . You may check lsp-java implementation. I have limited internet access so I cannot get into details.
@gavocanov @yyoncho Can you test this PR? https://github.com/joaotavora/eglot/pull/65
Everything works as expected, if there is no CLASSPATH it asks, if there is it starts with defaults and all works fine.
The only thing I'd change is the temporary folder created for the workspace. Should not be a temporary directory, should be a shared directory for all java-mode projects IMO, now for every eglot session all the project metadata is recreated instead of being persisted and incrementally updated. Makes startup/first completion (company) etc. slow for no good reason.
One more thing worth taking a look at is what @yyoncho was referring to, uris with prefix, I think that this would fix goto definition for stuff in dependencies if source jars available (mvn dependency:sources
) or return a decompiled java file (like IDEA/Eclipse do).
Other then that, great work, thnx a lot!
The only thing I'd change is the temporary folder created for the workspace. Should not be a temporary directory, should be a shared directory for all java-mode projects IMO, now for every eglot session all the project metadata is recreated instead of being persisted and incrementally updated. Makes startup/first completion (company) etc. slow for no good reason.
I guess a <user-emacs-directory>/eglot-eclipse-jdt-cache/
dir should be used then. @mkcms can you add this to #65?
@gavocanov @joaotavora Thanks, I just pushed that change to the PR branch.
One more thing worth taking a look at is what @yyoncho was referring to, uris with prefix, I think that this would fix goto definition for stuff in dependencies if source jars available
The problem is that the server is really undocumented. And even if it was, I think the solution would be too hacky for eglot.
I haven't read all the discussions but if you talked about root detection logic (as projectile-project-root
is mentioned). I raised a topic in lsp-mode https://github.com/emacs-lsp/lsp-mode/issues/293
In emacs-ccls, the default logic I use (https://github.com/MaskRay/emacs-ccls/commit/b02a5d8eaaecc84a423c1edc70e32c9aefc0010a#diff-2093f0a348c7bfdc7e0a9006283e081fR100):
(locate-dominating-file default-directory ".ccls-root")
if it exists(projectile-project-root)
otherwiseIf a project (say llvm, clickhouse) has subprojects (e.g. git submodules), we don't want files in subprojects think they are within their own project ((projectile-project-root)
).
Someone added build/compile_commands.json
to emacs-cquery, which I really dislike.
Hi,
I'm trying to use java lsp and all is working great, but I need to pass a command line argument for the workspace path when starting the lsp server.
I do not see a way to do it with the current options, is there anyway I can accomplish that ?
I should ad "-data" as the last option with project root. If I add a hardcoded path all works fine but I need this to be dynamic, ideally using
(projectile-project-root)
or something similar.Thnx in advance and keep up the good work, eglot is really great!