Skip to content

plugins/ltes-extra: support ltex_plus #3129

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+66 −17
Open

plugins/ltes-extra: support ltex_plus #3129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c793e8
plugins/lsp: enable ltex_plus
Kirens Mar 30, 2025
81b327e
plugins/lsp: fix incorrect path in ltex-settings
Kirens Mar 30, 2025
0633c9d
plugins/ltex-extra: prefer ltex_plus over ltex
Kirens Mar 30, 2025
d7048c8
plugins/lspsaga: fix incorrect scope in mkWarnings
Kirens Mar 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/by-name/lspsaga/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ in
{
enable = mkOverride 1490 true;
};
warnings = lib.nixvim.mkWarnings "plugins.ltex-extra" {
warnings = lib.nixvim.mkWarnings "plugins.lspsaga" {
# https://nvimdev.github.io/lspsaga/implement/#default-options
when = (cfg.implement.enable == true) && (cfg.symbolInWinbar.enable == false);

Expand Down
77 changes: 63 additions & 14 deletions plugins/by-name/ltex-extra/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,24 @@
lib,
helpers,
config,
options,
...
}:
with lib;
let
lspCfg = config.plugins.lsp;
in
lib.nixvim.plugins.mkNeovimPlugin {
name = "ltex-extra";
packPathName = "ltex_extra.nvim";
package = "ltex_extra-nvim";

maintainers = [ maintainers.loicreynier ];

description = ''
This plugin works with both the ltex or ltex_plus language servers and will enable ltex_plus if neither are.
Copy link
Member

Choose a reason for hiding this comment

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

e.g. this

'';

callSetup = false;

settingsOptions = {
Expand Down Expand Up @@ -44,24 +52,65 @@ lib.nixvim.plugins.mkNeovimPlugin {
};

extraConfig = cfg: {
warnings = lib.nixvim.mkWarnings "plugins.ltex-extra" {
when = !config.plugins.lsp.enable;
message = ''
You have enabled `ltex-extra` but not the lsp (`plugins.lsp`).
You should set `plugins.lsp.enable = true` to make use of the LTeX_extra plugin's features.
'';
};

plugins.lsp = {
servers.ltex = {
# Enable the ltex language server
enable = true;
warnings = lib.nixvim.mkWarnings "plugins.ltex-extra" [
{
when = !lspCfg.enable;
message = ''
You have enabled `ltex-extra` but not the lsp (`plugins.lsp`).
You should set `plugins.lsp.enable = true` to make use of the LTeX_extra plugin's features.
'';
}
(
let
expectedDefs = map toString [
./.
../../lsp/language-servers
];
isExternal = d: !elem d.file expectedDefs;
anyExternal =
acc: name: v:
let
e = findFirst isExternal null v.definitionsWithLocations;
in
if acc != null then
acc
else if e == null then
null
else
{
inherit name;
inherit (e) file;
};
external = foldlAttrs anyExternal null options.plugins.lsp.servers.ltex;
Copy link
Member

Choose a reason for hiding this comment

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

Interesting approach using the definition location 😎

I wonder if it'd be better to filter for external definitions, instead of reducing to the first match? That way we could list all offending definitions in the warning.

I'm also a little concerned about false-pisitives where there are irrelevant low-prio defs. I can't recall ottomh whether definitions includes all definitions or only definitions of the highestPrio? I can check this in nix repl when I'm next at a PC.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, it was sort of inspired by your options priority-suggestion.

If I recall from my testing, it filters based on priority, but only at the top level. So if there are recursive attributes, such as in .settings, even irrelevant options appear.

In this particular case, it would basically only be encountered if there is a config with an onAttached.function = mkDefault ... as we're not really setting any other property. And I estimate that to be exceedingly rare.

Copy link
Member

Choose a reason for hiding this comment

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

Another consideration is if/how mkIf false definitions show up. E.g. a user might have a toggle somewhere between a manual configuration of the lsp and using this plugin, implemented using mkIf.

in
{
# TODO: Added 2025-03-30; remove after 25.05
# Warn if servers.ltex seems to be configured outside of ltex-extra
when = !lspCfg.servers.ltex.enable && external != null;
message = ''
in ${external.file}
You seem to have configured `plugins.lsp.servers.ltex.${external.name}` for `ltex-extra`.
Copy link
Member

@MattSturgeon MattSturgeon Apr 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If we have a reference to the option itself, we can interpolate it here.

"${options.foo}"
=> "programs.nixvim.foo"

options come with a __toString attr with the value lib.showOption option.loc, so interpolating them will stringify to the option path.

This has the advantage of automatically including path prefixes, such as programs.nixvim.

It now uses `plugins.lsp.servers.ltex_plus` by default,
either move the configuration or explicitly enable `ltex` with `plugins.lsp.servers.ltex.enable = true`
Comment on lines +93 to +94
Copy link
Member

Choose a reason for hiding this comment

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

I'm probably being a bit dense, but I struggle to follow what "it" is refering to. Also what/where the "configuration" should be moved.

Layout nit: I wonder if bullet points would be slightly clearer? If not, maybe we should consider an Oxford-comma before the "or"?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I struggled to layout that part for a bit. Good suggestions, I'll revisit this.

'';
}
)
];

onAttach.function = ''
plugins.lsp =
let
attachLua = ''
require("ltex_extra").setup(${lib.nixvim.toLuaObject cfg.settings})
'';
in
{
servers.ltex.onAttach.function = attachLua;
servers.ltex_plus = {
# Enable ltex_plus if ltex is not already enabled
enable = mkIf (!lspCfg.servers.ltex.enable) (mkDefault true);
onAttach.function = attachLua;
};
};
};
};

settingsExample = {
Expand Down
2 changes: 1 addition & 1 deletion plugins/lsp/language-servers/ltex-settings.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ in
'';
};

fields = defaultNullOpts.mkAttrsOf' {
bibtex.fields = defaultNullOpts.mkAttrsOf' {
type = types.bool;
pluginDefault = { };
example = {
Expand Down
2 changes: 1 addition & 1 deletion plugins/lsp/lsp-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
"laravel_ls"
"lean3ls"
"lelwel_ls"
"ltex_plus"
"lwc_ls"
"m68k"
"marko-js"
Expand Down Expand Up @@ -281,6 +280,7 @@
lemminx = "lemminx";
lsp_ai = "lsp-ai";
ltex = "ltex-ls";
ltex_plus = "ltex-ls-plus";
lua_ls = "lua-language-server";
luau_lsp = "luau-lsp";
markdown_oxide = "markdown-oxide";
Expand Down