bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Troy Brown]

I discovered that the workspace configuration specified in a
`.dir-locals.el` file was not being used if the mode is not listed
first in `eglot-server-programs`.  Consider a scenario where the
current buffer's major mode is `ada-ts-mode` and Eglot is enabled in
that buffer.  Also, consider the following `.dir-locals.el` in a
parent directory containing the file of the buffer.

```elisp
((ada-ts-mode . ((eglot-workspace-configuration . (:ada (:projectFile
"src/gtkada.gpr"))))))
```

If I then execute `M-x eglot-show-workspace-configuration RET` in that
`ada-ts-mode` buffer, the "*EGLOT workspace configuration*" buffer
opens and displays a value of "null".

```json
null
```

However, if I change the mode in `.dir-locals.el` from `ada-ts-mode`
to `ada-mode` and repeat the "show configuration" command, I see the
configuration as expected, even though the major mode is
`ada-ts-mode`.

```json
{
  "ada": {
    "projectFile": "src/gtkada.gpr"
  }
}
```

I believe this is due to the following statement in
`eglot--workspace-configuration-plist`, which sets the major mode in
the temporary buffer before calling
`hack-dir-local-variables-non-file-buffer`:

```elisp
;; Set the major mode to be the first of the managed
;; modes.  This is the one the user started eglot in.
(setq major-mode (car (eglot--major-modes server)))
```

The comment indicates that the major mode that Eglot was started in
will be listed first, but that is not the case.  If I execute `M-:
(eglot--major-modes (eglot-current-server))` from the `ada-ts-mode`
buffer, the value is `(ada-mode ada-ts-mode)`, which is the same order
they are listed in `eglot-server-programs`, and is not ordered based
on the major mode of the buffer where Eglot was started.
Additionally, `eglot--connect` which populates `eglot--languages`
doesn't change the order from how they are listed in
`eglot-server-programs`.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Eli Zaretskii]

> Cc: João Távora <joaotavora@gmail.com>
> From: Troy Brown <brownts@troybrown.dev>
> Date: Sun, 15 Sep 2024 15:54:00 -0400
> 
> I discovered that the workspace configuration specified in a
> `.dir-locals.el` file was not being used if the mode is not listed
> first in `eglot-server-programs`.  Consider a scenario where the
> current buffer's major mode is `ada-ts-mode` and Eglot is enabled in
> that buffer.  Also, consider the following `.dir-locals.el` in a
> parent directory containing the file of the buffer.
> 
> ```elisp
> ((ada-ts-mode . ((eglot-workspace-configuration . (:ada (:projectFile
> "src/gtkada.gpr"))))))
> ```
> 
> If I then execute `M-x eglot-show-workspace-configuration RET` in that
> `ada-ts-mode` buffer, the "*EGLOT workspace configuration*" buffer
> opens and displays a value of "null".
> 
> ```json
> null
> ```
> 
> However, if I change the mode in `.dir-locals.el` from `ada-ts-mode`
> to `ada-mode` and repeat the "show configuration" command, I see the
> configuration as expected, even though the major mode is
> `ada-ts-mode`.
> 
> ```json
> {
>   "ada": {
>     "projectFile": "src/gtkada.gpr"
>   }
> }
> ```
> 
> I believe this is due to the following statement in
> `eglot--workspace-configuration-plist`, which sets the major mode in
> the temporary buffer before calling
> `hack-dir-local-variables-non-file-buffer`:
> 
> ```elisp
> ;; Set the major mode to be the first of the managed
> ;; modes.  This is the one the user started eglot in.
> (setq major-mode (car (eglot--major-modes server)))
> ```
> 
> The comment indicates that the major mode that Eglot was started in
> will be listed first, but that is not the case.  If I execute `M-:
> (eglot--major-modes (eglot-current-server))` from the `ada-ts-mode`
> buffer, the value is `(ada-mode ada-ts-mode)`, which is the same order
> they are listed in `eglot-server-programs`, and is not ordered based
> on the major mode of the buffer where Eglot was started.
> Additionally, `eglot--connect` which populates `eglot--languages`
> doesn't change the order from how they are listed in
> `eglot-server-programs`.

João and Stefan, any comments?

FWIW, I'd rather think this is a feature, since users don't need
separate Eglot settings for ada-mode and ada-ts-mode.  But maybe I'm
missing something.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[João Távora]

On Mon, Sep 16, 2024 at 12:38 PM Eli Zaretskii <eliz@gnu.org> wrote:

> João and Stefan, any comments?
>
> FWIW, I'd rather think this is a feature, since users don't need
> separate Eglot settings for ada-mode and ada-ts-mode.  But maybe I'm
> missing something.

You didn't miss that much.  This is half-feature, half
headache-I-didn't-want-to-worry-about. So I put int the 'car'
on purpose.  The first of the modes listed in a e-s-programs entry
is usually the most representative one of the language, more or less.

Is there a downside to mentioning a major mode you don't
actually use in your .dir-locals.el, Troy?

João

PS: This is again because Emacs doesn't have a great way to
refer to "language" or to associate languages to modes and
vice-versa. I certainly don't want to reopen that can of worms.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Troy Brown]

On Mon, Sep 16, 2024 at 9:48 AM João Távora <joaotavora@gmail.com> wrote:
>
> On Mon, Sep 16, 2024 at 12:38 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > João and Stefan, any comments?
> >
> > FWIW, I'd rather think this is a feature, since users don't need
> > separate Eglot settings for ada-mode and ada-ts-mode.  But maybe I'm
> > missing something.
>
> You didn't miss that much.  This is half-feature, half
> headache-I-didn't-want-to-worry-about. So I put int the 'car'
> on purpose.  The first of the modes listed in a e-s-programs entry
> is usually the most representative one of the language, more or less.
>
> Is there a downside to mentioning a major mode you don't
> actually use in your .dir-locals.el, Troy?
>

I would say the major downside is that it is not intuitive at all.
It's not consistent with how directory-local variables behave.  When I
saw that the variable was set locally in my buffer but that the
configuration wasn't actually applied to the server, I had to dig into
this and understand what was going on.  I have to imagine anyone else
using this would be just as stumped as I was when it wasn't working.

I don't think I'd agree with the first mode being representative, it
seems somewhat arbitrary.  For instance, in order to change
eglot-workspace-configuration for "sh-mode", you have to set the
configuration in .dir-locals.el for "bash-ts-mode"...who would've
guessed this?  Just looking through the lists of modes corresponding
to the same server, I don't think people would normally make this
connection, and the fact that it doesn't work the same way that
directory-local variables do, makes it even more unexpected and
confusing.

What about having eglot--workspace-configuration-plist cycle through
the list of modes for the server until it finds a non-nil
eglot-workspace-configuration, or has scanned them all?  This would
seem like a better approach since it will at least find a
configuration if one exists, rather than ignoring an existing
configuration.  I don't know if that would be considered too CPU
intensive or not.  It's still not the same behavior as a directory
local variable, but I think it would be an improvement over the
current behavior.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Eli Zaretskii]

> From: Troy Brown <brownts@troybrown.dev>
> Date: Mon, 16 Sep 2024 13:07:15 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>, 73280@debbugs.gnu.org
> 
> > Is there a downside to mentioning a major mode you don't
> > actually use in your .dir-locals.el, Troy?
> >
> 
> I would say the major downside is that it is not intuitive at all.

With the emergence of the *-ts-mode modes, we need to adjust our
intuition.  We decided that having foo-mode settings cover foo-ts-mode
as well as much as possible is an advantage, not a disadvantage.  So
our intuition needs to follow suit.

> It's not consistent with how directory-local variables behave.

I disagree.  Certain settings in .dir-locals are completely global,
which is even more radical than this case.  Having to specify a
setting once for several related modes is a Good Thing, IMO.

> I don't think I'd agree with the first mode being representative, it
> seems somewhat arbitrary.  For instance, in order to change
> eglot-workspace-configuration for "sh-mode", you have to set the
> configuration in .dir-locals.el for "bash-ts-mode"...who would've
> guessed this?

I'd like to think that in a not-so-distant future, _everyone_ will
guess that.  Why not? it makes perfect sense to me.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[João Távora]

On Mon, Sep 16, 2024 at 7:15 PM Eli Zaretskii <eliz@gnu.org> wrote:

> I'd like to think that in a not-so-distant future, _everyone_ will
> guess that.  Why not? it makes perfect sense to me.

I mean, we can fix this one if we really want to.  Or make a separate
entry just for sh-mode.  In fact the user can do that if she is very
bothered by this: this is defcustom after all.  I must say I don't find
any compelling arguments to worry much about this bug, except
that maybe some note in the documentation could be
added.

João

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 16 Sep 2024 19:39:03 +0100
> Cc: Troy Brown <brownts@troybrown.dev>, monnier@iro.umontreal.ca, 73280@debbugs.gnu.org
> 
> On Mon, Sep 16, 2024 at 7:15 PM Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > I'd like to think that in a not-so-distant future, _everyone_ will
> > guess that.  Why not? it makes perfect sense to me.
> 
> I mean, we can fix this one if we really want to.  Or make a separate
> entry just for sh-mode.  In fact the user can do that if she is very
> bothered by this: this is defcustom after all.  I must say I don't find
> any compelling arguments to worry much about this bug, except
> that maybe some note in the documentation could be
> added.

Fixing documentation is fine by me, thanks.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Troy Brown]

On Mon, Sep 16, 2024 at 2:15 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > I would say the major downside is that it is not intuitive at all.
>
> With the emergence of the *-ts-mode modes, we need to adjust our
> intuition.  We decided that having foo-mode settings cover foo-ts-mode
> as well as much as possible is an advantage, not a disadvantage.  So
> our intuition needs to follow suit.
>

That would be fine if this was isolated to only "ts" vs "non-ts"
modes.  Another example is "typescript-mode".  In order to configure
the server, you have to use "js-mode" in .dir-locals.el.  Why?
Because it's the first one in the list.

> > It's not consistent with how directory-local variables behave.
>
> I disagree.  Certain settings in .dir-locals are completely global,
> which is even more radical than this case.  Having to specify a
> setting once for several related modes is a Good Thing, IMO.
>

I proposed in my last response a mechanism where you could specify a
configuration for any of the modes associated with a server and have
it applied, rather than ignoring a configuration if it wasn't
explicitly specified for the first one in the list.  The way it
currently exists, you'll never be able to change the first mode in the
list or you will likely break someone's .dir-locals.el which had to
rely on this ordering in order to properly configure their server.

> > I don't think I'd agree with the first mode being representative, it
> > seems somewhat arbitrary.  For instance, in order to change
> > eglot-workspace-configuration for "sh-mode", you have to set the
> > configuration in .dir-locals.el for "bash-ts-mode"...who would've
> > guessed this?
>
> I'd like to think that in a not-so-distant future, _everyone_ will
> guess that.  Why not? it makes perfect sense to me.

What is that?  It's not consistent with the "foo-mode settings cover
foo-ts-mode", it's actually the other way around.  You have to set the
"ts" version in order to have it apply to the "non-ts" version.  YAML
is another one where the "ts" version is listed first in the list.
These are inconsistent.  Some lists specify the "non-ts" version
before the "ts" version, others swap those.  And furthermore, some
lists contain multiple "non-ts" modes, so it's arbitrary which one is
listed first.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[João Távora]

On Mon, Sep 16, 2024 at 7:54 PM Troy Brown <brownts@troybrown.dev> wrote:

> That would be fine if this was isolated to only "ts" vs "non-ts"
> modes.  Another example is "typescript-mode".  In order to configure
> the server, you have to use "js-mode" in .dir-locals.el.  Why?
> Because it's the first one in the list.

Well, typescript is a transpile-to-js language, doesn't strike me as
that odd.

Also have you read section 5.1.1 of the Eglot's user manual?  You
can just use `nil` as the mode and `w-s-configuration` will
be set buffer-locally in all modes for buffers within that dir.
This is probably easier.  Check it out.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Eli Zaretskii]

> From: Troy Brown <brownts@troybrown.dev>
> Date: Mon, 16 Sep 2024 14:53:53 -0400
> Cc: joaotavora@gmail.com, monnier@iro.umontreal.ca, 73280@debbugs.gnu.org
> 
> > With the emergence of the *-ts-mode modes, we need to adjust our
> > intuition.  We decided that having foo-mode settings cover foo-ts-mode
> > as well as much as possible is an advantage, not a disadvantage.  So
> > our intuition needs to follow suit.
> >
> 
> That would be fine if this was isolated to only "ts" vs "non-ts"
> modes.  Another example is "typescript-mode".  In order to configure
> the server, you have to use "js-mode" in .dir-locals.el.  Why?
> Because it's the first one in the list.

Isn't Typescript a variant of JavaScript?

> I proposed in my last response a mechanism where you could specify a
> configuration for any of the modes associated with a server and have
> it applied, rather than ignoring a configuration if it wasn't
> explicitly specified for the first one in the list.

I'm not against it, I just responded to the part of your arguments
which are more philosophical.

> > > I don't think I'd agree with the first mode being representative, it
> > > seems somewhat arbitrary.  For instance, in order to change
> > > eglot-workspace-configuration for "sh-mode", you have to set the
> > > configuration in .dir-locals.el for "bash-ts-mode"...who would've
> > > guessed this?
> >
> > I'd like to think that in a not-so-distant future, _everyone_ will
> > guess that.  Why not? it makes perfect sense to me.
> 
> What is that?  It's not consistent with the "foo-mode settings cover
> foo-ts-mode", it's actually the other way around.

Bash is a variant of a Bourne shell, isn't it?

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Troy Brown]

On Mon, Sep 16, 2024 at 3:06 PM João Távora <joaotavora@gmail.com> wrote:
>
> On Mon, Sep 16, 2024 at 7:54 PM Troy Brown <brownts@troybrown.dev> wrote:
>
> > That would be fine if this was isolated to only "ts" vs "non-ts"
> > modes.  Another example is "typescript-mode".  In order to configure
> > the server, you have to use "js-mode" in .dir-locals.el.  Why?
> > Because it's the first one in the list.
>
> Well, typescript is a transpile-to-js language, doesn't strike me as
> that odd.
>
> Also have you read section 5.1.1 of the Eglot's user manual?  You
> can just use `nil` as the mode and `w-s-configuration` will
> be set buffer-locally in all modes for buffers within that dir.
> This is probably easier.  Check it out.

Thanks for the pointer on using "nil", I had forgotten about that.

Since it appears I'm failing to make a convincing argument, I think
that using "nil" will be my approach going forward.  I'll avoid using
mode-based configuration in Eglot, as I find it deceptive and prone to
future breakage.  In its current state, I think it would be wise to
discourage its use in the user manual, to avoid misunderstanding and
potential future problems.

I'm surprised nobody has raised this issue in the past, but don't be
surprised if you receive additional bug reports in the future.

bug#73280: 30.0.90; Eglot: eglot-workspace-configuration might not be found in .dir-locals.el

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> ```elisp
>> ;; Set the major mode to be the first of the managed
>> ;; modes.  This is the one the user started eglot in.
>> (setq major-mode (car (eglot--major-modes server)))
>> ```

This code is indeed hackish/heuristic by nature.

I guess the reason for it is that it needs to work regardless of the
buffers that use that server (which could all use different major
modes).

Alternatives:

- Get `eglot-workspace-configuration` "once and for all" when starting
  the server, using the major mode currently active the buffer from
  which the server's launch was triggered.
- Collect all the buffers currently using that server, take the set of
  modes they currently use, fetch the corresponding
  `eglot-workspace-configuration` values, and "merge them" (and if they
  disagree, maybe emit a warning?).

But in the mean time, it seems `eglot-workspace-configuration` was
designed so that you can have a single setting shared for all modes, as
João mentions, so using nil (or `prog-mode`) instead of `ada-mode` or
`ada-ts-mode` seems like the better option.


        Stefan