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

WIP customize eglot-server-programs #224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lmmarsano
Copy link

eglot-server-programs is not a custom variable.
Customization variables can be convenient for persisting changes to variables that aren't yet defined due to autoload.
If the idea is acceptable, please let me know if the :type definition for TCP Connection or Class Name is wrong.
We can also change tag names.
If this gets accepted, we may want to update the README.
If making eglot-server-programs a custom variable is wrong, we may want to explain in the README how to handle the variable when eglot autoloads.

@joaotavora
Copy link
Owner

The idea is quite acceptable. Give me some time to review it.

You seem to have gone to great lengths to code all possible syntaxes there. Can you be sure we didn't miss any?

If making eglot-server-programs a custom variable is wrong,

It's not wrong! It was just a hard/boring thing to do. But you seem to have solved that part :-)

@lmmarsano
Copy link
Author

Great.

Can you be sure we didn't miss any?

Adding inline documentation led me to catch a mistake interpreting your docstring:

Remaining ARGS are passed to `open-network-stream' for
upgrading the connection with encryption or other capabilities.

Though I thought that meant ARGS (TCP Arguments) should be all parameters to open-network-stream

eglot/eglot.el

Lines 85 to 96 in 00f71cb

(list :tag "TCP Connection"
(string :tag "Host")
(number :tag "Port")
(list :tag "TCP Arguments"
:inline t
string
(choice (const nil)
string)
string
(choice string
number)
(plist :key-type symbol)))

open-network-stream already includes Host & Port parameters, so it seems ARGS should be the rest PARAMETER of open-network-stream, which the call in eglot--connect seems to corroborate.
Is this right?

eglot/eglot.el

Lines 93 to 104 in 7efbb5a

(list :format ,eglot-custom-format-doc
:tag "TCP Connection"
:doc "For connecting to a server via TCP."
(string :format ,eglot-custom-format-doc
:tag "Host"
:doc "Name or IP address.")
(number :tag "TCP Port")
(plist :format ,eglot-custom-format-doc
:tag "TCP Parameters"
:doc "Passed to `open-network-stream' for upgrading the connection with encryption or other capabilities.
Sequence of keywords and values: consult `open-network-stream' PARAMETERS documentation."
:key-type (symbol :tag "Keyword")))

I think the :type expression closely matches the syntax specification in your docstring.
However, I'd like that reviewed if possible.

I've updated the README.md about the customization feature and inlined documentation for the widget.
Please check that out: I can change the tags, reorder inline documentation relative to tags and values, change the inline documentation, etc.

@lmmarsano lmmarsano force-pushed the feature/custom-server-programs branch from 7efbb5a to 281fa83 Compare February 10, 2019 08:31
@joaotavora
Copy link
Owner

Wow, @lmmarsano perhaps we should resynchronize ourselves as to what is needed here. All I-m willing to accept is to turn (defvar eglot-server-programs...) into (defcustom eglot-server-programs ...) with the simplest possible type specification that makes most customizations easy to do. So please, no new widget types or variables or documentation that repeats the existing docstring, or anything like that. I was thinking of something some 10-15 lines at most I think.

Also, do you have copyright assigned to the FSF?

@lmmarsano lmmarsano force-pushed the feature/custom-server-programs branch 3 times, most recently from 74ba7b2 to 92ff457 Compare February 11, 2019 03:13
@lmmarsano
Copy link
Author

lmmarsano commented Feb 11, 2019

These revised patches should fit your request.
I agree duplicating documentation isn't ideal. However, filling out widgets can sometimes feel like guesswork (value type? input syntax? meaning?) despite good documentation for equivalent Lisp expressions, since they aren't identical. How would you recommend addressing usability?
Emacs doesn't provide a field type for keyword symbols, so the code in this branch will actually accept any symbol and not warn the user. lmmarsano/eglot@05ec06a, which I've left out, fixes that, however, in case you want it.

Also, do you have copyright assigned to the FSF?

Not sure what that means.
Doesn't it all go under the same licensing, copyleft, etc?

@lmmarsano lmmarsano force-pushed the feature/custom-server-programs branch from 92ff457 to aee0032 Compare February 12, 2019 10:02
@lmmarsano lmmarsano force-pushed the feature/custom-server-programs branch from aee0032 to 0657192 Compare February 22, 2019 17:04
@lmmarsano
Copy link
Author

lmmarsano commented Feb 22, 2019

@joaotavora FSF recently confirmed I've assigned this and future copyrights to them, which you can verify.
Please let me know what else you need.
Is this code short enough?
Though I think shortening it further would require unstructured input from the user and make it harder for them, I'll do it if that's needed.
I'm also open to alternatives for shortening the code.

@joaotavora
Copy link
Owner

joaotavora commented Feb 22, 2019

Hi @lmmarsano, I haven't had a chance to look at this properly yet. But nice nonetheless that you have the FSF assignment paperwork taken care of. I'll try to have a look next week.

lmmarsano added 2 commits May 3, 2019 17:47
update editing instructions
update eglot's customization group summary
define eglot-server-programs as a custom variable
define eglot-server-programs-contact as a constant to splice into the customization type for eglot-server-programs
@lmmarsano lmmarsano force-pushed the feature/custom-server-programs branch from 0657192 to e1bad18 Compare May 3, 2019 22:00
@skangas
Copy link
Collaborator

skangas commented Jan 8, 2022

Sorry for the long delay in replying here. I tried rebasing this on top of current master, but customize says that there is a mismatch between the type and the value. Could you perhaps look into it?

I would also suggest squashing the two commits into one. Here's the commit message I would use:

Make eglot-server-programs customizable

* eglot.el (eglot-server-programs-contact): New defconst.
(eglot-server-programs): Make into defcustom.
* README.md: Document the above change.

@skangas skangas added the WIP Work in progress label Jan 8, 2022
@skangas
Copy link
Collaborator

skangas commented Jan 15, 2022

Hmm, so currently the value of eglot-server-programs can be quite messy indeed:

((rust-mode eglot-rls "rls")
 (cmake-mode "cmake-language-server")
 (vimrc-mode "vim-language-server" "--stdio")
 (python-mode closure
              ((alternatives "pylsp" "pyls"
                             ("pyright-langserver" "--stdio"))
               . #1=(markdown-fontify-code-blocks-natively t))
              . #2=((&optional interactive)
                    (let*
                        ((listified
                          (let*
                              ((--cl-var-- alternatives)
                               (a nil)
                               (--cl-var-- nil))
                            (while
                                (consp --cl-var--)
                              (setq a
                                    (car --cl-var--))
                              (setq --cl-var--
                                    (cons
                                     (if
                                         (listp a)
                                         a
                                       (list a))
                                     --cl-var--))
                              (setq --cl-var--
                                    (cdr --cl-var--)))
                            (nreverse --cl-var--)))
                         (err
                          #'(lambda nil
                              (error "None of '%s' are valid executables"
                                     (mapconcat #'car listified ", ")))))
                      (cond
                       (interactive
                        (let*
                            ((augmented
                              (mapcar
                               #'(lambda
                                   (a)
                                   (let
                                       ((found
                                         (eglot--executable-find
                                          (car a)
                                          t)))
                                     (and found
                                          (cons
                                           (car a)
                                           (cons found
                                                 (cdr a))))))
                               listified))
                             (available
                              (remove nil augmented)))
                          (cond
                           ((cdr available)
                            (cdr
                             (assoc
                              (completing-read "[eglot] More than one server executable available:"
                                               (mapcar #'car available)
                                               nil t nil nil
                                               (car
                                                (car available)))
                              available #'equal)))
                           ((cdr
                             (car available)))
                           (t nil))))
                       (t
                        (let*
                            ((--cl-var-- listified)
                             (args nil)
                             (p nil)
                             (probe nil)
                             (--cl-var-- t)
                             (--cl-var-- t)
                             --cl-var--)
                          (while
                              (and
                               (consp --cl-var--)
                               (progn
                                 (setq args
                                       (car --cl-var--)
                                       p
                                       (car-safe
                                        (prog1 args
                                          (setq args
                                                (cdr args)))))
                                 (setq probe
                                       (eglot--executable-find p t))
                                 (if probe
                                     (setq --cl-var--
                                           (cons probe args)
                                           --cl-var-- nil)
                                   t)))
                            (setq --cl-var--
                                  (cdr --cl-var--))
                            (setq --cl-var-- nil))
                          (if --cl-var--
                              (progn
                                (funcall err)
                                nil)
                            --cl-var--)))))))
 ((js-mode typescript-mode)
  "typescript-language-server" "--stdio")
 (sh-mode "bash-language-server" "start")
 ((php-mode phps-mode)
  "php" "vendor/felixfbecker/language-server/bin/php-language-server.php")
 ((c++-mode c-mode)
  closure
  ((alternatives "clangd" "ccls")
   . #1#)
  . #2#)
 (((caml-mode :language-id "ocaml")
   (tuareg-mode :language-id "ocaml")
   reason-mode)
  "ocamllsp")
 (ruby-mode "solargraph" "socket" "--port" :autoport)
 (haskell-mode "haskell-language-server-wrapper" "--lsp")
 (elm-mode "elm-language-server")
 (mint-mode "mint" "ls")
 (kotlin-mode "kotlin-language-server")
 (go-mode "gopls")
 ((R-mode ess-r-mode)
  "R" "--slave" "-e" "languageserver::run()")
 (java-mode . eglot--eclipse-jdt-contact)
 (dart-mode "dart_language_server")
 (elixir-mode "language_server.sh")
 (ada-mode "ada_language_server")
 (scala-mode "metals-emacs")
 (racket-mode "racket" "-l" "racket-langserver")
 ((tex-mode context-mode texinfo-mode bibtex-mode)
  "digestif")
 (erlang-mode "erlang_ls" "--transport" "stdio")
 (yaml-mode "yaml-language-server" "--stdio")
 (nix-mode "rnix-lsp")
 (gdscript-mode "localhost" 6008)
 ((fortran-mode f90-mode)
  "fortls")
 (lua-mode "lua-lsp")
 (zig-mode "zls")
 (css-mode closure
           ((alternatives
             ("vscode-css-language-server" "--stdio")
             ("css-languageserver" "--stdio"))
            . #1#)
           . #2#)
 (html-mode closure
            ((alternatives
              ("vscode-html-language-server" "--stdio")
              ("html-languageserver" "--stdio"))
             . #1#)
            . #2#)
 (json-mode closure
            ((alternatives
              ("vscode-json-language-server" "--stdio")
              ("json-languageserver" "--stdio"))
             . #1#)
            . #2#)
 (dockerfile-mode "docker-langserver" "--stdio"))

I'm not sure if defcustom can handle that, maybe it can, but how about instead of having closures, we just use symbols that point to a function to call?

@joaotavora WDYT?

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

Successfully merging this pull request may close these issues.

3 participants