rossabaker / lsp-scala

Scala support for lsp-mode using metals

GNU General Public License v3.0

49 stars 6 forks source link

Invoking lsp-scala fails looking for lsp-define-stdio-client #17

Closed binary132 closed 5 years ago

binary132 commented 5 years ago

I get the following error upon entering scala-mode, both using MELPA stable and MELPA: File mode specification error: (void-function lsp-define-stdio-client)

rossabaker commented 5 years ago

You mean both MELPA stable and MELPA unstable of lsp-mode?

JesusMtnez commented 5 years ago

Having the same issue here.

Using the latest versions from melpa seems to not be working. Using stable versions from melpa-stable seems to work well.

Any idea why? Maybe I could dig into it later

binary132 commented 5 years ago

@rossabaker -- both lsp-mode and lsp-scala, in all 4 combinations, iirc @JesusMtnez -- what versions of the Emacs lsp package, lsp-scala, and metaLS are you using?

SethTisue commented 5 years ago

I've been working around it by commenting out these lines in lsp-scala.el:

; Legacy support for lsp-mode <= 5
(lsp-define-stdio-client lsp-scala "scala"
                         (lambda () (sbt:find-root))
                         (lsp-scala--server-command))

JesusMtnez commented 5 years ago

I'm using metals 0.3.3 I got it working the old way, using lsp-mode and lsp-ui pin to melpa-stable versions. But doing so, you need to configure it like this:

(use-package lsp-mode
  :pin melpa-stable)

(use-package lsp-ui
  :pin melpa-stable
  :hook (lsp-mode . lsp-ui-mode))

(use-package lsp-scala
  :after scala-mode
  :demand t
  :load-path "local/lsp-scala/"
  :hook (scala-mode . lsp-scala-enable))

rossabaker commented 5 years ago

The addition of this package to MELPA is hung up on concerns about the with-eval-after-load call, which could go away. Maybe it's time to decide that we support either stable or unstable and stop trying to do both.

Does anyone have a strong preference for one over the other? Since metals itself is a bit bleeding edge, and pinning seems to be a non-default, I'm leaning toward dropping support for stable, which would look like @SethTisue's solution.

JesusMtnez commented 5 years ago

@rossabaker I agree. I think trying to maintain support for both right now does not seem to be a priority right now, since metals in a high WIP right now.

rossabaker commented 5 years ago

This should be fixed by #18. I'm going to look to follow up with removing the legacy support altogether to get rid of that with-eval-after-load call and hopefully get past the objection to getting this into MELPA.

rossabaker commented 5 years ago

19 is the more forceful solution. It works for me. Second set of eyes appreciated before I merge it.