unified-language-server

unified-language-server copied to clipboard

Initial checklist

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

Problem

Comments in https://github.com/neovim/nvim-lspconfig/pull/1606, https://github.com/neovim/nvim-lspconfig/pull/1641, and https://github.com/mattn/vim-lsp-settings/issues/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.

• neovim/nvim-lspconfig#1641 shows that the projects that have this problem can solve this problem. In the case of nvim-lspconfig, the remark integration is different from similar integrations (e.g., stylelint, eslint). Perhaps the remark integrations should be similar to the other integrations?
• I think that it should be possible to run unified-language-server when there is no configuration
• I find the alternative solution very acceptable. It’s is how many integrations work at least in Atom work. I am not sure what makes it a burden?

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 alternative sounds good. Having a good default for when no config is present (maybe starting off, maybe defaulting to a preset). And offering the option to move away from the default if wanted.

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

Having used the prettier plugin having the option was nice, what in particular do you see as a risk/burden for its use with remark/unified?

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 alternative sounds good. Having a good default for when no config is present (maybe starting off, maybe defaulting to a preset). And offering the option to move away from the default if wanted.

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

Having used the prettier plugin having the option was nice, what in particular do you see as a risk/burden for its use with remark/unified?

I now think what I previously called burden, is better called control.

I suggest to add the following options:

• ${prefix}.enable: If false, the language server doesn’t perform any diagnostics nor formatting. This is useful for enabling or disabling the language server per workspace. (boolean, default: true)
• ${prefix}.requireConfig: The language server only perform diagnostics and formatting if a configuration file is found. (boolean, default true)

This means by default users only get remark language server functionality remark in the projects where remark has been configured. However, users can forcefully disable it on a project basis or enable the language server where no remark configuration exists (useful for formatting only).

I think it makes sense too. For the (neo)vim side of things, it wouldn't be too hard to add the same options as in vscode and pass them down. I'm wondering what would the language server do when enable is true and requireConfig is false? Does it do anything out-of-the-box? Or is that a separate discussion to be had?

I'm wondering what would the language server do when enable is true and requireConfig is false? Does it do anything out-of-the-box?

In this case it will do formatting, but effectively no diagnostics. (It will do diagnostics, but remark will never actually emit any. Other language servers implementing unified-language-server are allowed to do so.)

(Boolean) APIs are typically easier to understand if they default to false, and can instead be turned on. Can we come up different option names where the default of false makes sense, and they can be opted-into?

I believe that it’s weird for default behavior of “The language server only perform diagnostics and formatting if a configuration file is found”. unified formats code and does not need configuration to do so. If the vim world can only support all possible linters and formatters for every language combined in one package, then that’s a specific Vim problem, which we can solve for them. It does not have to affect the default for all Atom or VS Code users. I’m open to such behavior. I don’t see why it should be the normal default. Note: prettier-vscode, which seems to be the inspiration for this option as it is linked, defaults this to requireConfig: false.

• Where does ${prefix} from your examples come from?
• As we’re discussing this in unified-language-server, how would these new options be passed or detected?

If you imagine them being passed as parameters, then I don’t see the need for this project to support enable, because enable: false would be the same as not creating the language server?

(Boolean) APIs are typically easier to understand if they default to false, and can instead be turned on. Can we come up different option names where the default of false makes sense, and they can be opted-into?

I don’t think it’s wrong to default to true. Naming it enable (sometimes enabled) and defaulting to true appears to be an unwritten convention. For example this is how it’s done by the VS Code extensions for ESLint, Flow, git, Prettier, Stylelint, XML, XO, YAML, and more.

I believe that it’s weird for default behavior of “The language server only perform diagnostics and formatting if a configuration file is found”. unified formats code and does not need configuration to do so. If the vim world can only support all possible linters and formatters for every language combined in one package, then that’s a specific Vim problem, which we can solve for them. It does not have to affect the default for all Atom or VS Code users. I’m open to such behavior.

I think it’s important to listen to the feedback from the community. This feature is in demand.

I don’t see why it should be the normal default. Note: prettier-vscode, which seems to be the inspiration for this option as it is linked, defaults this to requireConfig: false.

The big difference between prettier-vscode and remark-language-server is that prettier-vscode works by default, whereas with remark-language-server the user is expected to install remark.

• Where does ${prefix} from your examples come from?

This would be a new option of createLanguageServer defined by remark-language-server.

• As we’re discussing this in unified-language-server, how would these new options be passed or detected?

Options are part of LSP. They are defined in .vscode/settings.json, .vimrc, or whatever configuration file the client supports. This can also be changed per workspace. I still need to investigate what the implementation would look like.

If you imagine them being passed as parameters, then I don’t see the need for this project to support enable, because enable: false would be the same as not creating the language server?

If this were the case, then I’d agree.

For example this is how it’s done by the VS Code extensions for ESLint, Flow, git, Prettier, Stylelint, XML, XO, YAML, and more.

I’m okay with checkboxes in VS Code’s UI. And I don’t want double negatives: https://youtu.be/HYl7ReNB5TA?t=996. But to reiterate, I find it wrong for APIs (https://twitter.com/domenic/status/721770094674776064), particularly if there are alternatives. Note my request is to try and find better terms if possible.

This feature is in demand.

I don’t see it. I am talking about defaults. Vim being broken can be solved regardless of what the default is. Vim being solved is in demand.

prettier-vscode […] remark-language-server

This compares apples and oranges. remark-language-server is not vscode-remark. And here we’re in unified-language-server. We can discuss vscode-remark passing defaultProcessor to unified-language-server to match Prettier. Regardless, even if remark has to be installed (locally or globally) by a user, I still believe that configuration should not be required by default. Because remark is useful by default.

This would be a new option of createLanguageServer defined by remark-language-server.

remark-language-server currently does not expose a createLanguageServer? I don’t think it exposes an API at all? I don’t quite understand what you mean.

Options are part of LSP

Do you mean workspace/configuration? https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspace_configuration

I do think it’s fair to compare with VSCode extensions settings, as many of them simply wrap a language server which defines those settings, just like is the case with this project. Also VSCode represents it’s settings as a JSON file. I just used that, because it’s autocomplete is a convenient way to search for similar settings.

---

This feature is in demand.

I don’t see it. I am talking about defaults. Vim being broken can be solved regardless of what the default is. Vim being solved is in demand.

I miss this feature as an end user using this via both Vim and VSCode. Just like I don’t want to have Prettier format my code in a project without Prettier configuration, I don’t want to have remark format my code in a project without a remark configuration. This is where the requireConfig option comes in.

I have changed my mind about the enabled option. I believe requireConfig should be sufficient, and clients should allow the user to disable a language server integration per workspace.

---

Options are part of LSP

Do you mean workspace/configuration? https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspace_configuration

I think that’s it. This is at a lower level though. I’m still thinking of this feature at a higher level. I’d need to fiddle with this to make sure.

This is how it should work:

VSCode users can define the following in their settings.json:

{
  // …
  "remark.requireConfig": true,
  // …
}

The VSCode extension should declare this configuration option using JSON schema.

Based on eslint.lua, it appears nvim-lspconfig doesn’t need a prefix. nvim-lspconfig users will be able to configure remark-language-server using:

require'lspconfig'.remark_ls.setup{
  settings = {
    requireConfig = true
  }
}

I thought remark-language-server would need to pass a prefix for the configuration options to createLanguageServer, but I don’t think this is necessary. Anyway, if this would be necessary, it means something like this:

  createUnifiedLanguageServer({
+   configurationPrefix: 'remark',  // Previously referenced to as `prefix`
    ignoreName: '.remarkignore',
    packageField: 'remarkConfig',
    pluginPrefix: 'remark',
    processorName: 'remark',
    processorSpecifier: 'remark',
    rcName: '.remarkrc'
  })

In unified-language-server, processDocuments() will read the configuration option requireConfig and if this is true and no configuration file can be found for the given file (using find-up), it will emit empty diagnostics for that file and stop further processing.

---

Personally I would recommend anyone to enable this option, but I don’t really mind what the default is. Also clients can set their own defaults.

I like how requireConfig is passed in VSC and nvim-lspconfig, I think it satisfies the majority of users.

I miss this feature as an end user using this via both Vim and VSCode. Just like I don’t want to have Prettier format my code in a project without Prettier configuration, I don’t want to have remark format my code in a project without a remark configuration. This is where the requireConfig option comes in.

Personally I would recommend anyone to enable this option, but I don’t really mind what the default is. Also clients can set their own defaults.

To reiterate what I try to voice: I am okay with supporting this feature in this language server. It can be the default for Vim, if that is how Vim does things (i.e. if Vim enables every tool ever, and then all those tools throw errors when they are not installed, then we can hide that error on our end). But it must not be the default in VS Code or Atom (or in this project). You reference prettier. Prettier does not default to this option in VS Code or Atom.

It makes sense for tools that do nothing by default (such as ESLint, which does default to this in at least Atom). It doesn’t make sense for tools to do things by default. unified does things by default.

And it makes sense for editors that can only install all tools at once (Vim). But in Atom and VS Code, people choose to install a remark extension. They opt-into remark by installing a remark extension.

I do think it’s fair to compare with VSCode extensions settings, as many of them simply wrap a language server which defines those settings, just like is the case with this project. Also VSCode represents it’s settings as a JSON file. I just used that, because it’s autocomplete is a convenient way to search for similar settings.

I am not opposing such comparison. This issue is in unifiedjs/unified-language-server. It touches on unified-engine too. I am attempting to discuss/understand what needs to happen here while leaving exactly what and how it’ll be implemented in VS Code extensions to the discussions in projects such as remark-language-server and vscode-remark.

It can be the default for Vim, if that is how Vim does things (i.e. if Vim enables every tool ever, and then all those tools throw errors when they are not installed, then we can hide that error on our end).

That's not how things happen in Vim, this was just someone's opinion. Most things in nvim-lspconfig use root_dir with root_pattern (such as elm.json) to detect whether the LS should start. And then it doesn't do anything necessarily, like eslint if you don't have config.

Most things in nvim-lspconfig use root_dir with root_pattern (such as elm.json) to detect whether the LS should start. [...]

Is it correct that your statement describes how nvim but not vim-lspconfig works? I.e., taking this quote:

As it does appears that vim-lsp and vim-lsp-settings allow only filetype server identification by default, this PR: — https://github.com/mattn/vim-lsp-settings/pull/545#issue-1157523832

remark was removed there because vim-lspconfig did not have this feature as I understand it?

And then it doesn't do anything necessarily, like eslint if you don't have config.

Regardless, you seem to describe that nvim currently has the “requireConfig” option, that we are discussing here, built in, correct?

Regardless, you seem to describe that nvim currently has the “requireConfig” option, that we are discussing here, built in, correct?

Sort of, but the LS would always be enabled when it finds a config file and otherwise not. So it's not an option to enable it based on user preference. And it's also only tied to nvim-lspconfig, not vim-lsp-settings, ale, coc or others.

I am not opposing such comparison. This issue is in unifiedjs/unified-language-server. It touches on unified-engine too. I am attempting to discuss/understand what needs to happen here while leaving exactly what and how it’ll be implemented in VS Code extensions to the discussions in projects such as remark-language-server and vscode-remark.

So it seems we only need requireConfig here, and let that trickle down from whatever editor/tool specific wrapper around it requires.

• It is not clear what has to happen in unified-engine. I am open to it getting a new option requireConfig, which probably has to throw without rcName, packageField, and rcPath. It should perhaps/optionally warn when files are passed to unified-engine that do not have configuration. But then another option is needed to hide that warning. How would stdin be affected by this option? Alternatively, it would work more like the ignore options such as silentlyIgnore, say, ignoreUnconfigured How does this affect CLIs (unified-args) and other projects wrapping unified-engine?
• It is not clear what has to happen in unified-language-server. It largely depends on a) what is supported in unified-engine b) how LSP handles workspace configuration. E.g., do VS Code and Vim just magically pass these config files? If there is a “cascade” between workspace and “global”/UI settings, do we need to handle them? Are there keys other than requireConfig that will be supported now/soon?

• It is not clear what has to happen in unified-engine.

Ideally requireOption would be implemented in unified-engine, because it already takes care of resolving the configuration. Alternatively this could be implemented in unified-language-server, but this is less ideal, as this code might run out of sync with unified-engine.

I am open to it getting a new option requireConfig, which probably has to throw without rcName, packageField, and rcPath. It should perhaps/optionally warn when files are passed to unified-engine that do not have configuration. But then another option is needed to hide that warning. How would stdin be affected by this option? Alternatively, it would work more like the ignore options such as silentlyIgnore, say, ignoreUnconfigured

Perhaps requireConfig could be one of false (default), true, or 'warn'.

How does this affect CLIs (unified-args) and other projects wrapping unified-engine?

unified-args could support this as a CLI flag, but personally I think unified-args can just ignore this option.

• It is not clear what has to happen in unified-language-server. It largely depends on a) what is supported in unified-engine b) how LSP handles workspace configuration. E.g., do VS Code and Vim just magically pass these config files?

Yes, these options are received from connection.onDidChangeConfiguration. This is documented in https://code.visualstudio.com/api/language-extensions/language-server-extension-guide#using-configuration-settings-in-the-server

If there is a “cascade” between workspace and “global”/UI settings, do we need to handle them?

This is the responsibility of the client. (I believe they are typically shallow merged.)

Are there keys other than requireConfig that will be supported now/soon?

We could for example allow users to configure remark through configuration. I would use a configuration file, but I wouldn’t be surprised to get a feature request for this. I.e.:

// VSCode settings.json
{
  "remark.requireConfig": false,
  "remark.config": {
    "settings": {
      "bullet": "-"
    },
    "plugins": [
      "remark-gfm",
      "remark-preset-lint-recommended"
    ]
  }
}

-- Neovim init.lua
require'lspconfig'.remark_ls.setup{
  settings = {
    requireConfig = true,
    config = {
      settings = {
        bullet = '-'
      },
      plugins = {
        'remark-gfm',
        'remark-preset-lint-recommended'
      }
    }
  }
}

Other options that come to mind are to disable specific features, such as formatting, diagnostics, or code actions. Perhaps a user wishes to disable message notes, change severity levels, filter diagnostics based on rule name, or alter the diagnostics output in some other way.

I don’t intend to implement any of these options (until there is user demand). I’m just thinking of potential future configuration options.

Looking at the options in unified-engine some more, I feel most for the option being in the “ignore” section and it including “ignore” in the name there, given that files are still found/passed, but then “filtered out” (aka ignored). A name such as requireConfig could be interpreted as that the whole process stops if there is no config file in CWD, ignore* doesn’t have that. An ignore option also plays with well with silentlyIgnore, so that normally an explicitly passed but ignored file is warned about, but optionally it’s silently ignored. ignoreUnconfigured, ignoreWithoutConfig?

---

Yes, these options are received from connection.onDidChangeConfiguration

How does that work with different workspaces? It seems like there’s only one configuration that can be retrieved/listened to, instead of per workspace/file?

We could for example allow users to configure remark through configuration. I would use a configuration file, but I wouldn’t be surprised to get a feature request for this. I.e.:

• This seems to be at odds with this issue: requireConfig (especially when implemented in unified-engine)
• It would make remark/etc unusable on its own (e.g., separately, as remark-cli or so)
• If the goal of doing this is to add a bunch of configs of different tools in one place, people can use package.jsons

---

Rest of the options: :+1:, later.

Looking at the options in unified-engine some more, I feel most for the option being in the “ignore” section and it including “ignore” in the name there, given that files are still found/passed, but then “filtered out” (aka ignored). A name such as requireConfig could be interpreted as that the whole process stops if there is no config file in CWD, ignore* doesn’t have that. An ignore option also plays with well with silentlyIgnore, so that normally an explicitly passed but ignored file is warned about, but optionally it’s silently ignored. ignoreUnconfigured, ignoreWithoutConfig?

I like it! I keep changing my mind whether ignoreUnconfigured or ignoreWithoutConfig is a better name. I suppose either works. :shrug:

---

Yes, these options are received from connection.onDidChangeConfiguration

How does that work with different workspaces? It seems like there’s only one configuration that can be retrieved/listened to, instead of per workspace/file?

My bad, connection.onDidChangeConfiguration also needs to be implemented, but the configuration is retrieved from connection.workspace.getConfiguration. This accepts a scopeUri, which according to the previously linked guide is the document uri, so no need for us to handle this. It’s up to the client to implement this.

---

We could for example allow users to configure remark through configuration. I would use a configuration file, but I wouldn’t be surprised to get a feature request for this. I.e.:

• This seems to be at odds with this issue: requireConfig (especially when implemented in unified-engine)
• It would make remark/etc unusable on its own (e.g., separately, as remark-cli or so)
• If the goal of doing this is to add a bunch of configs of different tools in one place, people can use package.jsons

It’s not an option I would encourage for the same reasons you mentioned. It’s just an example of a possible future configuration I could think of which is a bit more complex than a simple boolean field, so I figured this might give an idea of what such configurations would look like for end users. It’s also effectively what the old VSCode remark extension already did. Just as for the other options, I have no intent to implement this unless there is user demand.

Sweet. Looks like we have a plan! :)

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.