Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom desktop minor mode handler for lsp-deferred #3975

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LaurenceWarne
Copy link
Contributor

Hi, I've been encountering some issues with lsp servers starting with desktop-save-mode despite me using lsp-deferred . I've been able to reproduce it minimally with configuration:

(desktop-save-mode 1)

(require 'package)
(let* ((no-ssl (and (memq system-type '(windows-nt ms-dos))
                    (not (gnutls-available-p))))
       (proto (if no-ssl "http" "https")))
  (add-to-list 'package-archives (cons "melpa" (concat proto "://melpa.org/packages/")) t))

(package-initialize)

(unless (package-installed-p 'use-package)
  (package-refresh-contents)
  (package-install 'use-package)
  (eval-when-compile (require 'use-package)))

(setq use-package-always-ensure t)

(use-package lsp-mode
  :init
  ;; set prefix for lsp-command-keymap (few alternatives - "C-l", "C-c l")
  (setq lsp-keymap-prefix "C-c l")
  :hook (;; replace XXX-mode with concrete major-mode(e. g. python-mode)
         (python-mode . lsp-deferred))

Then perform the following:

  1. Open a python file, so that an lsp server starts
  2. Switch back to scratch
  3. Exit Emacs, in your desktop file lsp-mode should be in the buffer-minormodes section for the python file you opened
  4. Opening Emacs starts a python lsp server despite me starting now in a scratch buffer

The proposed fix is to add lsp--buffer-deferred to desktop-locals-to-save and add a custom handler to desktop-minor-mode-handlers which uses this persisted variable to determine whether to use lsp-deferred or lsp-mode when re-opening a file from the saved desktop. I've been using it for a while and haven't had any issues.

Let me know what you think, Thanks (also I'm on 28.1 and the most recent lsp-mode commit).

@yyoncho
Copy link
Member

yyoncho commented Feb 26, 2023

I don't like the top level require. I would rather go with either adding this to the FAQ, or using (with-eval-after-load 'desktop ... . If you make it FAQ entry - I will let it go right away. If you prefer to have it on by default I will have to spend a bit more time trying to figure out what the actual issue is(ATM it is bit unclear to me). Let me know what you think.

@LaurenceWarne
Copy link
Contributor Author

Yeah, with-eval-after-load should be used, I'll change that.

If you prefer to have it on by default I will have to spend a bit more time trying to figure out what the actual issue is(ATM it is bit unclear to me). Let me know what you think.

If you can reproduce the behaviour (is it expected?) I'd say the current behaviour is a bit counterintuitive 🙂 .

In terms of the issue, ff its helpful, my understanding is that once lsp-mode has been written to the minor mode list for a file in a desktop file, it's passed straight to desktop-create-buffer (when recreated) - at this point I'm not sure there's any discrimination as to if lsp-deferred was originally used or not.

Use lsp-deferred instead of lsp-mode even in the case that lsp-mode
appears in the list of minor modes for a file in a user's desktop file
by adding the buffer-local variable lsp--buffer-deferred (which tracks
whether a given buffer was started using lsp-deferred) to the variable
desktop-locals-to-save so that it is persisted in a user's desktop
file, and adding a custom handler to desktop-minor-mode-handlers which
uses this persisted variable to determine whether to use lsp-deferred
of lsp-mode when re-opening a file from the desktop file.
@LaurenceWarne
Copy link
Contributor Author

Possibly a simpler fix could be (add-to-list 'desktop-minor-mode-table '(lsp-mode nil)) so that desktop save mode will never try to restore lsp-mode.

@jo-so
Copy link

jo-so commented May 31, 2023

A better solution would be to restore lsp-mode by lsp-deferred.

  (defun lsp-deferred-restore-desktop-buffer (buf-locals)
    (lsp-deferred))

  (add-to-list 'desktop-minor-mode-handlers
               '(lsp-mode . lsp-deferred-restore-desktop-buffer))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants