Skip to content

Commit

Permalink
Close #29: Implement workspace/didChangeConfiguration (#40)
Browse files Browse the repository at this point in the history
* README.md (Supported Protocol Features, Commands and
keybindings): mention workspace/didChangeConfiguration.

* eglot.el (eglot-server-initialized-hook): New hook.
(eglot--connect): Run it.
(eglot-workspace-configuration): New variable.
(eglot-signal-didChangeConfiguration): New command.
  • Loading branch information
joaotavora authored Jul 11, 2018
1 parent 5d88eed commit a57d5d8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ Here's a summary of available commands:
- `M-x eglot-stderr-buffer` if the LSP server is printing useful debug
information in stderr, jumps to a buffer with these contents.

- `M-x eglot-signal-didChangeConfiguration` updates the LSP server
configuration according to the value of the variable
`eglot-workspace-configuration`, which you may, for example set in a
`.dir-locals` file.`

There are *no keybindings* specific to Eglot, but you can bind stuff
in `eglot-mode-map`, which is active as long as Eglot is managing a
file in your project. The commands don't need to be Eglot-specific,
Expand Down Expand Up @@ -163,7 +168,7 @@ eglot-shutdown`.
## Workspace
- [ ] workspace/workspaceFolders (3.6.0)
- [ ] workspace/didChangeWorkspaceFolders (3.6.0)
- [ ] workspace/didChangeConfiguration
- [x] workspace/didChangeConfiguration
- [ ] workspace/configuration (3.6.0)
- [x] workspace/didChangeWatchedFiles
- [x] workspace/symbol
Expand Down
22 changes: 22 additions & 0 deletions eglot.el
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,11 @@ INTERACTIVE is t if called interactively."

(defvar eglot-connect-hook nil "Hook run after connecting in `eglot--connect'.")

(defvar eglot-server-initialized-hook
'(eglot-signal-didChangeConfiguration)
"Hook run after server is successfully initialized.
Each function is passed the server as an argument")

(defun eglot--connect (managed-major-mode project class contact)
"Connect to MANAGED-MAJOR-MODE, PROJECT, CLASS and CONTACT.
This docstring appeases checkdoc, that's all."
Expand Down Expand Up @@ -514,6 +519,7 @@ This docstring appeases checkdoc, that's all."
(with-current-buffer buffer
(eglot--maybe-activate-editing-mode server)))
(jsonrpc-notify server :initialized `(:__dummy__ t))
(run-hook-with-args 'eglot-server-initialized-hook server)
(setf (eglot--inhibit-autoreconnect server)
(cond
((booleanp eglot-autoreconnect) (not eglot-autoreconnect))
Expand Down Expand Up @@ -1033,6 +1039,22 @@ Records START, END and PRE-CHANGE-LENGTH locally."
(eglot--signal-textDocument/didChange))))
'((name . eglot--signal-textDocument/didChange)))

(defvar-local eglot-workspace-configuration ()
"Alist of (SETTING . VALUE) entries configuring the LSP server.
Setting should be a keyword, value can be any value that can be
converted to JSON.")

(defun eglot-signal-didChangeConfiguration (server)
"Send a `:workspace/didChangeConfiguration' signal to SERVER.
When called interactively, use the currently active server"
(interactive (list (eglot--current-server-or-lose)))
(jsonrpc-notify
server :workspace/didChangeConfiguration
(list
:settings
(cl-loop for (k . v) in eglot-workspace-configuration

This comment has been minimized.

Copy link
@astoff

astoff Jun 27, 2022

Contributor

I wonder what the point of this loop is, instead of just passing eglot-workspace-configuration directly. Typically, the workspace configuration is a deeply nested JSON object. So the end effect of this special treatment for the outer-level keys is that eglot-workspace-configuration needs to be a weird mix of alists (with keyword keys...) and plists.

This comment has been minimized.

Copy link
@joaotavora

joaotavora Jun 27, 2022

Author Owner

needs to be a weird mix of alists (with keyword keys...) and plists.

Yes, that's true. In hindsight, this wasn't a very good design decision, but it is one which is hard to roll back.
I don't have much of a defense of why I did it like this in the beginning. I've never had need for eglot-workspace-configuration settings in my own projects, either because they are typically small or because I tend to choose servers and project layouts where the defaults make sense. So the "Typically, the workspace configuration is a deeply nested JSON object" wasn't an evidence to me. But it's clear that it is the 99% de facto reality.

We should probably introduce a eglot-workspace-configuration-plist (or maybe simply eglot-workspace-plist) and deprecate the old one. But merge both, of course. Give it a good docstring with real-life examples while we're at it. If you want to take on this PR, I'd be happy to merge it.

This comment has been minimized.

Copy link
@astoff

astoff Jun 27, 2022

Contributor

I think this can be changed backwards-compatibly, in the same way json-serialize handles plists or alists in a smart way. I'll have a look.

collect k collect v))))

(defun eglot--signal-textDocument/didChange ()
"Send textDocument/didChange to server."
(when eglot--recent-changes
Expand Down

1 comment on commit a57d5d8

@joaotavora
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

I'm afraid I don't know what to make of this, @jixiuf

Please sign in to comment.