unifiedjs / unified-language-server Goto Github PK

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

We already support quick fixes, but they require the user to manually select the quick fix. This is slightly tedious if there’s only one suggestion.

Solution

Code actions support the isPreferred option. This signals the editor that this is the preferred code action for the given diagnostic.

VS Code uses the Quick Fix… command to apply a code action. We already support this. If we mark a code action as isPreferred, VS Code can fix the issue using the Auto Fix… and Fix All commands, bypassing the quick fix selection. Users can also opt-in to auto fix all violations with preferred fixes on save.

Alternatives

Don’t support this. 🤷

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

A user wishes to use unified-language-server or remark-language-server without installing remark in their local project folder

Solution

unified-language-server should fall back to a global or bundled installation of remark if a local one is not found

Alternatives

installing remark locally solely for the sake of a specific class of editors is not ideal

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

You might also support ancestors: https://github.com/vfile/vfile-message#fields

Originally posted by @wooorm in #57 (review)

I looked into ancestors, but I think the information is not relevant here. It makes the messages very verbose. As an end user of the language server, the fact that everything is based on ASTs, is an implementation detail.

Originally posted by @remcohaszing in #57 (comment)

I’m not saying it should be a default. It isn’t in vfile-reporter either.

But I see if quite different. There are many reasons for errors to be thrown. I think users and the code that emits errors should decide whether the positional info and other info in the AST is an “implementation detail” or not.

Originally posted by @wooorm in #57 (comment)

Solution

Add option showAncestors. If this is true, the ancestry will be added to the LSP diagnostic message.

Alternatives

1. showAncestors was chosen arbritrarily. The name could be different.
2. Use DiagnosticRelatedInformation instead of appending to the message. This makes the trace clickable, but it renders differently.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Comments in neovim/nvim-lspconfig#1606, neovim/nvim-lspconfig#1641, and mattn/vim-lsp-settings#527 show that users don’t know what remark is. When they open a markdown file, they are prompted to enable remark-language-server. When they do enable it, they are prompted with a message to install remark.

Also VS Code users are going to run into this issue. They will install the extension to get the functionality in projects that use remark, but the extension will show a prompt every time they open a project which doesn’t use remark.

Solution

Don’t perform actions for workspaces where no remark configuration is found.

To accomplish this, the logic for resolving a configuration needs to be exposed in unified-engine.

Alternatives

An alternative would be to add language server settings. This allows the user to specify at least VS Code users to define settings to enable or disable certain features per project.

This would be similar to prettier.enable and prettier.requireConfig in prettier-vscode.

This puts control, but also burden in the hands of the users.

Subject of the feature

Review AST and only reprocess segments that have been changed.

Problem

Large documents can be slow to reparse as you type.

Expected behaviour

Only lint/review parts of the document that have changed.
Allowing for faster feedback.

Alternatives

https://github.com/micromark/micromark for incremental parsing.

Note

This may not be possible with the current design of unified.
Thoughts on how it may be achievable are welcome.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

As part of remarkjs/remark#910 I would like to make unified-language-server based on unified-engine, so it yields the same result as CLIs created using unified-args.

Currently the language server is written in CJS, has a custom configuration file, and is synchronous. Unfortunately this means a lot of the existing code needs to be rewritten. Fortunately this also means a lot of code can be removed.

In order to do this right, the command line interface of the language server needs to be redesigned.

Solution

I imagine the CLI works like this:

unified-language-server \
  --prefix remark \
  --plugin remark-parse \
  --plugin remark-stringify

• --prefix defines the prefix that’s passed to unified-engine. I.e. the example loads .remarkrc.js, .remarkignore, etc.
• --plugin specifies plugins to be loaded that aren’t defined in the configuration file. They will be loaded using load-plugin.

Also I suggest we add a pre existing command line parser, i.e. commander

Alternatives

Another possibility is to load remark. This better matches behavior of remark-cli. However, I’m not sure how this should be defined, because remark uses a named export.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

latest

Link to runnable example

No response

Steps to reproduce

cwd is not set when creating a file or when creating an engine.

file.cwd must be set to the folder that is the “workplace”, e.g., the place where the package.json and .git are typically.
Right now, it’s not set, so it’s (90% sure) set to a wrong value. Which means certain plugins break (example: https://github.com/remarkjs/remark-usage/blob/ae591a38bfb5424a802f25ae77b69f5dc06836fa/lib/index.js#L13-L27)

Expected behavior

Here’s how I found a logical one for atom: https://github.com/unifiedjs/unified-engine-atom/blob/605326d08a6d04b66aa8abf3143313f24c507699/index.js#L44-L53

Actual behavior

(unknown behavior, likely broken plugins or unexpected results)

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

workspace/didChangeWatchedFiles means a file or folder changed. https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspace_didChangeWatchedFiles. Instead of reprocessing every markdown file (

Solution

perhaps we should use cheap hashes to ensure we only perform work when something changed.

Alternatives

n/a

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Input and output file URIs sometimes don‘t match. E.g.,:

While your problem currently presents itself around symlinks, it might not be a problem unique to symlinks (e.g., there might be other values that are sent which are sent back differently, due to escapes/encoding of characters, slashes, case-sensitive/-insensitive file systems, &c.).

(GH-32).

Solution

For a solution, I think we can solve this quite like what you’re currently doing it. But I dislike the name file.data.uri. And commenting about symlinks only.
What about calling it file.data.lspInputUri or so? To signal that its purpose is to pass a certain string from that protocol through the engine and back?

lspDocumentUri is great, yeah! I like it more than lspInputUri

(GH-32).

Alternatives

n/a

Subject of the feature

Add automated tests

Problem

Currently the code has only been tested by running manually starting server and running an editor to manually test.

Expected behaviour

Some testing should be automated.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

If the server uses the stdio protocol for communication, stdout is used for communication. However, console.log() etc statements also write to stdout. This causes the following error:

node_modules/vscode-jsonrpc/lib/common/messageReader.js:138
                    throw new Error('Header must provide a Content-Length property.');
                    ^

Error: Header must provide a Content-Length property.
    at StreamMessageReader.onData (node_modules/vscode-jsonrpc/lib/common/messageReader.js:138:27)
    at Socket.<anonymous> (node_modules/vscode-jsonrpc/lib/common/messageReader.js:122:18)
    at Socket.emit (node:events:520:28)

The language server simply shouldn’t use console.log(), but third party plugins or other third party packages might do so.

Solution

The language server can rewrite the global console, so its calls are rerouted to connection.console.

Alternatives

The output could be disabled entirely.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

There are several reasons the unified engine throws an error. It’s currently not tested though:

Solution

Add a test

Alternatives

n/a

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

If a plugin throws, an error diagnostic is created. This diagnostic contains the stack trace as the message.

Solution

It might look a bit nicer to clean the stack trace using clean-stack, or to trim out even unified internals, which requires stack-utils.

Alternatives

Don’t trim stack traces. 🤷

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

2.x

Link to runnable example

remarkjs/vscode-remark#65

Steps to reproduce

• git clone [email protected]:remcohaszing/vscode-remark.git
  cd vscode-remark
  npm ci

• Close all instances of Visual Studio Code
• Open Visual Studio Code either in Windows or from any directory that isn’t the repo you just cloned
• Start debugging
• In the debugged VS Code instance which opened readme.md, add the text Foo. Bar.

Expected behavior

A diagnostic should appear

Actual behavior

No diagnostic appears.

Further inspection showed this is caused by the fact that the current working directory of the language server doesn’t reflect the file opened.

I found this out first by troubleshooting Windows. This shows the following values:

• workspacesAsPaths: C:\Users\remco\AppData\Programs\Microsoft VS Code
• file.path: z:\vs-code\readme.md

There is no correlation between the two.

I was also able to reproduce this on Linux by opening VSCode using the desktop launcher. This sets the cwd to /home/remco, while the file is in /home/remco/Projects/vscode-remark/readme.md. remark should be resolved from /home/remco/Projects/vscode-remark, not /home/remco.

While typing this issue, I also noticed the different casing styles used for the drive letters on Windows.

Runtime

Node v16

Package manager

npm v7

OS

Other (please specify in steps to reproduce)

Build and bundle tools

esbuild

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

latest

Link to runnable example

No response

Steps to reproduce

1. Install Sublime Text
2. Install LSP-remark Repo
3. See that remark defaults to a local directory due to unified engine logic

Expected behavior

Provides ability to send custom node_module location

Actual behavior

Doesn't allow custom paths

Runtime

Node v16

Package manager

yarn v2

OS

macOS

Build and bundle tools

No response

Subject of the feature

Document usage in more editors

Problem

Language servers are designed to be usable with almost any editor.
Currently only NeoVim usage is documented.

Expected behaviour

Include documentation for other editors, including:

• VSCode
• Vim
• Emacs
• SublimeText
• Atom

https://langserver.org/#implementations-client

Alternatives

Link to external documentation

In a markdown file

Server is running responding 1 (:call LanguageClient_isServerRunning())

When I do :call LanguageClient#textDocument_formatting() I get

[LC] Error: Failure { jsonrpc: Some(V2), error: Error { code: MethodNotFound, message: "Unhandled method textDocument/formatting", data: None }, id: Num(2) }

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Some unified plugins support the vfile-message#note property. It might be nice to support this in the language server as well.

Solution

If note is define on a vfile message, add it to the diagnostic message joined by 1 or 2 newlines.

Alternatives

1. Don’t support it
2. Make it customizable. This is more work and I think users are unlikely to explicitly enable or disable this.

Subject of the feature

Better documentation around handling of configuration file, or specifying config file as an argument when starting server

Problem

Currently, the docs do not make it clear where and how the configuration file is supposed to be sent to server.

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

unified-language-server has the potential to work great for vim-lsp users editing markdown files. However, it’s a bit annoying to set up.

Solution

If it’s registered to vim-lsp-settings, it can be configured using a single command.

Alternatives

🤷