Skip to content

[Neovim] Handle textDocument/inactiveRegions#402

Open
evanwporter wants to merge 5 commits into
hudson-trading:mainfrom
evanwporter:neovim-inactive
Open

[Neovim] Handle textDocument/inactiveRegions#402
evanwporter wants to merge 5 commits into
hudson-trading:mainfrom
evanwporter:neovim-inactive

Conversation

@evanwporter

@evanwporter evanwporter commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

This adds the inactiveRegions capability (introduced in #287) to the neovim plugin.

We should still merge #352 since some people might not have the plugin, and some editors may not have support for this (ie: zed).

It works in basic cases, but there is an error when deleting many lines.

Feedback would be appreciated, since this is my first time doing substantial work w/ Neovim.

FYI: This PR is bootstrapped by AI.

Images:

image


local ns = vim.api.nvim_create_namespace("SlangServerInactiveRegions")

vim.api.nvim_set_hl(0, "SlangServerInactiveRegion", {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i thought about moving this to the highlight lua file, but I was having issues with the load order when I did

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

highlights.lua is for sure where this should live, with the value ideally pulled from config.lua like the other highlight groups.

I'm not sure what load order problems there could be if you local highlights = require('slang-server._core.highlights') above. If you want to share specifics of the errors hit then we can debug.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

init.lua only gets executed when calling require('slang-server').setup({}), which may not ever happen, depending on the installation method (particularly after #399).

Prior to this, all plugin functionality has been lazy-loaded on the first :SlangServer invocation, but this is a new type of behaviour. The eager execution path when opening an SV buffer is ftplugin/systemverilog.lua, so the handler will have to be registered somewhere under there.

Also, while this init.lua is currently light, I'd prefer not to require('slang-server') under ftplugin/systemverilog.lua as it leaves a possible future footgun for more code to creep into the eager execution path.

I think the best approach is to move the whole handler attachment into inactive_regions.lua and require('slang-server._lsp.experimental.inactive_regions') in ftplugin/systemeverilog.lua.

Comment on lines +15 to +28
-- note: this config requires the slang_server to be explicitly named slang_server
vim.lsp.config("slang_server", {
capabilities = {
experimental = {
inactiveRegions = {
inactiveRegions = true,
},
},
},

handlers = {
["textDocument/inactiveRegions"] = inactive_regions.handler,
},
})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this races with the LSP initialisation. Once the slang-server LSP client has started, updating its capabilities retroactively won't have any effect, and we can't guarantee when this plugin will be loaded relative to the client.

Possible solutions:

  • Document that the user needs to add this to their own LSP config if they want to use this feature.
  • If the client is already running, update its capabilities and notify the server with "workspace/didChangeConfiguration" (if that's supported, @AndrewNolte?)

The former is obviously less than ideal. Thoughts, @toddstrader?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Incidentally the handler can be added after the client has started; it just has to be added to local client = vim.lsp.get_clients("slang_server") instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants