Minor bug in eglot; managed-major-mode must be a list

Minor bug in eglot; managed-major-mode must be a list

[Stephen Leake]

See bug#58745; there's a patch included. Report duplicated here:

I'm calling `eglot' from lisp. The doc string says the first argument
`managed-major-mode' is an atom, not a list. But it is passed to
eglot--connect as the first argument, which is expected to be a list of
major modes. When called interactively, `managed-major-mode' is set by
(eglot--guess-contact t), which returns a list.

So when not interactive, `eglot' must convert `managed-major-mode' into
a list:

-------------------

[2. text/x-patch]
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 71001ba680..dee88db022 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1078,6 +1078,8 @@ eglot
 
 INTERACTIVE is t if called interactively."
   (interactive (append (eglot--guess-contact t) '(t)))
+  (unless (listp managed-major-mode)
+    (setq managed-major-mode (list managed-major-mode)))
   (let* ((current-server (eglot-current-server))
          (live-p (and current-server (jsonrpc-running-p current-server))))
     (if (and live-p
------------------


João Távora; ok to commit this patch?

-- 
-- Stephe

Re: Minor bug in eglot; managed-major-mode must be a list

[Theodor Thornhill]

On 23 October 2022 19:55:33 CEST, Stephen Leake <stephen_leake@stephe-leake.org> wrote:
>See bug#58745; there's a patch included. Report duplicated here:
>
>I'm calling `eglot' from lisp. The doc string says the first argument
>`managed-major-mode' is an atom, not a list. But it is passed to
>eglot--connect as the first argument, which is expected to be a list of
>major modes. When called interactively, `managed-major-mode' is set by
>(eglot--guess-contact t), which returns a list.
>
>So when not interactive, `eglot' must convert `managed-major-mode' into
>a list:
>
>-------------------
>
>[2. text/x-patch]
>diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>index 71001ba680..dee88db022 100644
>--- a/lisp/progmodes/eglot.el
>+++ b/lisp/progmodes/eglot.el
>@@ -1078,6 +1078,8 @@ eglot
> 
> INTERACTIVE is t if called interactively."
>   (interactive (append (eglot--guess-contact t) '(t)))
>+  (unless (listp managed-major-mode)
>+    (setq managed-major-mode (list managed-major-mode)))
>   (let* ((current-server (eglot-current-server))
>          (live-p (and current-server (jsonrpc-running-p current-server))))
>     (if (and live-p
>------------------
>
>
>João Távora; ok to commit this patch?
>

I believe eglot has a helper for this - 'eglot--ensure-list', maybe use that for consistency?

Theodor

Re: Minor bug in eglot; managed-major-mode must be a list

[Robert Pluim]

>>>>> On Mon, 24 Oct 2022 06:49:56 +0200, Theodor Thornhill <theo@thornhill.no> said:

    Theodor> I believe eglot has a helper for this - 'eglot--ensure-list', maybe
    Theodor> use that for consistency?

Or even the `ensure-list' from subr.el

Robert
-- 

Re: Minor bug in eglot; managed-major-mode must be a list

[Philip Kaludercic]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 24 Oct 2022 06:49:56 +0200, Theodor Thornhill <theo@thornhill.no> said:
>
>     Theodor> I believe eglot has a helper for this - 'eglot--ensure-list', maybe
>     Theodor> use that for consistency?
>
> Or even the `ensure-list' from subr.el

That was defined in 28.1 but Eglot intends to support versions back to
26.1.  I would suggest using Compat to avoid issues like this but that
is a matter of taste.

Re: Minor bug in eglot; managed-major-mode must be a list

[Robert Pluim]

>>>>> On Mon, 24 Oct 2022 15:54:19 +0000, Philip Kaludercic <philipk@posteo.net> said:

    Philip> Robert Pluim <rpluim@gmail.com> writes:
    >>>>>>> On Mon, 24 Oct 2022 06:49:56 +0200, Theodor Thornhill <theo@thornhill.no> said:
    >> 
    Theodor> I believe eglot has a helper for this - 'eglot--ensure-list', maybe
    Theodor> use that for consistency?
    >> 
    >> Or even the `ensure-list' from subr.el

    Philip> That was defined in 28.1 but Eglot intends to support versions back to
    Philip> 26.1.  I would suggest using Compat to avoid issues like this but that
    Philip> is a matter of taste.

Iʼd forgotten eglot started life outside emacs :-)

Robert
-- 

Re: Minor bug in eglot; managed-major-mode must be a list

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

On Mon, Oct 24, 2022, 17:55 Robert Pluim <rpluim@gmail.com> wrote:

> >>>>> On Mon, 24 Oct 2022 15:54:19 +0000, Philip Kaludercic <
> philipk@posteo.net> said:
>
>     Philip> Robert Pluim <rpluim@gmail.com> writes:
>     >>>>>>> On Mon, 24 Oct 2022 06:49:56 +0200, Theodor Thornhill <
> theo@thornhill.no> said:
>     >>
>     Theodor> I believe eglot has a helper for this - 'eglot--ensure-list',
> maybe
>     Theodor> use that for consistency?
>     >>
>     >> Or even the `ensure-list' from subr.el
>
>     Philip> That was defined in 28.1 but Eglot intends to support versions
> back to
>     Philip> 26.1.  I would suggest using Compat to avoid issues like this
> but that
>     Philip> is a matter of taste.
>
> Iʼd forgotten eglot started life outside emacs :-)
>

Yes. Be sure to read the commentary on the file's header able compatibility
with other Emacs versions, and design philosophy in general. Happy to add
clarifications. Grammar fixes welcome there, too ;-)

Also, please try to CC me in Eglot-related matters, i don't always track
the mailing lists so closely recently.

João

>

[-- Attachment #2: Type: text/html, Size: 2055 bytes --]

eglot.el commentary suggestion

[Robert Pluim]

>>>>> On Tue, 25 Oct 2022 09:33:22 +0100, João Távora <joaotavora@gmail.com> said:

    João> Yes. Be sure to read the commentary on the file's header able compatibility
    João> with other Emacs versions, and design philosophy in general. Happy to add
    João> clarifications. Grammar fixes welcome there, too ;-)

I know we all laugh about how weʼre all pedants, but there are people
who suffer from debilitating OCD and related syndromes, so perhaps we
can tone down the humour:

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 827a5e9848..eeec1488e0 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -81,7 +81,8 @@
 ;;   in place during Eglot's LSP-enriched tenure over a project.  Even
 ;;   so, some of those decisions will invariably aggravate a minority
 ;;   of Emacs power users, but these users can use `eglot-stay-out-of'
-;;   and `eglot-managed-mode-hook' to quench their OCD.
+;;   and `eglot-managed-mode-hook' to adjust things to their
+;;   preferences.
 ;;
 ;; * On occasion, to enable new features, Eglot can have soft
 ;;   dependencies on popular libraries that are not in Emacs core.

Robert
-- 

Re: eglot.el commentary suggestion

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

Hi Robert, thanks for reading the commentary,

On Tue, Oct 25, 2022 at 11:16 AM Robert Pluim <rpluim@gmail.com> wrote:
>     João> Yes. Be sure to read the commentary on the file's header able
compatibility
>     João> with other Emacs versions, and design philosophy in general.
Happy to add
>     João> clarifications. Grammar fixes welcome there, too ;-)
> I know we all laugh about how weʼre all pedants, but there are people
> who suffer from debilitating OCD and related syndromes, so perhaps we
> can tone down the humour:

Or we can turn it up to 11 with your other much better suggestion!

-;;   of Emacs power users, but these users can use `eglot-stay-out-of'
-;;   and `eglot-managed-mode-hook' to quench their OCD.
+;;   of Emacs power pedants, but these users can use
+;;   `eglot-stay-out-of' and `eglot-managed-mode-hook' to quench their
+;;   debilitating OCD and related syndromes.

Then maybe add a heart emoji or fluffy ascii kitten to subdue the
hyperventilation.

I'd focus efforts on making eglot-stay-out-of and e-m-m-hook less necessary
in the first place.  For example:

* If the default of eldoc-documentation-strategy were eldoc-d-compose Eglot
could
  stop setting eldoc-documentation-strategy and tweak more ElDoc stuff.

* If Eglot supported multiple LSP servers simultaneously, less people would
need to
  tweak flymake-diagnostic-functions to add e.g. a ESlint backend to it.

* If Emacs had a decent built-in TUI and GUI completion tooltip with less
  baggage, Eglot could stop setting company-backends, etc.

* I _think_ Eglot can already stop setting xref-prompt-for-identifier, but
I'm not
  100% sure.

--
João power pedant doctrinaire emeritus III

[-- Attachment #2: Type: text/html, Size: 2066 bytes --]

Re: eglot.el commentary suggestion

[Po Lu]

João Távora <joaotavora@gmail.com> writes:

> Or we can turn it up to 11 with your other much better suggestion!

+1.  If the jokes are removed from Emacs, then many people will quickly
lose interest.

Re: eglot.el commentary suggestion

[Robert Pluim]

>>>>> On Tue, 25 Oct 2022 19:16:26 +0800, Po Lu <luangruo@yahoo.com> said:

    Po Lu> João Távora <joaotavora@gmail.com> writes:
    >> Or we can turn it up to 11 with your other much better suggestion!

    Po Lu> +1.  If the jokes are removed from Emacs, then many people will quickly
    Po Lu> lose interest.

Thereʼs no need to attempt to such a hyperbolic line of argument. Iʼm
only suggesting that a statement that can easily be read as mocking
people with OCD be changed.

Robert
-- 

Re: eglot.el commentary suggestion

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

A joke, in any form, will always be done at the expense of
someone.  This is how they're funny. Key is to recognize whether
characteristics being made fun of are someone's choices.
If they're not, the joke can still be funny to some, but is universally
unfair and inelegant.

OCD is a term commonly appropriated by nerds like
me.  However, being a medical term strongly suggests
it's not someone's choice.  So you're right.

So feel free to replace "OCD" with "meticulousness" or
something like that.

I just typed that up in a hurry to frame a design decisions in some
humour.  But now it's just been over-thought too much and
wasn't really that funny to begin with. IOW you ruined it, :-)

So the other patch is fine too. If the rest of the commentary is less
controversial, can we spend more brain and typing on those other
topics I listed, perhaps?

João

[-- Attachment #2: Type: text/html, Size: 1239 bytes --]

Re: eglot.el commentary suggestion

[Robert Pluim]

>>>>> On Tue, 25 Oct 2022 13:51:02 +0100, João Távora <joaotavora@gmail.com> said:

    João> I just typed that up in a hurry to frame a design decisions in some
    João> humour.  But now it's just been over-thought too much and
    João> wasn't really that funny to begin with. IOW you ruined it, :-)

Youʼre welcome :-)

    João> So the other patch is fine too. If the rest of the commentary is less
    João> controversial, can we spend more brain and typing on those other
    João> topics I listed, perhaps?

Pushed to master.

I canʼt do anything sensible with your suggestions until I actually
use eglot in real life for a while. First I have to fix the
defcustomʼs :-)

Robert
-- 

Re: Minor bug in eglot; managed-major-mode must be a list

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Mon, Oct 24, 2022, 05:38 Stephen Leake <stephen_leake@stephe-leake.org>
wrote:

> See bug#58745; there's a patch included. Report duplicated here:
>
> I'm calling `eglot' from lisp. The doc string says the first argument
> `managed-major-mode' is an atom, not a list. But it is passed to
> eglot--connect as the first argument, which is expected to be a list of
> major modes. When called interactively, `managed-major-mode' is set by
> (eglot--guess-contact t), which returns a list.
>
> So when not interactive, `eglot' must convert `managed-major-mode' into
> a list:
>
> -------------------
>
> [2. text/x-patch]
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 71001ba680..dee88db022 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -1078,6 +1078,8 @@ eglot
>
>  INTERACTIVE is t if called interactively."
>    (interactive (append (eglot--guess-contact t) '(t)))
> +  (unless (listp managed-major-mode)
> +    (setq managed-major-mode (list managed-major-mode)))
>    (let* ((current-server (eglot-current-server))
>           (live-p (and current-server (jsonrpc-running-p current-server))))
>      (if (and live-p
> ------------------
>
>
> João Távora; ok to commit this patch?
>

Yes (but please CC me next time to grab my attention).

Also see other's comments about ensure-list.

Also eglot, the function, does not have any non-interactive use i know of.

João

>

[-- Attachment #2: Type: text/html, Size: 2226 bytes --]

Re: eglot.el commentary suggestion

[Payas Relekar]

João Távora <joaotavora@gmail.com> writes:

> ...
> João power pedant doctrinaire emeritus III

Heh, off-topic, but this signature is sopt-on and highly appreciated :D
--