Skip to content
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

Support for Emacs #110

Closed
rsrock opened this issue Jul 9, 2017 · 29 comments
Closed

Support for Emacs #110

rsrock opened this issue Jul 9, 2017 · 29 comments

Comments

@rsrock
Copy link

rsrock commented Jul 9, 2017

I've started to play with this in Emacs (using lsp-mode.el), and was finally able to get both sides to talk to each other. Shocked and delighted that jump to symbol works! Here are a couple of questions/issues that I've run into.

  1. I ran into a problem with an empty or malformed message taking down the server right at the start. I solved this by wrapping the process(request, server) line in languageserverinstance.jl with a try-catch block, ignoring any errors. I'm guessing there's a better approach, but I'm obviously not too familiar with the inner workings of this package.

  2. Emacs refuses to quit, because it cannot shut down the language server. It looks like the required message is not implemented here? https://github.com/JuliaEditorSupport/LanguageServer.jl/blob/eff7b98c8a3a3834dc3111ed96abaef0b9e414bd/src/provider_misc.jl#L68

Can that be added? It does make hacking on this somewhat difficult.

Thanks

@MartinWolke
Copy link
Contributor

I started using lsp-mode.el with LanguageServer.jl a few days ago and had the same problems.

The first error might be caused by an empty initialized request send by lsp-mode.el, which leads to a MethodError. I did not have a look into why the empty request is send, I just fixed it by changing the method signature to
function process(r::JSONRPC.Request{Val{Symbol("initialized")}, A}, server) where {A <: Any} . Not very nice, but it seems to work.

The second error can temporarily be fixed by changing the shutdown request to a shutdown notification (lsp--send-notification (lsp--make-notification "shutdown" (make-hash-table))) in the lsp-mode.el package.

@rsrock
Copy link
Author

rsrock commented Jul 10, 2017

Thanks for the tips-- both worked great.

@davidanthoff
Copy link
Member

This is very cool!

On the empty initialized request, that looks like a bug in lsp-mode.el? Or is that something we should address on our end?

I guess the correct way to handle the exist story would be to send an exit notification? I don't really understand the shutdown request, I guess the idea is that the server stops processing messages after that one, but the process doesn't shut down?

Would be great if one of you would add a wiki page on how to get emacs working and then open a PR that adds a link to that wiki page to the README, similar to what we have for Neovim.

@MartinWolke
Copy link
Contributor

The empty initialized notification send by lsp-mode.el looks like this:

Content-Length: 54

{"jsonrpc":"2.0","method":"initialized","params":null}

According to the language server 3.0 protocol, the initialized notification should be of the form:

Notification:

method: 'initialized'
params: void

Looks like it might not be a bug in lsp-mode.el. I could make a PR to change the method signature if its ok with you.

I guess the idea of sending a shutdown request before sending the exit notification is that the server could do some cleanup, but I am not sure what the LanguageServer should do when a shutdown request is issued. Maybe the LanguageServer could just send an empty response?

I can start writing a wiki page over the weekend and hopefully send a PR at the beginning of next week.

@ekmecic
Copy link

ekmecic commented Jul 10, 2017

I've also been trying to get the language server working with emacs and I haven't had much success. If either of you (@MartinWolke @rsrock) could be so kind to post your setup I would be very grateful and I'd be happy to help in any way I could.

@davidanthoff
Copy link
Member

Looks like it might not be a bug in lsp-mode.el. I could make a PR to change the method signature if its ok with you.

That looks right to me, but lets see what @ZacLN says, he has done more on this part of the code lately.

I guess the idea of sending a shutdown request before sending the exit notification is that the server could do some cleanup, but I am not sure what the LanguageServer should do when a shutdown request is issued. Maybe the LanguageServer could just send an empty response?

Yes, just doing nothing on that one also sounds good to me.

Does that mean that we can easily integrate once we have those two changes merged here in LanguageServer.jl? That would be very cool!

@ZacLN
Copy link
Contributor

ZacLN commented Jul 15, 2017

That fix to the method signature looks fine to me, though could it just be changed to
process(r::JSONRPC.Request{Val{Symbol("initialized")}}, server) in correspondence with the signatures to exit and shutdown?

I was confused by the descriptions for shutdown/exit though I suppose then can both just call exit?

@davidanthoff
Copy link
Member

I think we might just try and see what happens if we handle a shutdown request by essentially doing nothing? I assume the emacs client is then still sending an exit request, right?

@ZacLN
Copy link
Contributor

ZacLN commented Jul 15, 2017

Ah ok I get you now, PR

@MartinWolke
Copy link
Contributor

Thank you for making a PR for the shutdown request, @ZacLN. The PR fixes the problem, Emacs is now able to shutdown properly. The emacs-lsp.el language server client always sends an exit notification after sending a shutdown request.

@rsrock
Copy link
Author

rsrock commented Jul 15, 2017

On the Emacs side of things, I was doing this. Hopefully it can be a useful start.

;;; Code:

(require 'lsp-mode)
(require 'julia-mode)

;; patch, until PR #117 lands
(defun lsp--shutdown-cur-workspace ()
  "Shut down the language server process for lsp--cur-workspace"
  (lsp--send-notification (lsp--make-notification "shutdown" (make-hash-table)))
  (lsp--send-notification (lsp--make-notification "exit" nil))
  (lsp--uninitialize-workspace))

;;;###autoload

(lsp-define-stdio-client
  'julia-mode "julia" 'stdio
  #'(lambda () default-directory)
  "Julia Language Server"
  '("julia"
    "--startup-file=no"
    "--history-file=no"
    "-e"
    "using LanguageServer; server = LanguageServer.LanguageServerInstance(STDIN, STDOUT, false); server.runlinter = true; run(server);")
  :ignore-regexps '("^langserver-go: reading on stdin, writing on stdout$"))

(provide 'lsp-julia)
;;; lsp-julia.el ends here

I haven't explored options for the default-directory line yet.

@davidanthoff davidanthoff added this to the Backlog milestone Jul 28, 2017
@davidanthoff
Copy link
Member

Is this solid enough now that we could add a wiki page like for Neovim that describes how to set this up? Or are there still things that we need to fix here to make this a smooth ride? I'm not an Emacs user, so we need some help here with testing, figuring things out etc.

@MartinWolke
Copy link
Contributor

Sorry for the delay. I made a PR to fix the initialized issue. There seems to be a new error caused by an empty message send by the emacs package. After opening a .jl file, the lsp-mode.el package sends a didChange notification with "contentChanges":null. This leads to an error when creating the DidChangeTextDocumentParams struct. I am not yet sure why the emacs package sends an empty didChange request.

While investigating the error, I found it quite usefull to display stackstraces as info messages using the following code in the run method:

     while true
        message = read_transport_layer(server.pipe_in, server.debug_mode)
        request = parse(JSONRPC.Request, message)
        server.isrunning && serverbusy(server)
        process(request, server)
        server.isrunning && serverready(server)
        try
            message = read_transport_layer(server.pipe_in, server.debug_mode)
            request = parse(JSONRPC.Request, message)
            server.isrunning && serverbusy(server)
            process(request, server)
            server.isrunning && serverready(server)
        catch
            info("Stackframe: ")
            foreach(s -> info("$s"), catch_stacktrace())
        end
     end

Something like this might be a quite useful feature.

@MartinWolke
Copy link
Contributor

The error mentioned in the last post is now fixed. I added a wiki entry on how to setup Emacs to work with the LanguageServer. I hope that everything works, but it would be great if someone could test it and provide feedback.

@davidanthoff
Copy link
Member

Perfect, thanks so much! I've asked for some more testers on the julia slack channel.

@tpapp
Copy link

tpapp commented Aug 18, 2017

I tried it out, following the instructions in the wiki but could not get LanguageServer.jl to load (issue here). Is there something that was not made explicit in the wiki, about package versions or related?

@tpapp
Copy link

tpapp commented Aug 18, 2017

Got it working. Current glitches:

Unknown method: window/setStatusReady
Unknown method: window/setStatusBusy

in Emacs. They seem to block cursor movement occasionally, maybe something was compiled or parsed.

Otherwise it is amazing. Thanks!

I will be using it from now on. If I find specific issues, should they be reported in this one, or a new one?

@MartinWolke
Copy link
Contributor

Thank you for testing! It should be possible to disable the window/setStatusReady popups in the lsp-julia package by telling lsp-mode to ignore them. I made an issue in the lsp-julia package and will let you know once its fixed.

@ZacLN
Copy link
Contributor

ZacLN commented Aug 18, 2017

Fantastic! As a temporary fix you can comment out lines 34 and 36 of LanguageServer/src/languageserverinstance.jl that make reference to serverbusy and serverready

@davidanthoff
Copy link
Member

Could we somehow use the server capabilities stuff at initialization to decide whether we want to send these non-standard messages?

@MartinWolke
Copy link
Contributor

One quick option might be to activate non-standard messages using the experimental field in the ClientCapabilities struct at initialization.

interface ClientCapabilities {
	/**
	 * Workspace specific client capabilities.
	 */
	workspace?: WorkspaceClientCapabilities;

	/**
	 * Text document specific client capabilities.
	 */
	textDocument?: TextDocumentClientCapabilities;

	/**
	 * Experimental client capabilities.
	 */
	experimental?: any;
}

According to the 3.0 protocol, another option is to add a new field to the `ClientCapabilities' struct. "Servers receiving a ClientCapabilities object literal with unknown properties should ignore these properties. A missing property should be interpreted as an absence of the capability. "

@tpapp
Copy link

tpapp commented Aug 24, 2017

I found that I cannot exit Emacs if the language server is not responding: in *Messages*, I have

lsp--stdio-send-sync: Julia Language Server: Cannot communicate with the process (exit)

How can I force it to stop?

@MartinWolke
Copy link
Contributor

@tpapp Sometimes the communication to the language server seems to crash. I am not yet sure why. Does terminating the process (list-processes command in emacs) manually before trying to exit emacs work for you?

Btw., the lsp-julia package should now ignore the serverBusy and serverReady message now.

@tpapp
Copy link

tpapp commented Nov 15, 2017

I experimented with Emacs & LanguageServer.jl earlier, but went back to julia-repl for daily work in September. I would like to try it again, and have two questions:

  1. is anyone using it for productive work? is it stable enough? I can live with the occasional quirk.
  2. can someone share a snippet for lsp-define-stdio-client for use with Julia? I am unsure about the server path (should it be the julia executable?) and the arguments.

@rsrock
Copy link
Author

rsrock commented Nov 15, 2017

I've toyed with it too, but turned it off recently. From what I've seen, it works well. One annoyance is that eldoc shoves the documentation in the minibuffer. So just moving the point around makes the minibuffer resize like crazy because we have multiline docs.

I was starting to look into sending the docs to a dedicated buffer instead (perhaps triggered by a keybinding), but it wasn't a top priority for me right now.

@MartinWolke
Copy link
Contributor

As a workaround I just display the first line of a multiline comment:

(defun julia--hover-format (message)
  (if eldoc-echo-area-use-multiline-p
    message
    (car (split-string message "\n"))))

(advice-add #'lsp--text-document-hover-string :filter-return #'julia--hover-format)

My lsp-define-stdio-client setup looks like this:

(require 'ess-site)
(require 'julia-mode)
(require 'lsp-mode)

(defun lsp-julia--get-root ()
  "Try to find the package directory by searching for a .gitignore file.
If no .gitignore file can be found use the default directory "
  (let ((dir (locate-dominating-file default-directory ".gitignore")))
    (if dir
        (expand-file-name dir)
      default-directory)))

(defun lsp-julia--rls-command ()
  `("julia" "--startup-file=no" "--history-file=no" "-e"
    "using LanguageServer; server = LanguageServer.LanguageServerInstance(STDIN, STDOUT, false); server.runlinter = true; run(server);"))

(defconst lsp-julia--handlers
  '(("window/setStatusBusy" .
     (lambda (w _p)))
    ("window/setStatusReady" .
     (lambda(w _p)))))

(defun lsp-julia--initialize-client(client)
  (mapcar #'(lambda (p) (lsp-client-on-notification client (car p) (cdr p))) lsp-julia--handlers))

(lsp-define-stdio-client lsp-julia "julia" #'lsp-julia--get-root nil
                         :command-fn #'lsp-julia--rls-command
                         :initialize #'lsp-julia--initialize-client)

(provide 'lsp-julia)

julia has to be in the path and the LanguageServer package has to be installed. The helm-xref package is very useful if you use helm. Here is my setup for helm-xref:

(defun julia--line-at-location (file line)
  (with-temp-buffer
    (find-file file)
    (goto-char (point-min))
    (forward-line line)
    (substring (thing-at-point 'line t) 0 -2)))

(defun julia--location-to-xref (location)
  "Convert Location object LOCATION to an xref-item.
interface Location {
    uri: string;
    range: Range;
}"
  (lsp--send-changes lsp--cur-workspace)
  (let ((uri (string-remove-prefix "file://" (gethash "uri" location)))
        (ref-pos (gethash "start" (gethash "range" location))))
    (xref-make (julia--line-at-location uri (gethash "line" ref-pos))
               (xref-make-file-location uri
                                        (1+ (gethash "line" ref-pos))
                                        (gethash "character" ref-pos)))))

  ;; use helm-xref
  (require 'helm-xref)
  (setq xref-show-xrefs-function 'helm-xref-show-xrefs)

  ;; replace xref message
  (defun lsp--location-to-xref (location)
    (julia--location-to-xref location))

Overall the setup is useable for me, initializing a large project takes a few seconds though.

@gdkrmr
Copy link

gdkrmr commented Dec 22, 2017

I tried it and the language server keeps crashing with different error messages, e.g.:

WARNING: Module URIParser with uuid 26552956908380 is missing from the cache.
This may mean module URIParser does not support precompilation but is imported by a module that does.
ERROR: LoadError: Declaring precompile(false) is not allowed in files that are being precompiled.
Stacktrace:
[1] _require(::Symbol) at ./loading.jl:455
[2] require(::Symbol) at ./loading.jl:405
[3] _include_from_serialized(::String) at ./loading.jl:157
[4] _require_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:200
[5] _require_search_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:236
[6] _require(::Symbol) at ./loading.jl:441
[7] require(::Symbol) at ./loading.jl:405
[8] _include_from_serialized(::String) at ./loading.jl:157
[9] _require_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:200
[10] _require_search_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:236
[11] _require(::Symbol) at ./loading.jl:441
[12] require(::Symbol) at ./loading.jl:405
[13] include_from_node1(::String) at ./loading.jl:576
[14] include(::String) at ./sysimg.jl:14
[15] anonymous at ./:2
while loading /home/me/.julia/v0.6/PyCall/src/PyCall.jl, in expression starting on line 28

@gdkrmr
Copy link

gdkrmr commented Dec 18, 2018

Update: emacs support works pretty well, there are currently issues, because lsp-mode master has some breaking changes and LanguageServer.jl is not officially released for Julia 1.0 yet, but master works just fine. See gdkrmr/lsp-julia/pull/5.

@ordicker
Copy link

LanguageServer.jl works pretty well on emacs (both lsp-mode and eglot), so I think this issue should be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants