bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[Augusto Stoffel]

Arranging for Eglot to send the correct configuration via the
`eglot-signal-didChangeConfiguration' command or
`eglot-workspace-configuration' variable is very tricky and error prone.

The following command, which reads a configuration plist from the
minibuffer with history, makes this trial and error process much easier
for me:

  (defvar eglot-edit-workspace-configuration--history nil)
  (defun eglot-edit-workspace-configuration ()
    (interactive)
    (let* ((server (eglot-current-server))
           (config (read-from-minibuffer
                    (format "New configuration for `%s': "
                            (eglot-project-nickname server))
                    (when-let ((old (eglot--workspace-configuration-plist server)))
                      (concat "\n" (pp-to-string old)))
                    minibuffer-local-map t
                    'eglot-edit-workspace-configuration--history)))
      (setq-local eglot-workspace-configuration config)
      (save-window-excursion
        (let ((default-directory (project-root (eglot--current-project))))
          (add-dir-local-variable nil 'eglot-workspace-configuration config)
          (save-buffer)))
      (eglot-signal-didChangeConfiguration server)))

I would suggest adding a refined version of this, using a regular buffer
for input.  Moreover, one could allow editing the configuration as a
JSON (perhaps also retaining the option to edit as a plist).  This would
be a natural extension of the existing
`eglot-show-workspace-configuration' command.

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[João Távora]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I would suggest adding a refined version of this, using a regular buffer
> for input.  Moreover, one could allow editing the configuration as a
> JSON (perhaps also retaining the option to edit as a plist).  This would
> be a natural extension of the existing
> `eglot-show-workspace-configuration' command.

I like this idea.  I agree it's crazy hard to set and get right.  But
this idea needs to be refined indeed.

I'm not crazy about adding this complexity to Eglot, especially because
there's nothing really eglot-specific about it.  It could be used for
any variable you want to add to the current project's root
.dir-locals.el.  But that involves much more consultation in
emacs-devel.

Here's an idea.  Let's make a bare-bones
eglot-show-workspace-configuration that simplifies this 80%.  The other
20% can for later.

In the patch I sent for the other "workspace configuration" bug, the
.dir-locals.el is consulted late (actually it is even now).  So a simple
implementation of eglot-edit-workspace-configuration could be just:

  (defun eglot-edit-workspace-configuration () (interactive)
    (find-file (expand-file-name ".dir-locals.el" (project-root (project-current)))))

My bet is that that two-liner would go a long way.

João

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[Augusto Stoffel]

On Tue, 28 Feb 2023 at 19:33, João Távora wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> I would suggest adding a refined version of this, using a regular buffer
>> for input.  Moreover, one could allow editing the configuration as a
>> JSON (perhaps also retaining the option to edit as a plist).  This would
>> be a natural extension of the existing
>> `eglot-show-workspace-configuration' command.
>
> I like this idea.  I agree it's crazy hard to set and get right.  But
> this idea needs to be refined indeed.
>
> I'm not crazy about adding this complexity to Eglot, especially because
> there's nothing really eglot-specific about it.  It could be used for
> any variable you want to add to the current project's root
> .dir-locals.el.  But that involves much more consultation in
> emacs-devel.

I see this differently.  Emacs doesn't have deeply nested namespaces for
variables and user options.  It's all pretty flat, so there is almost
surely never going to be a variable that is nearly as tricky to set.

[ Incidentally, when I suggested to allow such syntax:

       ((python-mode
       . ((eglot-workspace-configuration
           . (("pylsp.plugins.jedi_completion.include_params" . t)
              ("pylsp.plugins.jedi_completion.fuzzy" . t)
              ("pylsp.plugins.pylint.enabled" . :json-false))))))

   you said it was un-Lispy or something, but I really think it's rather
   the opposite.  Emacs has flat names pretty much like the above. ]

> Here's an idea.  Let's make a bare-bones
> eglot-show-workspace-configuration that simplifies this 80%.  The other
> 20% can for later.
>
> In the patch I sent for the other "workspace configuration" bug, the
> .dir-locals.el is consulted late (actually it is even now).  So a simple
> implementation of eglot-edit-workspace-configuration could be just:
>
>   (defun eglot-edit-workspace-configuration () (interactive)
>     (find-file (expand-file-name ".dir-locals.el" (project-root (project-current)))))
>
> My bet is that that two-liner would go a long way.

No, this is not enough.  At the very least I need a history variable to
look at the previous configurations.  This feature has to be a thing on
top of of `eglot-show-workspace-configuration'.

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[Augusto Stoffel]

On Tue, 28 Feb 2023 at 21:35, Augusto Stoffel wrote:

>> My bet is that that two-liner would go a long way.
>
> No, this is not enough.  At the very least I need a history variable to
> look at the previous configurations.  This feature has to be a thing on
> top of of `eglot-show-workspace-configuration'.

Here's a hasty sketch of what I have in mind, sans the history commands.

--8<---------------cut here---------------start------------->8---
;; -*- lexical-binding: t; -*-

(defvar eglot-configuration-map
  (let ((map (make-sparse-keymap)))
    (define-key map "\C-c\C-c" #'eglot-configuration-save)
    (define-key map "\C-c\C-v" #'eglot-configuration-apply)
    (define-key map "\C-c\C-k" #'kill-current-buffer)
    map))

(define-minor-mode eglot-configuration--mode
  "Mode to edit LSP config"
  :interactive nil
  :keymap eglot-configuration-map
  (setq-local header-line-format (substitute-command-keys "\
\\<eglot-configuration-map>\
Press \\[eglot-configuration-apply] to apply, \\[eglot-configuration-save] to save, \\[kill-current-buffer] to abort.")))

(defun eglot-configuration-apply (&optional save)
  (interactive)
  (let ((eglot-workspace-configuration
         (save-excursion
           (goto-char (point-min))
           (jsonrpc--json-read))))
    (with-current-buffer eglot-configure--buffer
      (eglot-signal-didChangeConfiguration (eglot--current-server-or-lose))
      (when save
        (save-window-excursion
          (let ((default-directory (project-root (eglot--current-project))))
            (add-dir-local-variable
             nil ;; Tricky choice
             'eglot-workspace-configuration
             eglot-workspace-configuration)
            (save-buffer)))))))

(defun eglot-configuration-save ()
  (interactive)
  (eglot-configuration-apply t))

(defvar-local eglot-configure--buffer nil) ;; Ugh, get rid of this?

(defun eglot-configure (&optional server)
  "Dump `eglot-workspace-configuration' as JSON for debugging."
  (interactive (list (and (eglot-current-server)
                          (eglot--read-server "Server configuration"
                                              (eglot-current-server)))))
  (let ((buffer (current-buffer))
        (conf (eglot--workspace-configuration-plist server)))
    (with-current-buffer (get-buffer-create "*EGLOT workspace configuration*")
      (erase-buffer)
      (insert (jsonrpc--json-encode conf))
      (with-no-warnings
        (require 'json)
        (when (functionp #'js-json-mode) (js-json-mode))
        (json-pretty-print-buffer))
      (setq eglot-configure--buffer buffer)
      (eglot-configuration--mode)
      (pop-to-buffer (current-buffer)))))
--8<---------------cut here---------------end--------------->8---

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[João Távora]

Augusto Stoffel <arstoffel@gmail.com> writes:

> I see this differently.  Emacs doesn't have deeply nested namespaces for
> variables and user options.  It's all pretty flat, so there is almost
> surely never going to be a variable that is nearly as tricky to set.

Oh but no :-) there are even scary DSLs in Emacs variables.  See
project-kill-buffer-conditions for example.  And customize-variable
groks it.

In effect you want something like customize-variable, but different UI
and with support for saving to .dir-locals.el.  I sympathize with the
idea, I really do, for two distinct reasons:

* I agree learning about dir-locals.el and its relation to a project's
configuration is confusing for newcomers

* I personally get extremely confused, even intimidated, by the Custom
  UI.

So if this UI appears elsewhere, like a separate package, Eglot can take
advantage. But I'm afraid I don't like the idea of starting this in
Eglot, so it's probably not going to happen as long as I am maintainer.

> [ Incidentally, when I suggested to allow such syntax:
>
>        ((python-mode
>        . ((eglot-workspace-configuration
>            . (("pylsp.plugins.jedi_completion.include_params" . t)
>               ("pylsp.plugins.jedi_completion.fuzzy" . t)
>               ("pylsp.plugins.pylint.enabled" . :json-false))))))
>
>    you said it was un-Lispy or something, but I really think it's rather
>    the opposite.  Emacs has flat names pretty much like the above. ]

It's indeed un-Lispy, but it's not very hard to write a helper function
to turn that into a plist, is it?

>>   (defun eglot-edit-workspace-configuration () (interactive)
>>     (find-file (expand-file-name ".dir-locals.el" (project-root (project-current)))))
>>
>> My bet is that that two-liner would go a long way.
>
> No, this is not enough.  At the very least I need a history variable to
> look at the previous configurations.  This feature has to be a thing on
> top of of `eglot-show-workspace-configuration'.

That idea is the best I can offer for now.  You'd have undo for history,
which is not bad.  And Git, since it's a file after all.

João

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[Augusto Stoffel]

On Wed,  1 Mar 2023 at 02:02, João Távora wrote:

> Oh but no :-) there are even scary DSLs in Emacs variables.  See
> project-kill-buffer-conditions for example.  And customize-variable
> groks it.

Yes, but the server configuration is not a DSL.  It's just a list of
variable names, but with deeply nested namespace.  So, say
pylsp.plugins.jedi_completion.include_params can only be true of false.
There is no mini-language to say when it's effectively true of false.

> In effect you want something like customize-variable, but different UI
> and with support for saving to .dir-locals.el.  I sympathize with the
> idea, I really do, for two distinct reasons:

A Customize kind of UI would be really cool, but then LSP needs a new
capability whereby the server sends a "schema" of the accepted options
(their names, types, documentation, etc.)

> So if this UI appears elsewhere, like a separate package, Eglot can take
> advantage. But I'm afraid I don't like the idea of starting this in
> Eglot, so it's probably not going to happen as long as I am maintainer.

The UI I'm suggesting is absolutely commonplace in Emacs: you edit the
eglot-show-workspace-configuration buffer and press C-c C-c to commit
the changes.  Compare with VC log, Magit commit, Org capture, PDF-Tools
annotations.  We'll just use the same base libraries.

>> [ Incidentally, when I suggested to allow such syntax:
>>
>>        ((python-mode
>>        . ((eglot-workspace-configuration
>>            . (("pylsp.plugins.jedi_completion.include_params" . t)
>>               ("pylsp.plugins.jedi_completion.fuzzy" . t)
>>               ("pylsp.plugins.pylint.enabled" . :json-false))))))
>>
>>    you said it was un-Lispy or something, but I really think it's rather
>>    the opposite.  Emacs has flat names pretty much like the above. ]
>
> It's indeed un-Lispy, but it's not very hard to write a helper function
> to turn that into a plist, is it?

Is it though?  Replace the dots by hyphens, and you will find some real
Emacs user options:

    vc.svn.program
    vc.git.program
    vc.git.log-edit.summary.target-len
    vc.git.log-edit.summary.max-len


>>>   (defun eglot-edit-workspace-configuration () (interactive)
>>>     (find-file (expand-file-name ".dir-locals.el" (project-root (project-current)))))
>>>
>>> My bet is that that two-liner would go a long way.
>>
>> No, this is not enough.  At the very least I need a history variable to
>> look at the previous configurations.  This feature has to be a thing on
>> top of of `eglot-show-workspace-configuration'.
>
> That idea is the best I can offer for now.  You'd have undo for history,
> which is not bad.  And Git, since it's a file after all.

This command is not really very useful, and if it were it would belong
in files-x.el.

bug#61868: 29.0.60; Eglot: setting "workspace" configurations should be easier

[João Távora]

On Wed, Mar 1, 2023 at 7:39 AM Augusto Stoffel <arstoffel@gmail.com> wrote:

> A Customize kind of UI would be really cool, but then LSP needs a new
> capability whereby the server sends a "schema" of the accepted options
> (their names, types, documentation, etc.)

If this feature gets added to Emacs, Eglot will probably recommend it
in the manual.  I hope it's not a "Customize kind of UI", though.
Editing sexps deserves better.

> > It's indeed un-Lispy
> Is it though?  Replace the dots by hyphens, and you will find some real
> Emacs user options:
>
>     vc.svn.program
>     vc.git.program
>     vc.git.log-edit.summary.target-len
>     vc.git.log-edit.summary.max-len

Gargantuan variable names are not what I would call the most Lispy
part of Emacs.

João