search

emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.75k stars 873 forks source link

lsp-find-session-folder seems to return the wrong output (at least when lsp-find-workspace uses it) #2610

Open ag91 opened 3 years ago

ag91 commented 3 years ago

Hi, thank you very much for this mode! I am starting appreciating it just now and I am really happy with my Scala development!

I was trying to analyze the stacktrace in a sbt-mode buffer, and for some weird reasons the lsp-session was not recognized.

The issue is lsp-find-session-folder because the current implementation is:

(defun lsp-find-session-folder (session file-name)
  "Look in the current SESSION for folder containing FILE-NAME."
  (let ((file-name-canonical (lsp-f-canonical file-name)))
    (->> session
         (lsp-session-folders)
         (--filter (and (lsp--files-same-host it file-name-canonical)
                        (or (lsp-f-same? it file-name-canonical)
                            (and (f-dir? it)
                                 (lsp-f-ancestor-of? it file-name-canonical)))))
         (--max-by (> (length it)
                      (length other))))))

In my use case, (lsp-session-folders) returns a list containing both "/my-dir" and "/my-dir/". The last --max-by will pick the second. The problem arises with lsp-find-workspace, the current implementation:

(defun lsp-find-workspace (server-id &optional file-name)
  "Find workspace for SERVER-ID for FILE-NAME."
  (-when-let* ((session (lsp-session))
               (folder->servers (lsp-session-folder->servers session))
               (workspaces (if file-name
                               (gethash (lsp-find-session-folder session file-name) folder->servers)
                             (lsp--session-workspaces session))))

    (--first (eq (lsp--client-server-id (lsp--workspace-client it)) server-id) workspaces)))

The bit (gethash (lsp-find-session-folder session file-name) folder->servers) works only for /my-dir and NOT for /my-dir/. I guess this bug escaped you because the file-name field is optional and probably not very used. I can open a PR to fix it, but I would need the maintainers wisdom to understand if it is better fixing that in lsp-find-workspace because maybe changing lsp-find-session-folder may break other things.

Again thanks for the amazing work!

yyoncho commented 3 years ago

In my use case, (lsp-session-folders) returns a list containing both "/my-dir" and "/my-dir/". The last --max-by will pick the second.

This should not happen. Can you provide steps to reproduce?

ag91 commented 3 years ago

Mmm, okay. On my system the way I can reproduce this is by cloning a repository in the /tmp/ directory. At that point (lsp-find-session-folder (lsp-session) "build.sbt") gives me "/tmp/", which it is strange in itself. The other projects I have that cause the issue are old: I have been using lsp-metals on them from a long time and maybe before the directory was stored with the final "/"? Maybe I can try to reset those projects: how would I do that?