From 22b6eaed0349b23f4c9fe02adc5f4f50b80fa9eb Mon Sep 17 00:00:00 2001 From: Henrik Lissner Date: Sun, 11 Oct 2020 18:13:59 -0400 Subject: [PATCH] Rethink lsp defaults + Allow LSP to prompt to install servers. All this machinary just adds more confusion for beginners, and at least LSP asks for your permission before it does it. + Reverts lsp-enable-file-watchers and lsp-enable-indentation to their default (enabled), hopefully to help lsp-java, lsp-dart, and lsp-clojure users, for whom file-watchers seems to be necessary. + Apply GC/IPC optimizations globally, to ensure their reach. By only setting them buffer-locally we don't have a guarantee that subprocesses will be affected when the lsp buffer isn't focused. Closes #3989 Co-authored-by: Eric Dallo --- modules/tools/lsp/+eglot.el | 8 ++-- modules/tools/lsp/+lsp.el | 73 +++++++----------------------------- modules/tools/lsp/README.org | 21 ++++------- modules/tools/lsp/config.el | 39 ++++++++++++------- 4 files changed, 51 insertions(+), 90 deletions(-) diff --git a/modules/tools/lsp/+eglot.el b/modules/tools/lsp/+eglot.el index 8e3fdcaac..aaa5f285f 100644 --- a/modules/tools/lsp/+eglot.el +++ b/modules/tools/lsp/+eglot.el @@ -2,7 +2,7 @@ (use-package! eglot :commands eglot eglot-ensure - :hook (eglot-managed-mode . +lsp-init-optimizations-h) + :hook (eglot-managed-mode . +lsp-optimization-mode) :init (setq eglot-sync-connect 1 eglot-connect-timeout 10 @@ -34,11 +34,13 @@ server getting expensively restarted when reverting buffers." (letf! (defun eglot-shutdown (server) (if (or (null +lsp-defer-shutdown) (eq +lsp-defer-shutdown 0)) - (funcall eglot-shutdown server) + (prog1 (funcall eglot-shutdown server) + (+lsp-optimization-mode -1)) (run-at-time (if (numberp +lsp-defer-shutdown) +lsp-defer-shutdown 3) nil (lambda (server) (unless (eglot--managed-buffers server) - (funcall eglot-shutdown server))) + (prog1 (funcall eglot-shutdown server) + (+lsp-optimization-mode -1)))) server))) (funcall orig-fn server)))) diff --git a/modules/tools/lsp/+lsp.el b/modules/tools/lsp/+lsp.el index a9f98e379..30aa1af64 100644 --- a/modules/tools/lsp/+lsp.el +++ b/modules/tools/lsp/+lsp.el @@ -4,13 +4,6 @@ "The backends to prepend to `company-backends' in `lsp-mode' buffers. Can be a list of backends; accepts any value `company-backends' accepts.") -(defvar +lsp-auto-install-servers nil - "If non-nil, automatically install LSP servers. - -`lsp-mode' will prompt you to install an LSP server when you enter a major mode -with LSP support but no available server. I believe changes to your system -should be a deliberate act (as is flipping this variable).") - ;; ;;; Packages @@ -18,30 +11,27 @@ should be a deliberate act (as is flipping this variable).") (use-package! lsp-mode :commands lsp-install-server :init - (setq lsp-session-file (concat doom-etc-dir "lsp-session")) - ;; For `lsp-clients' - (setq lsp-server-install-dir (concat doom-etc-dir "lsp/")) + ;; Don't touch ~/.emacs.d, which could be purged without warning + (setq lsp-session-file (concat doom-etc-dir "lsp-session") + lsp-server-install-dir (concat doom-etc-dir "lsp/")) ;; Don't auto-kill LSP server after last workspace buffer is killed, because I ;; will do it for you, after `+lsp-defer-shutdown' seconds. (setq lsp-keep-workspace-alive nil) - ;; Let doom bind the lsp keymap. - (when (featurep! :config default +bindings) - (setq lsp-keymap-prefix nil)) - ;; NOTE I tweak LSP's defaults in order to make its more expensive or imposing ;; features opt-in. Some servers implement these poorly and, in most ;; cases, it's safer to rely on Emacs' native mechanisms (eldoc vs ;; lsp-ui-doc, open in popup vs sideline, etc). ;; Disable features that have great potential to be slow. - (setq lsp-enable-file-watchers nil - lsp-enable-folding nil + (setq lsp-enable-folding nil lsp-enable-text-document-color nil) + ;; Reduce unexpected modifications to code + (setq lsp-enable-on-type-formatting nil) - ;; Disable features that modify our code without our permission. - (setq lsp-enable-indentation nil - lsp-enable-on-type-formatting nil) + ;; Let doom bind the lsp keymap. + (when (featurep! :config default +bindings) + (setq lsp-keymap-prefix nil)) :config (pushnew! doom-debug-variables 'lsp-log-io 'lsp-print-performance) @@ -62,43 +52,6 @@ should be a deliberate act (as is flipping this variable).") :type-definition #'lsp-find-type-definition :references #'lsp-find-references) - (when lsp-auto-configure - (mapc (lambda (package) (require package nil t)) - (cl-remove-if #'featurep lsp-client-packages))) - - (defadvice! +lsp--dont-auto-install-servers-a (orig-fn &rest args) - "Replace auto-install behavior with warning and support indirect buffers." - :around #'lsp - (if +lsp-auto-install-servers - (apply orig-fn args) - (letf! ((buffer-file-name - ;; Add support for indirect buffers (org src or capture buffers) - (or buffer-file-name - (buffer-file-name (buffer-base-buffer)))) - ;; Already loaded them. No need to do so again. - (lsp-client-packages nil) - ;; `lsp' is normally eager to automatically install LSP servers, or - ;; prompting to do so, but (in my opinion) server installation - ;; should be a deliberate act by the end-user: - (defun lsp--completing-read (msg clients &rest _) - (lsp--warn (concat msg "%s\n" - " Use `M-x lsp-install-server' to install a supported server, or read " - "https://emacs-lsp.github.io/lsp-mode/page/languages for more.") - (mapcar #'lsp--client-server-id clients)) - (throw 'not-installed nil))) - (catch 'not-installed - (apply orig-fn args))))) - - (defadvice! +lsp--respect-user-defined-checkers-a (orig-fn &rest args) - "Set up flycheck-mode or flymake-mode, depending on `lsp-diagnostic-package'." - :around #'lsp-diagnostics--flycheck-enable - (if flycheck-checker - ;; Respect file/dir/explicit user-defined `flycheck-checker'. - (let ((old-checker flycheck-checker)) - (apply orig-fn args) - (setq-local flycheck-checker old-checker)) - (apply orig-fn args))) - (add-hook! 'lsp-mode-hook (defun +lsp-display-guessed-project-root-h () "Log what LSP things is the root of the current project." @@ -107,7 +60,7 @@ should be a deliberate act (as is flipping this variable).") (if-let (root (lsp--calculate-root (lsp-session) path)) (lsp--info "Guessed project root is %s" (abbreviate-file-name root)) (lsp--info "Could not guess project root.")))) - #'+lsp-init-optimizations-h) + #'+lsp-optimization-mode) (add-hook! 'lsp-completion-mode-hook (defun +lsp-init-company-backends-h () @@ -128,7 +81,8 @@ server getting expensively restarted when reverting buffers." restart (null +lsp-defer-shutdown) (= +lsp-defer-shutdown 0)) - (funcall orig-fn restart) + (prog1 (funcall orig-fn restart) + (+lsp-optimization-mode -1)) (when (timerp +lsp--deferred-shutdown-timer) (cancel-timer +lsp--deferred-shutdown-timer)) (setq +lsp--deferred-shutdown-timer @@ -137,7 +91,8 @@ server getting expensively restarted when reverting buffers." nil (lambda (workspace) (let ((lsp--cur-workspace workspace)) (unless (lsp--workspace-buffers lsp--cur-workspace) - (funcall orig-fn)))) + (funcall orig-fn) + (+lsp-optimization-mode -1)))) lsp--cur-workspace))))) diff --git a/modules/tools/lsp/README.org b/modules/tools/lsp/README.org index a79da7a74..c6c92af3d 100644 --- a/modules/tools/lsp/README.org +++ b/modules/tools/lsp/README.org @@ -7,7 +7,6 @@ - [[#description][Description]] - [[#module-flags][Module Flags]] - [[#plugins][Plugins]] - - [[#hacks][Hacks]] - [[#prerequisites][Prerequisites]] - [[#features][Features]] - [[#lsp-powered-project-search][LSP-powered project search]] @@ -70,23 +69,17 @@ As of this writing, this is the state of LSP support in Doom Emacs: + [[https://github.com/emacs-lsp/helm-lsp][helm-lsp]] + [[https://github.com/joaotavora/eglot][eglot]] -** Hacks -+ ~lsp-mode~ has been modified not to automatically install missing LSP servers. - This is done to adhere to our "Your system, your rules" mantra, which insist - that it is better etiquette to let the user decide when their development - environment is modified. Use ~M-x lsp-install-server~ to install LSP servers - manually. - * Prerequisites -This module has no direct prerequisites, but major-modes require you to install -language servers. +This module has no direct prerequisites, but different languages will need +different language servers, which =lsp-mode= will prompt you to auto-install. +=eglot= will not. -You'll find a table that lists available language servers and how to install -them [[https://github.com/emacs-lsp/lsp-mode#supported-languages][in the lsp-mode project README]]. The documentation of the module for your +A table that lists available language servers and how to install them can be +found [[https://emacs-lsp.github.io/lsp-mode/page/languages/][on the lsp-mode project README]]. The documentation of the module for your targeted language will contain brief instructions as well. -For eglot users, you can see the list of [[https://github.com/joaotavora/eglot/blob/master/README.md#connecting-to-a-server][default servers supported in the README]]. -There is also instructions to add another server easily. +For eglot users, a list of [[https://github.com/joaotavora/eglot/blob/master/README.md#connecting-to-a-server][default servers supported is on Eglot's README]], +including instructions to register your own. * TODO Features ** LSP-powered project search diff --git a/modules/tools/lsp/config.el b/modules/tools/lsp/config.el index d2df9922c..8831e1fd4 100644 --- a/modules/tools/lsp/config.el +++ b/modules/tools/lsp/config.el @@ -12,20 +12,31 @@ killing and opening many LSP/eglot-powered buffers.") ;; ;;; Common -(defun +lsp-init-optimizations-h () - "Deploys universal optimizations for `lsp-mode' and `eglot'." - (when (or (bound-and-true-p eglot--managed-mode) - (bound-and-true-p lsp-mode)) - ;; `read-process-output-max' is only available on recent development - ;; builds of Emacs 27 and above. - (setq-local read-process-output-max (* 1024 1024)) - ;; REVIEW LSP causes a lot of allocations, with or without Emacs 27+'s - ;; native JSON library, so we up the GC threshold to stave off - ;; GC-induced slowdowns/freezes. Doom uses `gcmh' to enforce its GC - ;; strategy, so we modify its variables rather than - ;; `gc-cons-threshold' directly. - (setq-local gcmh-high-cons-threshold (* 2 (default-value 'gcmh-high-cons-threshold))) - (gcmh-set-high-threshold))) +(defvar +lsp--default-read-process-output-max nil) +(defvar +lsp--default-gcmh-high-cons-threshold nil) + +(define-minor-mode +lsp-optimization-mode + "Deploys universal GC and IPC optimizations for `lsp-mode' and `eglot'." + :global t + :init-value nil + (if +lsp-optimization-mode + (progn + (setq +lsp--default-read-process-output-max + (default-value 'read-process-output-max) + +lsp--default-gcmh-high-cons-threshold + (default-value 'gcmh-high-cons-threshold)) + ;; `read-process-output-max' is only available on recent development + ;; builds of Emacs 27 and above. + (setq-default read-process-output-max (* 1024 1024)) + ;; REVIEW LSP causes a lot of allocations, with or without Emacs 27+'s + ;; native JSON library, so we up the GC threshold to stave off + ;; GC-induced slowdowns/freezes. Doom uses `gcmh' to enforce its + ;; GC strategy, so we modify its variables rather than + ;; `gc-cons-threshold' directly. + (setq-default gcmh-high-cons-threshold (* 2 +lsp--default-gcmh-high-cons-threshold))) + (setq-default read-process-output-max +lsp--default-read-process-output-max + gcmh-high-cons-threshold +lsp--default-gcmh-high-cons-threshold)) + (gcmh-set-high-threshold)) ;;