Package: emacs;
Reported by: Augusto Stoffel <arstoffel <at> gmail.com>
Date: Tue, 28 Feb 2023 12:39:02 UTC
Severity: normal
Found in version 29.0.60
Fixed in version 29.1
Done: Augusto Stoffel <arstoffel <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: João Távora <joaotavora <at> gmail.com> To: Augusto Stoffel <arstoffel <at> gmail.com> Cc: 61866 <at> debbugs.gnu.org Subject: bug#61866: 29.0.60; Eglot: Dir-local workspace config doesn't work as described Date: Tue, 28 Feb 2023 19:20:35 +0000
Augusto Stoffel <arstoffel <at> gmail.com> writes:
> Quoting from the manual:
>
>> Here’s an example of defining the workspace-configuration settings [...]
>>
>> ((python-mode
>> . ((eglot-workspace-configuration
>> . (:pylsp (:plugins (:jedi_completion (:include_params t
>> :fuzzy t)
>> :pylint (:enabled :json-false)))))))
>> (go-mode
>> . ((eglot-workspace-configuration
>> . (:gopls (:usePlaceholders t))))))
>
> The above doesn't work.
I've just tried it and it works fine. Here's what I did: I put that
example in a .dir-locals.el nearby a thingy.py and thingy.go file in the
same dir. I start two Eglots managing each file and inspect that the
correct config is sent in each case.
> => *temp* ~/project_dir/
Have you tried adding a trace of the 'major-mode' variable to that
'message' call? It should produce the correct major mode.
> Presumably, the temp buffer is in fundamental mode.
This is why we set major-mode and call
hack-dir-local-variables-non-file-buffer.
You could write a recipe that demonstrating that it fails, but read on
as I have a patch to present to fix another separate thing you reported
here.
> Note also that if you later do `M-x
> eglot-signal-didChangeConfiguration RET', then the that buffer's local
> value of `eglot-workspace-configuration' is used, since:
>
> => actual_file.py ~/project_dir/subdir
This is how it should work. A buffer in python-mode should report its
directory-local values.
> [ This behavior is confusing and reinforces my opinion that server
> information should be kept away from buffer-local variables.
I'm not sure I see the benefit. This would cache things and necessitate
some invalidation when the user changes the .dir-locals.el file.
> Related glitch: if you have two servers running, A and B, and then,
> from a buffer in project A you do
> M-x eglot-show-workspace-configuration RET
> and select server B in the prompt, you get server A's information. ]
I've reproduced this, and indeed this was quite broken. The patch at
the end should fix it, but I'd like you to test it.
> There's probably no better place than dir-locals to store "workspace"
> (aka project) configuration. But it would be better to then record this
> configuration in the server object after it's read for .dir-locals.el,
.dir-locals.el is short for "dir-local variables". buffer-local
variables are an integral part to that mechanism. If we add a new slot
in Eglot's server object we're adding caching/duplication/entangling
(however you prefer to call this). Sometimes that is needed, for
example for performance reasons, but it comes with invalitation
challenges.
Please try this patch:
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index ffc9511469f..89cdd8c14c7 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1335,10 +1335,7 @@ eglot--connect
(lambda ()
(setf (eglot--inhibit-autoreconnect server)
(null eglot-autoreconnect)))))))
- (let ((default-directory (project-root project))
- (major-mode (car managed-modes)))
- (hack-dir-local-variables-non-file-buffer)
- (run-hook-with-args 'eglot-connect-hook server))
+ (run-hook-with-args 'eglot-connect-hook server)
(eglot--message
"Connected! Server `%s' now managing `%s' buffers \
in project `%s'."
@@ -2444,9 +2441,7 @@ eglot-workspace-configuration
(defun eglot-show-workspace-configuration (&optional server)
"Dump `eglot-workspace-configuration' as JSON for debugging."
- (interactive (list (and (eglot-current-server)
- (eglot--read-server "Server configuration"
- (eglot-current-server)))))
+ (interactive (list (eglot--read-server "Show workspace configuration for" t)))
(let ((conf (eglot--workspace-configuration-plist server)))
(with-current-buffer (get-buffer-create "*EGLOT workspace configuration*")
(erase-buffer)
@@ -2457,14 +2452,23 @@ eglot-show-workspace-configuration
(json-pretty-print-buffer))
(pop-to-buffer (current-buffer)))))
-(defun eglot--workspace-configuration (server)
- (if (functionp eglot-workspace-configuration)
- (funcall eglot-workspace-configuration server)
- eglot-workspace-configuration))
-
-(defun eglot--workspace-configuration-plist (server)
- "Returns `eglot-workspace-configuration' suitable for serialization."
- (let ((val (eglot--workspace-configuration server)))
+(defun eglot--workspace-configuration-plist (server &optional path)
+ "Returns SERVER's workspace configuration as a plist.
+If PATH consider that file's `file-name-directory' to get the
+local value of the `eglot-workspace-configuration' variable, else
+use the root of SERVER's `eglot--project'."
+ (let ((val (with-temp-buffer
+ (setq default-directory
+ (if path
+ (file-name-directory path)
+ (project-root (eglot--project server))))
+ ;; FIXME; Should probably join values of all managed
+ ;; major mode of this server.
+ (setq major-mode (car (eglot--major-modes server)))
+ (hack-dir-local-variables-non-file-buffer)()
+ (if (functionp eglot-workspace-configuration)
+ (funcall eglot-workspace-configuration server)
+ eglot-workspace-configuration))))
(or (and (consp (car val))
(cl-loop for (section . v) in val
collect (if (keywordp section) section
@@ -2489,25 +2493,18 @@ eglot-handle-request
(apply #'vector
(mapcar
(eglot--lambda ((ConfigurationItem) scopeUri section)
- (with-temp-buffer
- (let* ((uri-path (eglot--uri-to-path scopeUri))
- (default-directory
- (if (and uri-path
- (not (string-empty-p uri-path))
- (file-directory-p uri-path))
- (file-name-as-directory uri-path)
- (project-root (eglot--project server)))))
- (setq-local major-mode (car (eglot--major-modes server)))
- (hack-dir-local-variables-non-file-buffer)
- (cl-loop for (wsection o)
- on (eglot--workspace-configuration-plist server)
- by #'cddr
- when (string=
- (if (keywordp wsection)
- (substring (symbol-name wsection) 1)
- wsection)
- section)
- return o))))
+ (cl-loop with scope-uri-path =
+ (and scopeUri (eglot--uri-to-path scopeUri))
+ for (wsection o)
+ on (eglot--workspace-configuration-plist server
+ scope-uri-path)
+ by #'cddr
+ when (string=
+ (if (keywordp wsection)
+ (substring (symbol-name wsection) 1)
+ wsection)
+ section)
+ return o))
items)))
(defun eglot--signal-textDocument/didChange ()
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.