[WIP] Support ltex-ls #863

lbiaggi · 2021-04-23T12:59:38Z

Initial working code to support ltex-ls with suggestion to address #858
thanks, @mjlbach, @mfussenegger

Looking for help/suggestion about these things:

How to generalize dictionary, disabledRules, hiddenFalsePositives, they currently need to be hardcoded using a valid table entry with an existing file previously created.
Discover a better way to expose tables with file entries.

mjlbach · 2021-04-23T15:52:35Z

I started reviewing, but I would highly recommend looking at another configuration example (rust-analyzer is a good one) to see the form these should take, you shouldn't have a setup{} call in the config for example, you shouldn't check for configs.ltex-ls etc.

lbiaggi · 2021-04-23T19:39:49Z

I started reviewing, but I would highly recommend looking at another configuration example (rust-analyzer is a good one) to see the form these should take, you shouldn't have a setup{} call in the config for example, you shouldn't check for configs.ltex-ls etc.

Ok

mjlbach · 2021-06-06T06:45:01Z

Ok, so I looked more into this when I wanted to use it. I am considering reconsidering allowing custom commands to be registered per language server. This https://github.com/neovim/nvim-lspconfig/pull/863/files#diff-b6698738921d6c7c76e0bb67c33623ba1d213719c995c8353468bcdb3411049bR156-R186 is extremely messy, and I'd like to avoid it somehow. I'm not sure what the appropriate way to do this is @mfussenegger

For now, I would recommend instead of overriding execute_command, I would override the handler for textDocument/codeAction, which can be passed as a key to setup. This will be cleaner and avoid mutating the global function which is used by other language servers.

mjlbach · 2021-06-06T06:45:47Z

lua/lspconfig/ltex.lua

+        dictionary_files = { ["en"] = {vim.fn.getcwd() .. "dictionary.ltex"} };
+        disabledrules_files = { ["en"] = {vim.fn.getcwd() .. "disable.ltex"} };
+        falsepositive_files = { ["en"] = {vim.fn.getcwd() .. "false.ltex"}};
+        settings = {


I'm pretty sure everything under settings is either a default, or too opinionated to be worth using.

With the size of this my module, my recommendation is to create ltex-ls.nvim and add a smaller minimal config in lspconfig to start. The following "works". We can gradually add back in the functionality while making any necessary changes in neovim core to support more "off-spec" language servers like this one:

local configs = require'lspconfig/configs' local util = require 'lspconfig/util' local server_name = "ltex-ls" configs[server_name] = { default_config = { cmd = {"ltex-ls"}; filetypes = {'tex', 'bib', 'markdown'}; root_dir = function(filename) return util.path.dirname(filename) end; settings = { ltex = { checkFrequency="edit", }; }; }; docs = { package_json = 'https://raw.githubusercontent.com/valentjn/vscode-ltex/develop/package.json', description = [[ https://github.com/valentjn/ltex-ls LTeX Language Server: LSP language server for LanguageTool 🔍✔️ with support for LaTeX 🎓, Markdown 📝, and others To install, download the latest [release](https://github.com/valentjn/ltex-ls/releases) and ensure `ltex-ls` is on your path. ]]; }; };

edit: Actually we need to send at least one setting for the server to trigger... seems like a bug.

What exactly didn't work? At least in my test, except to the "add to ..." and pickyrules which is disabled by default.

The server doesn't provide any diagnostics if not at least one settings item is sent, I'm advocating for the first round of inclusion of lspconfig, using the simpler config you quoted while we clean up this PR.

mjlbach · 2021-06-06T06:46:09Z

lua/lspconfig/ltex.lua

+        };
+        on_attach = function(client, bufnr)
+                    -- local lang = client.config.settings.ltex.language
+            for lang,_ in ipairs(client.config.dictionary_files) do       --


settings are automatically sent on_attach, I don't thin you need to update this.

This came from my use case, where I use two languages in the same document with different files for each language.

Do you have a better suggestion how to do this with multiple languages?

mjlbach · 2021-06-06T06:53:07Z

With the size of this my module, my recommendation is to create ltex-ls.nvim and add a smaller minimal config in lspconfig to start. The following "works". We can gradually add back in the functionality while making any necessary changes in neovim core to support more "off-spec" language servers like this one:

local configs = require'lspconfig/configs'
local util = require 'lspconfig/util'

local server_name = "ltex-ls"

configs[server_name] = {
  default_config = {
    cmd = {"ltex-ls"};
    filetypes = {'tex', 'bib', 'markdown'};
    root_dir = function(filename)
      return util.path.dirname(filename)
    end;
    settings = {
      ltex = {
        checkFrequency="edit",
      };
    };
  };
  docs = {
    package_json = 'https://raw.githubusercontent.com/valentjn/vscode-ltex/develop/package.json',
    description = [[
https://github.com/valentjn/ltex-ls

LTeX Language Server: LSP language server for LanguageTool 🔍✔️ with support for LaTeX 🎓, Markdown 📝, and others

To install, download the latest [release](https://github.com/valentjn/ltex-ls/releases) and ensure `ltex-ls` is on your path.

]];
  };
};

edit: Actually we need to send at least one setting for the server to trigger... seems like a bug.

lua/lspconfig/ltex.lua

mfussenegger · 2021-06-06T10:24:56Z

Ok, so I looked more into this when I wanted to use it. I am considering reconsidering allowing custom commands to be registered per language server. This https://github.com/neovim/nvim-lspconfig/pull/863/files#diff-b6698738921d6c7c76e0bb67c33623ba1d213719c995c8353468bcdb3411049bR156-R186 is extremely messy, and I'd like to avoid it somehow. I'm not sure what the appropriate way to do this is @mfussenegger

For now, I would recommend instead of overriding execute_command, I would override the handler for textDocument/codeAction, which can be passed as a key to setup. This will be cleaner and avoid mutating the global function which is used by other language servers.

neovim/neovim#14115 would help :)

In nvim-jdtls I provide a dedicated code_action function:

https://github.com/mfussenegger/nvim-jdtls/blob/8bd4eac08c637961b10ac01c124fb5091484c265/lua/jdtls.lua#L638-L702

But I only do that because otherwise I can't access the original code_action_params which are required for the java specific client commands. (the VSCode code action API gives access to them, so language servers naturally ended up depending on them). Otherwise I'd probably also override the handler. But the problem with that is that different plugins can become incompatible with each other, so the handler should only be overriden in the scope of the language specific client and not globally.

mjlbach · 2021-06-06T10:28:15Z

neovim/neovim#14115 would help :)

I totally forgot about that :) , we should stop punting on the decision about breaking symmetry in the args. Maybe we can convince @tjdevries to review next Friday.

Otherwise I'd probably also override the handler. But the problem with that is that different plugins can become incompatible with each other, so the handler should only be overriden in the scope of the language specific client and not globally.

For sure, in this case I'd advocate just adding a default handler in the config that overrides it just for ltex-ls, as I believe the current PR overrides a global function which IMO is not good. We can always make this cleaner once neovim/neovim#14115 in one form or another.

lua/lspconfig/ltex.lua

mjlbach · 2021-07-11T09:53:39Z

lua/lspconfig/ltex.lua

+
+local function findLtexFiles(filetype, value)
+    local buf_clients = vim.lsp.buf_get_clients()
+    for _, client in ipairs(buf_clients) do


mjlbach · 2021-07-11T09:53:46Z

lua/lspconfig/ltex.lua

+local function updateConfig(lang, configtype)
+    local buf_clients = vim.lsp.buf_get_clients()
+    local client = nil
+    for _, lsp in ipairs(buf_clients) do


abalmos · 2021-09-17T00:28:42Z

I would love to see this merged. Is there a hold up we could help with?

In the meantime, this is an option: https://github.com/brymer-meneses/grammar-guard.nvim

mjlbach · 2021-09-17T00:31:48Z

The hacks around buffer_execute command can be removed with the new handler signature, and this: neovim/neovim#14115 once merged will make adding client side commands easier.

I also have an unresolved comment that hasn't been addressed.

David-Else · 2021-09-27T15:26:38Z

@lbiaggi Neovim 0.5.1 is out, I think neovim/neovim#15504 might be the relevant change in the handler signature @mjlbach mentioned. Really looking forward to this getting merged :)

mjlbach · 2021-09-27T15:50:56Z

The client side command registry wasn't backported so this would still need to be gated behind a 0.6 'has' neovim/neovim#14115

Also, given the amount of client-side code I think this is probably better as a separate plugin, we'll see.

lbiaggi · 2021-09-27T20:35:42Z

The client side command registry wasn't backported so this would still need to be gated behind a 0.6 'has' neovim/neovim#14115

Also, given the amount of client-side code I think this is probably better as a separate plugin, we'll see.

Is there a better way to check nvim version (from lua) than this?

vim.fn.has('nvim-0.6')

mjlbach · 2021-09-27T23:10:27Z

That's what I would recommend.

williamboman · 2021-10-05T19:47:54Z

I would love to see this merged. Is there a hold up we could help with?

I've also added this in williamboman/nvim-lsp-installer#134 for the time being. Hopefully should be fairly straight forward to eject the custom code from that plugin, and only keep the installer, once things are figured out.

lbiaggi · 2021-10-07T17:14:35Z

Hi, I removed the custom code from the MR. I will send later a working patch with 0.6 changes to the project from @brymer-meneses (I got almost everything working as in VS Code) 😄

David-Else · 2021-10-14T10:34:49Z

Brand new version 14 of ltex-ls just came out, https://github.com/valentjn/ltex-ls/releases/tag/14.0.0 , can't wait to get this in Neovim!

mjlbach · 2021-10-18T20:21:51Z

@lbiaggi let me know if you have any questions

oblitum · 2021-10-19T01:06:27Z

@David-Else for your info (only, and if you care): valentjn/ltex-ls#103 (comment).

David-Else · 2021-10-19T09:39:52Z

@oblitum Thanks for the info! Unfortunately I am not using coc, so will have to wait until this PR lands.

mjlbach · 2021-10-19T15:10:08Z

If a user wants to use this now, I recommend https://github.com/brymer-meneses/grammar-guard.nvim which is an extension that provides ltex-ls support for the built-in client (it's now added to the wiki). Having it in the monorepo is great (many users like/prefer the convenience of the monorepo, similar to how vim itself ships runtime filetype and syntax plugins in a "monorepo"), but this shouldn't be blocking anyone.

abalmos · 2021-10-29T20:48:56Z

@mjlbach This can get merged now I think? It makes ltex work and there is an issue on grammar guard to implement the local command handlers for false positives, etc. (brymer-meneses/grammar-guard.nvim#4) I plan to attempt that PR if someone else doesn't do it first, but that is likely at least 4-5 weeks in the future.

lua/lspconfig/ltex.lua

mjlbach · 2021-10-31T23:54:50Z

I'm merging this by popular demand (lspconfig is ofc community contributed), a couple notes:

It's really hard to have a universal default for grammar/language (english is hardcoded here, which I dislike)
ltex-ls requires many custom client-side/off-spec commands
ltex-ls especially is one of those servers that users probably want to use in arbitrary places, so scratch server support would be nice to have, but is currently blocked by the lspconfig rewrite

For those reasons, I highly encourage people to either contribute to grammar-guard, or start their own custom plugin which provides additional features (which we can then reference in the wiki as with the other growing number of language specific plugins that integrate or bypass lspconfig to use the client directly). Lspconfig is always going to try to be as "on-spec" and minimal as possible (for servers that are on-spec/may not necessitate their own plugin since it would be about 10 LOC, are unpopular, etc.). I updated CONTRIBUTING to make this more clear the other day.

Co-authored-by: William Boman <william@redwill.se>

disrupted · 2021-11-01T09:26:18Z

I add the executables to my $PATH like this: .bashrc
alias ltex-ls="/home/david/bin/ltex-ls-14.0.0/bin/ltex-ls"
alias ltex-cli="/home/david/bin/ltex-ls-14.0.0/bin/ltex-cli"
Any idea what could be happening?

if you're just aliasing the commands, the directory is not in your $PATH and the commands are only known to your shell. What you'd want is export PATH="$PATH:~/bin/ltex-ls-14.0.0/bin" or you could move it to another directory within your PATH.

oblitum · 2021-11-01T10:27:12Z

english is hardcoded

The thread previously referred by me here is originally about language auto detection, which ltex/language-tool does support (with the given limitations as per thread).

David-Else · 2021-11-01T11:49:41Z

@disrupted Can't believe I made such a dumb error! Here is the install script I just made, in case in saves anyone some time. No need to add anything to the path if you install to the proper directory and add a symlink :)

NOTE: You need Java on your computer using the version of ltex-ls I download in the script, headless minimal version should be OK, I use Rocky Linux package:

Name         : java-11-openjdk-headless
Epoch        : 1
Version      : 11.0.13.0.8

#!/bin/bash
set -euo pipefail

BIN_INSTALL_DIR=/usr/local/bin

LTEXLS_LOCATION=valentjn/ltex-ls/releases/download/15.0.0/
LTEXLS_FILENAME=ltex-ls-15.0.0.tar.gz
LTEXLS_SHA=76bed42397cd52a05415e107cd1163c05e13f00fa2e8d4e6c841f7f7f0fd4a67d99474dd6f86aa4ebef5d46e4cd227310badc52bc1b2c055c5aa390a9ac706a4

if [ "$(id -u)" != 0 ]; then
    echo "You're not root! Run script with sudo" && exit 1
fi

# Call with arguments (${1} location,${2} filename,${3} sha)
download_verify() {
    curl -LOf "https://github.com/${1}${2}"
    echo "${3} ./${2}" | sha512sum --check
}

download_verify "$LTEXLS_LOCATION" "$LTEXLS_FILENAME" "$LTEXLS_SHA"
tar --no-same-owner -C $BIN_INSTALL_DIR/ -xf $LTEXLS_FILENAME --no-anchored 'bin' --strip=1
tar --no-same-owner -C $BIN_INSTALL_DIR/ -xf $LTEXLS_FILENAME --no-anchored 'lib' --strip=1
ln -s $BIN_INSTALL_DIR/bin/ltex-ls $BIN_INSTALL_DIR/ltex-ls
rm $LTEXLS_FILENAME

David-Else · 2021-11-01T12:24:31Z

Also, if people want to use the ngram data this is now working great:

-- optionally install ngram data into home directory (~/en) and uncomment languageModel = '~/'
-- https://dev.languagetool.org/finding-errors-using-n-gram-data

require('lspconfig').ltex.setup {
  settings = {
    ltex = {
      additionalRules = {
        enablePickyRules = true,
        motherTongue = 'en',
        -- languageModel = '~/',
      },
    },
  },
}

You get a lot more corrections, brings it closer to the online grammarly results while keeping everything on your own computer.

oblitum · 2021-11-01T13:01:37Z

I'd advice to not extract directly at ~ and have its own ngram directory with en, etc. underneath. So to avoid potential problems in case you happen to put some specially (for ltex) named directory under ~ without realizing.

David-Else · 2021-11-01T20:48:24Z

If the user doesn't add language then they get no spell checking, only grammar checking. Maybe adding it should be default, or maybe this is intentional as users are expected to be using the built in Neovim spell check?

require('lspconfig').ltex.setup {
  settings = {
    ltex = {
      language = 'en-US',
    },
  },
}

I understand that adding words to the dictionary is something that needs a plugin.

valentjn · 2021-11-01T21:22:19Z

@David-Else The reason for the missing spelling corrections is that the default config in this PR uses the language en. As the docs say, if you want spelling corrections, you have to use a specific variant like LT_EX's default value en-US. I don't know if this is on purpose or not (if Neovim already has a spell checker that's activated by default, then it makes sense).

On a related note, generally, I would advise not setting defaults explicitly and let LT_EX take care of those, if possible—except for good client-specific reasons. This makes changes of settings structure/default values easier and it enables a more consistent experience across clients. Plus, there are some thoughts behind most default values. Picky rules for instance, which this PR activates, are disabled by default as they generate quite some false positives, which can confuse users (e.g., valentjn/vscode-ltex#419).

mjlbach · 2021-11-01T21:32:24Z

@valentjn Thank you so much for stopping by, it's extremely appreciated when upstream authors provide direct input.

@David-Else The reason for the missing spelling corrections is that the default config in this PR uses the language en. As the docs say, if you want spelling corrections, you have to use a specific variant like LTEX's default value en-US. I don't know if this is on purpose or not (if Neovim already has a spell checker that's activated by default, then it makes sense).

setspell is not on by default, and IMO ltex-ls should be preferred over it.

On a related note, generally, I would advise not setting defaults explicitly and let LTEX take care of those, if possible—except for good client-specific reasons.

Is there any setting here we need to be sending? I'm fine just wiping this, I believe I had asked about this before during review from the original PR author.

 ltex = {
        enabled = { 'latex', 'tex', 'bib', 'markdown' },
        checkFrequency = 'edit',
        language = 'en',
        diagnosticSeverity = 'information',
        setenceCacheSize = 2000,
        additionalRules = {
          enablePickyRules = true,
          motherTongue = 'en',
        },
        dictionary = {},
        disabledRules = {},
        hiddenFalsePositives = {},
      },
    },

I'm fine doing a quick follow-up PR based on your feedback, although it would be great if a neovim user who actively uses ltex-ls can take ownership over maintaining these settings from time-to time (perhaps keeping it in sync with grammar guard).

valentjn · 2021-11-02T08:30:40Z

LT_EX itself doesn't need any setting to be set. Missing settings are automatically set to their default values.

The only setting that's actually necessary in this case should be ltex.enabled, because I think Neovim uses the language ID tex for L^AT_EX documents. LT_EX follows the LSP spec, which recommends latex (tex is preferred for plain T_EX or undetermined T_EX dialects). That means there won't be L^AT_EX checking unless tex is added to ltex.enabled.

We had that problem before for the coc.nvim version in valentjn/vscode-ltex#425, where the problem was fixed by changing the mapping of filetypes to LSP language IDs in coc.nvim. Not sure if something like this would be possible here.

If not, then I'd suggest setting ltex.enabled to the default plus 'tex':

'bibtex', 'html', 'latex', 'markdown', 'org', 'restructuredtext', 'rsweave', 'tex'

The other settings can be removed. I haven't tried it, though.

mjlbach · 2021-11-02T16:30:05Z

Yes, we can pass get_language_id to start client to do the remapping, similar to the solution in coc.nvim. I'll file the PR, thanks for the input!

mjlbach · 2021-11-02T16:35:30Z

ref: for those in the thread #1364

Haven't tested, will check everything works later.

David-Else · 2021-12-19T22:39:30Z

I am not sure grammar guard is happening any time soon, getting the ability to define a file to be the user dictionary and have the code action to add to it work would be pretty amazing at this point :)

mjlbach · 2021-12-19T22:41:09Z

I encourage someone else to start an additiona ltexls.nvim then, it's out of scope of lspconfig to add off-spec handlers/requests. Lspconfig should always be a fallback/backup to a dedicated extension (or the defacto if the client is on-spec I guess).

mjlbach mentioned this pull request Apr 25, 2021

How to intercept ["workspace/executeCommand"] to use a custom lua function instead of sending to LS? #858

Closed

mjlbach reviewed Jun 6, 2021

View reviewed changes

lua/lspconfig/ltex.lua Outdated Show resolved Hide resolved

brymer-meneses mentioned this pull request Jul 9, 2021

Add LanguageTool kabouzeid/nvim-lspinstall#50

Open

mjlbach reviewed Jul 11, 2021

View reviewed changes

lua/lspconfig/ltex.lua Outdated Show resolved Hide resolved

mjlbach reviewed Jul 11, 2021

View reviewed changes

David-Else mentioned this pull request Oct 20, 2021

Problems with setup: attempt to index field 'grammar_guard' (a nil value) brymer-meneses/grammar-guard.nvim#5

Closed

williamboman mentioned this pull request Oct 29, 2021

ltex fails to update configuration williamboman/nvim-lsp-installer#209

Closed

williamboman reviewed Oct 30, 2021

View reviewed changes

lua/lspconfig/ltex.lua Show resolved Hide resolved

mjlbach force-pushed the patch-1 branch 2 times, most recently from fa4e657 to 39caff8 Compare October 31, 2021 23:50

feat: add support for ltex-ls

ee0e5b3

Co-authored-by: William Boman <william@redwill.se>

mjlbach force-pushed the patch-1 branch from 39caff8 to ee0e5b3 Compare October 31, 2021 23:57

mjlbach merged commit dfa8c8f into neovim:master Nov 1, 2021

David-Else mentioned this pull request Dec 1, 2021

Does ltex-ls support neovim local dictionaries? valentjn/ltex-ls#124

Closed

David-Else mentioned this pull request Apr 3, 2022

ltex-ls language server code actions don't work helix-editor/helix#1942

Closed

David-Else mentioned this pull request Jun 19, 2022

Code actions not working in neovim valentjn/ltex-ls#168

Open

[WIP] Support ltex-ls #863

[WIP] Support ltex-ls #863

Conversation

lbiaggi commented Apr 23, 2021

mjlbach commented Apr 23, 2021

lbiaggi commented Apr 23, 2021

mjlbach commented Jun 6, 2021

mjlbach Jun 6, 2021

Choose a reason for hiding this comment

lbiaggi Jun 6, 2021

Choose a reason for hiding this comment

mjlbach Jun 7, 2021

Choose a reason for hiding this comment

mjlbach Jun 6, 2021

Choose a reason for hiding this comment

lbiaggi Jun 6, 2021 • edited

Choose a reason for hiding this comment

mjlbach commented Jun 6, 2021 • edited

mfussenegger commented Jun 6, 2021 • edited

mjlbach commented Jun 6, 2021

mjlbach Jul 11, 2021

Choose a reason for hiding this comment

mjlbach Jul 11, 2021

Choose a reason for hiding this comment

lbiaggi Jul 14, 2021

Choose a reason for hiding this comment

abalmos commented Sep 17, 2021

mjlbach commented Sep 17, 2021

David-Else commented Sep 27, 2021

mjlbach commented Sep 27, 2021 • edited

lbiaggi commented Sep 27, 2021

mjlbach commented Sep 27, 2021

williamboman commented Oct 5, 2021

lbiaggi commented Oct 7, 2021

David-Else commented Oct 14, 2021

mjlbach commented Oct 18, 2021

oblitum commented Oct 19, 2021

David-Else commented Oct 19, 2021

mjlbach commented Oct 19, 2021 • edited

abalmos commented Oct 29, 2021

mjlbach commented Oct 31, 2021 • edited

disrupted commented Nov 1, 2021

oblitum commented Nov 1, 2021

David-Else commented Nov 1, 2021 • edited

David-Else commented Nov 1, 2021

oblitum commented Nov 1, 2021

David-Else commented Nov 1, 2021

valentjn commented Nov 1, 2021

mjlbach commented Nov 1, 2021

valentjn commented Nov 2, 2021

mjlbach commented Nov 2, 2021

mjlbach commented Nov 2, 2021

David-Else commented Dec 19, 2021

mjlbach commented Dec 19, 2021

lbiaggi Jun 6, 2021 •

edited

mjlbach commented Jun 6, 2021 •

edited

mfussenegger commented Jun 6, 2021 •

edited

mjlbach commented Sep 27, 2021 •

edited

mjlbach commented Oct 19, 2021 •

edited

mjlbach commented Oct 31, 2021 •

edited

David-Else commented Nov 1, 2021 •

edited