gdkrmr / lsp-julia

MIT License
70 stars 19 forks source link

Potential issue of lsp-julia--rls-command #49

Open xukai92 opened 3 years ago

xukai92 commented 3 years ago

I'm a bit confused of the function, especially this part https://github.com/non-Jedi/lsp-julia/blob/master/lsp-julia.el#L279-L289

In short, this essentially gives something like

julia --startup-file=no --history-file=no -eimport Pkg; Pkg.instantiate(); ...

instead of

julia --startup-file=no --history-file=no -e 'import Pkg; Pkg.instantiate(); ...'

i.e. the quotation is missing.

How the current master is working at all?

PS: I found this when making lsp-julia work over TRAMP, which I have to add the quotation around all the part after -e to make things work.

gdkrmr commented 3 years ago

How the current master is working at all?

Magic? :-)

Thanks for noting, if you have already fixed it, could you make a PR?

xukai92 commented 3 years ago

Actually adding quotes would fail the local version, but works with the TRAMP version. For more context, here are the related codes for the TRAMP version (note the "'" I added around the body codes)

  (defun lsp-julia--rls-command-remote ()
    "The command to lauch the Julia Language Server."
    `(,lsp-julia-command
      ,@lsp-julia-flags
      ,(concat "-e "
               "'"
               "import Pkg; Pkg.instantiate(); "
               "using InteractiveUtils, Sockets, SymbolServer, LanguageServer; "
               "Union{Int64, String}(x::String) = x; "
               "server = LanguageServer.LanguageServerInstance("
               "stdin, stdout, "
               (lsp-julia--get-root-remote) ", "
               (lsp-julia--get-depot-path) ", "
               "nothing, "
               (lsp-julia--symbol-server-store-path-to-jl-no-expand) "); "
               "run(server);"
               "'")))

  (lsp-register-client
    (make-lsp-client :new-connection (lsp-tramp-connection 'lsp-julia--rls-command-remote)
                     :major-modes '(julia-mode ess-julia-mode)
                     :server-id 'julia-ls-remote
                     :remote? t))

Here lsp-julia--get-root-remote and lsp-julia--symbol-server-store-path-to-jl-no-expand are just some of my work-arounds to handle TRAMP path. This version works well for me.

However, if I use the same quotes in the local version, i.e.

 (defun lsp-julia--rls-command()
    "The command to lauch the Julia Language Server."
    `(,lsp-julia-command
      ,@lsp-julia-flags
      ,(concat "-e "
               "'"
               "import Pkg; Pkg.instantiate(); "
               "using InteractiveUtils, Sockets, SymbolServer, LanguageServer; "
               "Union{Int64, String}(x::String) = x; "
               "server = LanguageServer.LanguageServerInstance("
               "stdin, stdout, "
               (lsp-julia--get-root) ", "
               (lsp-julia--get-depot-path) ", "
               "nothing, "
               (lsp-julia--symbol-server-store-path-to-jl) "); "
               "run(server);"
               "'")))

I would get

Process julia-ls stderr finished
ERROR: syntax: cannot juxtapose string literal
Stacktrace:
 [1] top-level scope
   @ none:1

I think it might trace down to the difference between lsp-stdio-connection and lsp-tramp-connection, which call the rls-command? :|

gdkrmr commented 3 years ago

How is your setup? I have never gotten lsp-julia to work with remote projects. You open a file from a julia project on the remote server and want the language server to spawn that same server? Is it possible to have a local language server work on the remote files?

lsp-tramp-connection is provided by lsp-mode, right?

xukai92 commented 3 years ago

How is your setup?

This is my full setup (contains hacks/bugfixes collected from multiple places, see my comments in codes)

FULL SETUP

```elisp ;; Julia over TRAMP (after! tramp ;; Ignore "Remote file error: Forbidden reentrant call of Tramp" ;; Ref: https://www.gnu.org/software/tramp/ (setq debug-ignored-errors (cons 'remote-file-error debug-ignored-errors)) (add-to-list 'tramp-remote-path "~/bin") (add-to-list 'tramp-remote-path 'tramp-own-remote-path)) ;; To make lsp-mode work over TRAMP ;; Ref: https://github.com/emacs-lsp/lsp-mode/pull/2531 (after! tramp (defun lsp-tramp-connection (local-command &optional generate-error-file-fn) "Create LSP stdio connection named name. LOCAL-COMMAND is either list of strings, string or function which returns the command to execute." ;; 2.5.0-pre (as built from native-comp branch before M Albinus released tramp-2.5) ;; worked fine (defvar tramp-version) (defvar tramp-connection-properties) (when (version< tramp-version "2.5.0-pre") (lsp-warn "Your tramp version - %s - might fail to work with remote LSP. Update to version 2.5 or greater (available on elpa)" tramp-version)) ;; Force a direct asynchronous process. (add-to-list 'tramp-connection-properties (list (regexp-quote (file-remote-p default-directory)) "direct-async-process" t)) (list :connect (lambda (filter sentinel name environment-fn) (let* ((final-command (lsp-resolve-final-function local-command)) (stderr-buf (or (when generate-error-file-fn (funcall generate-error-file-fn name)) (format "/tmp/%s-%s-stderr" name (cl-incf lsp--stderr-index)))) (process-name (generate-new-buffer-name name)) (process-environment (lsp--compute-process-environment environment-fn)) (proc (make-process :name process-name :buffer (format "*%s*" process-name) :command final-command :connection-type 'pipe :coding 'no-conversion :noquery t :filter filter :sentinel sentinel :stderr stderr-buf :file-handler t))) (cons proc proc))) :test? (lambda () (-> local-command lsp-resolve-final-function lsp-server-present?))))) ;; Register lsp-julia for remote mode (after! lsp-julia (defun lsp-julia--get-root-remote () "Get the (Julia) project root directory of the current file over TRAMP." (concat "\"" (tramp-file-name-localname (tramp-dissect-file-name (expand-file-name (or (locate-dominating-file default-directory "Project.toml") (locate-dominating-file default-directory "JuliaProject.toml") lsp-julia-default-environment)))) "\"")) (defun lsp-julia--symbol-server-store-path-to-jl-no-expand () "Convert the variable `lsp-julia-symbol-server-store-path' to a string or \"nothing\" if `nil'" (if lsp-julia-symbol-server-store-path (let ((sssp lsp-julia-symbol-server-store-path)) (make-directory sssp t) (concat "\"" sssp "\"")) "nothing")) (defun lsp-julia--rls-command-remote () "The command to lauch the Julia Language Server." `(,lsp-julia-command ,@lsp-julia-flags ,(concat "-e " "'" "import Pkg; Pkg.instantiate(); " "using InteractiveUtils, Sockets, SymbolServer, LanguageServer; " "Union{Int64, String}(x::String) = x; " "server = LanguageServer.LanguageServerInstance(" "stdin, stdout, " (lsp-julia--get-root-remote) ", " (lsp-julia--get-depot-path) ", " "nothing, " (lsp-julia--symbol-server-store-path-to-jl-no-expand) "); " "run(server);" "'"))) (lsp-register-client (make-lsp-client :new-connection (lsp-tramp-connection 'lsp-julia--rls-command-remote) :major-modes '(julia-mode ess-julia-mode) :server-id 'julia-ls-remote ;;:multi-root t :remote? t))) ```

This setup pataches a recent PR in lsp-mode. Along with this, you have to upgrade TRAMP to 2.5.0. The setup for the lsp-julia is straightfoward -- only need to make sure all pathes are the remote ones.

You open a file from a julia project on the remote server and want the language server to spawn that same server?

Exactly. My config above does this.

Is it possible to have a local language server work on the remote files?

I don't know but I suppose this would be hard as the dependencies are on the remote.

lsp-tramp-connection is provided by lsp-mode, right?

Yes :)

tecosaur commented 2 years ago

@xukai92 I've just come across this thread after looking for a way to get a remote lsp server working. Do you think your work could be turned into a PR?

ordicker commented 1 year ago

Making lsp-julia works remotely won't be great.

gdkrmr commented 1 year ago

I have tried #61 and it doesn't work locally. I think emacs calls julia --project=... is a single command. I have also tried creating a single string and calling that with the same result.

I have also tried implementing the solution by @xukai92 and get the same error on the remote machine. maybe this is a path issue, I didn't have time to check.

uncomfyhalomacro commented 1 year ago

I just saw this issue. I made a script for this instead to fix some corner cases https://codeberg.org/uncomfyhalomacro/erudite-macs/src/branch/main/scripts. and to use that, i made a this huge elisp code

(with-eval-after-load "lsp-julia"
      (add-to-list 'lsp-language-id-configuration '(julia-ts-mode . "julia"))
      (setq
       lsp-julia-command "julia"
       lsp-julia-package-dir "@emacs-lspconfig"
       lsp-julia-flags `(,(concat "--project=" lsp-julia-package-dir)
                         "--startup-file=no"
                         "--history-file=no"
                         ,(concat "-J" (shell-command-to-string "julia --startup-file=no --history-file=no -e 'print(homedir())'") "/.julia/environments/emacs-lspconfig/languageserver.so"))
       lsp-julia-default-environment (string-trim(shell-command-to-string "julia --startup-file=no --history-file=no -e 'print(dirname(Base.active_project()))'")))

      (defun lsp-julia-update-languageserver()
        "Custom command to update the Julia Language Server."
        (interactive)
        (setq lsp-julia-update-flags `(,(concat "--project=" lsp-julia-package-dir)
                                       "--startup-file=no"
                                       "--history-file=no"))
        (apply 'start-process
               "lsp-julia-languageserver-updater"
               "*lsp-julia-languageserver-updater*"
               lsp-julia-command
               (append lsp-julia-update-flags
                       (list (expand-file-name "scripts/julia-ls-update-or-install.jl" user-emacs-directory)))))

      (defun lsp-julia--get-root ()
        "Get the (Julia) project root directory of the current file. Customized."
        (concat ""
                (expand-file-name
                 (or (locate-dominating-file buffer-file-name "Project.toml")
                     (locate-dominating-file buffer-file-name "JuliaProject.toml")
                     lsp-julia-default-environment))
                ""))

      (defvar julia-ls-script "" "Path to Julia Script")

      (setq julia-ls-script
            (expand-file-name "scripts/julia-ls.jl" user-emacs-directory))

      (defun lsp-julia--rls-command ()
        "This is a custom command to launch the Julia Language Server.
                   The reason being that I am using Julia inside a distrobox."
        `(,lsp-julia-command
          ,@lsp-julia-flags
          ,julia-ls-script
          ,(buffer-file-name)))
      (lsp-register-client
       (make-lsp-client :new-connection (lsp-tramp-connection 'lsp-julia--rls-command)
                        :activation-fn (lsp-activate-on "julia")
                        :major-modes '(julia-mode julia-ts-mode)
                        :remote? t
                        :priority '1
                        :server-id 'lsp-julia-remote))
      (lsp-register-client
       (make-lsp-client :new-connection (lsp-stdio-connection 'lsp-julia--rls-command)
                        :major-modes '(julia-mode julia-ts-mode)
                        :server-id 'julia-ls
                        :multi-root t))
      ))