Support on-demand workspace/didChangeWorkspaceFolders notification after initialization

[wyuenho]

Describe the bug
Currently, due to persistent session and the LSP initialize message signature, all the workspace folders ever created by LSP for all the projects that a multi-root LSP server activates on, e.g. eslint, will be sent to the server for indexing when that server is initialized. Furthermore, if the server supports file watches, (eslint again), and some project consists of thousands of files, this server start up process will consume too much memory unnecessarily, slow down start up, and prompt the user for file watch threshold reached.

To Reproduce

1. Configure lsp-mode by adding lsp-deferred to js-mode-hook
2. Configure lsp-eslint per the wiki page
3. Delete the .lsp-session-v1 file
4. Open some Typescript or Javascript repos with eslint configured
5. M-x lsp-describe-session shows the eslint process is running for all of these JS repos
6. Restart emacs
7. Open a JS file in any of repos
8. M-x lsp-describe-session shows eslint is running for all of the repos, not just the repo of the opened file.

Expected behavior
LSP should not notify multi-root servers the full list of workspace folders on initialization. When a file belonging to a different workspace folder is opened, it is only then should LSP notify the multi-root server applicable with the workspace/didChangeWorkspaceFolders message.

Which Language Server did you use
lsp-eslint

[yyoncho]

The following piece of code should be sufficient to achieve what you want:

  (with-eval-after-load 'lsp-mode
    (eval '(setf (lsp-session-server-id->folders (lsp-session)) (ht))))

[wyuenho]

It doesn't. I set lsp-keep-workspace-alive to nil. I can visit 10 different JS files from 10 different repos, kill all the JS buffers, and then the next one opened will trigger indexing all 10 repos again.

[yyoncho]

the proposed code works between Emacs restarts, here it is the code to make that work in single session:

(advice-add 'lsp :before (lambda (&rest _args) (setf (lsp-session-server-id->folders (lsp-session)) (ht))))

[wyuenho]

I get this in Messages

let*: Symbol’s function definition is void: \(setf\ lsp-session-server-id->folders\)

But even if it did work, it's not what I ask for, because this effectively wipes out the session folders every time I visit a file. What I'm asking is have the multi-root LSP servers to index incrementally and on demand instead of indexing everything or nothing at all.

[yyoncho]

No, it won't work like that. The effect of this code is that lsp mode will always start the language server with only the current workspace folder. Then when you open a file from another project it will be added for indexing at that point.

Something like that will be able to bypass the error(not sure what is the best way to avoid that error though).

(advice-add 'lsp :before (lambda (&rest _args) (eval '(setf (lsp-session-server-id->folders (lsp-session)) (ht)))))

[wyuenho]

This just gets me this error after visiting a second file in a different folder, despite lsp-describe-session shows the server is clearly running.

LSP :: Sending to process failed with the following error: Process eslint not running

[yyoncho]

This is odd. I will have to setup eslint + typescript ls to try to reproduce it. I have tested the path with jdtls which is also multiroot server and works fine but it is standalone server.

[wyuenho]

It doesn't alway give me that error. As I've said, I have lsp-keep-workspace-alive set to nil. After closing all the JS buffers, the workspace kills all servers, but then after opening 2 JS files from 2 repos, the second file will give me that error.

[yyoncho]

I think that your issue was fixed by 7b0b9c3 . Seems like we were not cleaning up the buffers correctly when you kill the server.

[leungbk]

Yeah, this problem doesn't appear for me anymore.

Never mind, I still see it.

[yyoncho]

@leungbk do you have steps to reproduce?

[leungbk]

@leungbk do you have steps to reproduce?

Same procedure as OP, basically.

I cloned vscode-eslint, and then made a separate git project in with nothing but a .eslintrc.base.json. Then I deleted .lsp-session-v1 and exited Emacs.

I visited for the first time the above projects using the below init, with ESLint starting up in each of them. Then I restarted Emacs. Once I opened .eslintrc.base.json in a single repo, I did M-x lsp-describe-session and saw that the ESLint server was running for both repos.

Your workaround posted above works for me, though.

(let ((bootstrap-file (concat user-emacs-directory "straight/repos/straight.el/bootstrap.el"))
      (bootstrap-version 5))
  (unless (file-exists-p bootstrap-file)
    (with-current-buffer
        (url-retrieve-synchronously
         "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
         'silent 'inhibit-cookies)
      (goto-char (point-max))
      (eval-print-last-sexp)))
  (load bootstrap-file nil 'nomessage))

(straight-use-package 'use-package)
(setq straight-use-package-by-default t)

(setq debug-on-error t)

(setq read-process-output-max (* 1024 1024 3))

(use-package company
  :config
  (global-company-mode 1)
  (setq company-show-numbers t))

(use-package lsp-mode
  :hook (((
           js-mode
           typescript-mode) . lsp)
         )
  :init
  (setq lsp-prefer-flymake nil)
  (setq lsp-prefer-capf t)

  (setq
   lsp-eslint-server-command
   '("node"
     "/home/brian/.vscode/extensions/dbaeumer.vscode-eslint-2.1.5/server/out/eslintServer.js"
     "--stdio"))

  :config
  (setq lsp-log-io t))

(use-package projectile
  :config
  (projectile-mode 1))

(use-package yasnippet
  :config
  (yas-global-mode 1))

(require 'dired-x)

(use-package typescript-mode)

(use-package flycheck)

[wyuenho]

I don't understand why we are spending so much time on hacks and documentation when we could just support delayed workspace/didChangeWorkspaceFolders notifications to the server. Is that even in the works?

[kiennq]

This is actually really useful especially when Emacs is running as daemon (so basically only one big session).
Would you like to provide a PR for this?

[wyuenho]

I'd love to, but all the anaphoric macros in the code gives me a headache. Someone more familiar with the code base should do it.

[yyoncho]

The issue with restart was sorted out and it was not related to the suggested fix. I am marking this as closed(refer to the FAQ section on how to achieve that behavior). Please reopen or file another issue if you hit another issue related to the provided snippet.

[wyuenho]

And... now lsp-keep-workspace-alive is broken, I'll file another issue.

[raxod502]

I am confused, why is this advice not the default behavior? In what situation is the default behavior desired?