Problem:
`FileType directory` fires *before* the `BufEnter` path where `dir.lua`
populates the buffer, in order to allow a 3P dir-browser plugin to handle the
event and (optionally) rename the buffer to e.g. `example:///tmp/foo/`.
But currently, `open_buffer()` continues after `filetypedetect BufRead` as if
the original directory buffer is *still* current and valid, which may fall
through to the built-in `nvim.dir` path after a 3P plugin ALREADY handled the
directory.
Solution:
Keep a buf ref around the early `filetypedetect BufRead` call for directory
buffers.
If the `FileType directory` handler switches away from/deletes the original
directory buffer, stop the open path instead of continuing into the built-in
`nvim.dir` flow.
FAILED …/plugin/pack_spec.lua:92: …/plugin/pack_spec.lua @ 70: vim.pack update() can use lockfile revision as a target
…/plugin/pack_spec.lua:92: …/plugin/pack_spec.lua:70: process still exists
stack traceback:
D:/a/neovim/neovim/test/functional/testnvim/exec_lua.lua:124: in function 'system_sync'
…/plugin/pack_spec.lua:92: in function 'git_cmd'
…/plugin/pack_spec.lua:113: in function 'git_add_commit'
…/plugin/pack_spec.lua:1209: in function <…/plugin/pack_spec.lua:1173>
fix#40167
Problem:
A successful `BufReadCmd` can leave an existing `filetype` without
rerunning its `FileType` setup, so state cleared during reload may not
be restored.
This also causes `:edit`/reload syntax `directoryDirectory` groups to
not be reapplied.
Solution:
After `BufReadCmd` handles a read, trigger `FileType` for the existing
`filetype` if no `FileType` event fired during the read.
Problem:
Continue separators normalization, and try to keep it at the
nvim <-> external boundary, e.g., `fn.xxx`, `api.xxx`, `:xxx`
Solution:
some key changes
- normalize `$HOME-windows`
- normalize entry points of changing directory
- `nvim_set_current_dir()`, `chdir()`
- normalize the named pipe
- `--listen` arg, `--server` arg, `:restart`
- `serverstart()`, `sockconnect()`, `serverstop()`
- make `expand()` respect 'shellslash' again
- clean up `did_set_shellslash`
- replace `forward_slash` with `TO_SLASH`
- remove obsolete `TMP_PATHSEPSTR`
- remove `slash_adjust` in `do_autocmd_dirchanged`?
- make `fnamemodify()` always return `/` (except when substituting
separators via `:s`, `:gs` ?)
Note:
- these funcs still apply `slash_adjust` on return, as before:
`:pwd`, `chdir()`, `exepath()`, `getcwd()`
- these funcs alwarys return `/`, unlike before the normalization PRs
- `getcompletion()`, `finddir()`, `findfile()`
Also
- clean up code, comments, formatting and tests
- move `TO_SLASH` from `f_chdir` to `changedir_func`
- move `TO_SLASH` from `f_bufadd` to `buflist_new`
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Problem:
Malformed server responses with `null` (vim.NIL) values, do not show
clear error messages.
Solution:
Assert two known, specific cases, to avoid user confusion.
Problem:
`vim.filetype.match()` needs a cheap way to recognize directory buffers
without doing filesystem stat work.
Solution:
Ensure full buffer names for directories end in a trailing slash. Now
directory buffers can proceed through the normal 'filetype' path.
Note side-effects: session and ShaDa buffer-list restore behavior must
be compatible, so those + corresponding tests must be updated.
Problem: when an LS client detaches from the buffer, only pull diagnostics
are cleared via capability framework. Push diagnostics remain stuck even
when client stops/restarts.
Solution: clear push diagnostics on client detach.
ref #33864
When a server supports both document and workspace pull diagnostics,
`on_refresh` only dispatched a `workspace/diagnostic` request. The
workspace response handler skips buffers with `pull_kind == "document"`
(i.e. all buffers opened by the user), so their diagnostics went stale
until the next `didChange` or `didOpen` event.
Change `on_refresh` to always refresh document-pull buffers via
`textDocument/diagnostic`, regardless of whether the server also
supports workspace diagnostics. This ensures that opened buffers
see updated diagnostics (e.g. after a save triggers an external
tool like PHPStan) without requiring the user to re-enter insert
mode.
Problem: The codelens LSP module was using its own raw buffer events and
its own debounce mechanism for refreshing code lens in attached buffers.
Solution: Switch the module to using the LspNotify autocmd events.
LspNotify fires just after document versions are synced with the server
and provides a built in debounce mechanism for changes.
Additionally, this fixes some bugs with the previous implementation:
1. The workspace/codeLens/refresh handler re-requested codelens for all
buffers but when the response came back, it forced an extra redraw
after clearing the work the handler had just done.
2. Document synchronization was reworked to be more resilient to
multiple clients providing codelens for a single buffer. The latest
document version is now separately tracked per client (and per
client's lenses per row) instead of for the buffer as a whole. This
allows the on_win() function to properly redraw all codelens even
when different clients' responses for a particular document version
come back at different times.
Problem:
Inlay hints used separate global and per-buffer bufstates tables and
bespoke global autocmds for managing the inlay hint state across buffers
and clients, duplicating the lifecycle logic already provided by the
Capability framework. This caused inconsistencies in how client state
was handled and inlay hint state lifecycle was managed compared to other
LSP features.
Solution:
Replace the ad-hoc bufstate tracking and global autocmds in
vim.lsp.inlay_hint with a proper InlayHint subclass of Capability.
This also refactors the way inlay hint state is managed and fixes bugs I
found while doing this:
1. For each line with inlay hints, the list of the hints along with
whether they have been applied is stored in a current result on the
client state. This allows the on_win decorator to clear all inlay
hints for an old document version once, and then re-add the new
version's hints line-by-line as they are drawn to the screen,
modeling the semantic tokens module.
2. It fixes problems with mixing results from multiple clients attached
to the buffer by fully moving each client's state to its own table.
Previously, only the most recent document version used to populate a
line's inlay hints was stored, but there was no distinction for which
client the hints may have come from. (Fixes#36318)
3. It fixes the workspace/inlayHint/refresh server->client notification
behavior. Previously it would only re-request inlay hints for buffers
currently displayed in a window but would not invalidate them in
non-displayed buffers (or provide any mechanism for those buffers to
re-request at a later time). Model semantic token module here again
by invalidating all buffers, and adding a BufWinEnter autocmd to
refresh hints.
4. Add a mechanism to cancel in-flight requests if a new request for a
newer document version is made before the last one returned
5. Handle stale results by simply dropping them.
Problem:
`vim.lsp.buf.format()` accepts ranges using nvim indexing, where an
end column of -1 means end of line. LSP ranges cannot use that,
which is confusing for things like range formatting.
Solution:
Resolve -1 end columns to the line length before converting the range to
LSP positions.
Problem:
When buffers are by default `nomodifiable`, such as when Nvim starts with
`-M`, the health buffer cannot be updated.
Solution:
Always set `modifiable` before modifying the buffer.
Problem:
The dir.lua "-" mapping cannot be easily overridden (because of autocmd
ordering).
Solution:
- Move it to defaults.lua.
- Also to be extra polite: fall back to builtin `-` motion if the user
disabled the `dir.lua` plugin.
Problem: due to totally async install/update/checkout there is no
guaranteed order of `PackChanged{Pre,}` events across different
plugins. This might lead to conflicts when callback for some "main"
plugin relies on features from "dependency" plugin: i.e. callback for
"main" plugin can trigger before installing/updating "dependency"
plugin. The installation order can be enforced by separate
vim.pack.add() calls, but update/checkout can not.
Solution: Trigger events in bulk independently of async execution:
- `PackChangedPre` before any action for all input plugins in order
they are supplied. It will also trigger even if an action will fail.
- `PackChanged` after all actions finished for all sucessfully
affected plugins in order they are supplied.
This also comes with a couple of side effect changes:
- `PackChangedPre kind=delete` is now also triggered even if the
delete won't be done. This makes it more aligned with `kind=install`
and `kind=update`.
- Force update (`:packupdate!`) and "udpate LSP action" now do two
async steps: download/compute updates and apply them. This also
results in two progress reports.
This is mostly a by-product of the implementation (there has to be
a pre-computation of target revision for all plugins before doing
`PackChangedPre` in bulk before possibly applying an update), but I
kind of like it more this way as it is more explicit of what's going
on. If absolutely not acceptable, there might be some hacks to
mitigate it at least in code action, but I'd keep it like this.
Problem:
dir.lua leaves previously-navigated directory buffers around.
This is fine by default, but users need a simple way to opt out.
Solution:
1. Respect `set hidden` (via `'bufhidden'`) as one way to make
previously-navigated dir buffers from showing up.
2. Document a one-liner to hide these buffers
Problem: plugin/shada.lua does not respect the `+` flag of 'cpoptions'
when writing a *.shada buffer into a different file.
Solution: Check the presence of `+` in 'cpoptions' and whether the
buffer is written to a different file in BufWriteCmd for
*.shada before resetting the 'modified' option of the buffer.
This little defect appeared after plugin/shada.vim was rewritten into
Lua in #34725.
Problem:
Diagnostic tracking used a separate bufstates table and manual
LspDetach/LspNotify autocmd management via _enable()/_refresh(),
duplicating the lifecycle logic already provided by the Capability
framework. This caused inconsistencies in how client attach/detach and
buffer teardown were handled compared to other LSP features.
Solution:
Replace the ad-hoc bufstate tracking and _enable/_refresh pattern in
vim.lsp.diagnostic with a proper Diagnostics subclass of Capability.
This cleans up a few random places in the main lsp module and client
module that were poking the diagnostics. It also fixes some pre-existing
bugs and inconsistencies that were discovered:
- Refresh diagnostics immediately on attach instead of lazily by the
first didOpen/didChange notification
- Fix Capability.active lookup in M.enable() to key by it_bufnr instead
of the filter bufnr
- Set lsp defaults before calling the _text_document_did_open_handler in
Client:on_attach() so defaults are there before any lsp notification
occurs
- Log (and return early) on any error from a diagnostic request result
instead of only returning early for server cancelled errors
Problem:
cmd given as string[] always starts using Nvim's CWD, which is arbitrary.
Solution:
If cmd_cwd is not given, use root_dir as CWD.
BREAKING CHANGE: LSP commands given as string arrays now use `root_dir` as
the process working directory when `cmd_cwd` is unset.
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Problem:
`:edit <dir>` and `nvim <dir>` currently rely on netrw to show local directory
contents.
Solution:
- Provide `filetype=directory`.
- Introduce dir.lua, a small plugin that provides directory listing, opening
items, parent navigation, and refresh.
- `netrw` remains available for `:Explore`, remote paths, archives, and file
operations. To continue
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Some servers register `workspace/didChangeWatchedFiles` watchers for URI
schemes that cannot be watched locally. Skipping the unsupported glob
and keep the rest of the registration batch active.
Problem: LspNotify autocmds were not being triggered for didChange
requests when being used during undo/redo (and possibly other) actions.
autocmds are blocked when calling on_lines() callbacks while doing the
undo/redo action.
Solution: Defer firing the autocmd until after the action is complete.
This is closer to what existed before, but now there's a check in the
deferred function to only fire the autocmd if the client is still
active and the buffer is still attached, if applicable.
Problem: LspNotify never passed a buffer when executing the autocmds, so
buffer-local LspNotify autocmd subscriptions didn't have the correct buf
in the event metadata. It was also wrapped in a schedule() so the actual
autocmd was delayed until after the event loop.
This could result in the wrong buffer receiving the notification if
multiple LspNotify autocmds with buffer filters were added. Only the
"latest" one would actually receive non-buffer-filtered autocmds, not
the matching one. It also caused listeners to receive the notification
"out of sync" with when the notification is actually sent. If a buffer
is being deleted (which fires a textDocument/didClose notification), the
notification is scheduled and fired after the buffer is already gone.
Solution: For LSP notifications that pertain to a particular buffer, set
it when executing the LspNotify autocmds so the callback functions that
are filtered on that buffer will get the correct notifications and the
metadata buf field will be correct. Additionally, there is no need to
wrap the LspNotify callback in vim.schedule when it can be called inline
when the notification to the rpc server is fired.
This is tested by removing now-unnecessary autocmds from semantic tokens
(InsertEnter and BufWinEnter should no longer be necessary now that
requests are fired by LspNotify). Without this fix, simply modifying a
buffer doesn't actually trigger LspNotify correctly, and the test for
that fails.
Problem: When multiline semantic token support was introduced, the loop
that finds the end line for a particular token didn't sanitize the token
length sent back by the LSP server. If the server returned an overflowed
length (near uint32 max), neovim would burn cpu and loop for an
extremely long time while trying to find the "end line" represented by
the massively large token, causing neovim to seemingly hang.
Solution: Stop looping once the calculated end_line reaches the actual
last line of the buffer.
Fixes#36257
Problem:
an empty `{ isIncomplete = true, items = {} }` ends completion
instead of requerying.
Solution:
keep isIncomplete on empty lists and retrigger on keypress while incomplete.
Reset on <C-e> so it doesn't immediately re-query.
Problem: The refactor to use Capability left around some cruft and
semi-broken configuration for debounce.
Solution: Clean up now-unnecessary helper methods and simplify
deprecated ones to pass through to the non-deprecated ones. `debounce`
now defaults to 200 for all buffers but is overridable via the
deprecated start() method, which continues to take the max value
specified for any client attached to the buffer.
If we wish to expose changing the debounce in a non-deprecated way, we
will need to consider a "configuration" function, or even a bespoke
method to set the debounce time on the main metaclass (or provide
options to override for a particular buffer). General configuration of
specific LSP features is an as-of-yet unsolved problem.
Problem: The semantic token module is using its own debounce timer for
the buffer on_lines event. If its internal debounce is shorter than the
changetracking module's debounce, it's possible for semantic token
requests to fire for changed buffers before the textDocument/didChange
notification is sent to the server.
Solution: Trigger semantic token requests from the LspNotify autocmd
when the method is the didChange or didOpen notifications, which
enforces a strict happens-before relationship for the sync change
notification followed by a semantic token request.
Note: There is still an internal debounce mechanism in the semantic
token module to handle other debouncing needs specific to its
functionality, such as debouncing server refresh notifications and
handling WinScrolled events when using range requests.
Problem:
After 400f247397 the undotree test became flaky.
It seems to be caused by some kind of race condition where an entry in the undotree is not added when a change in the buffer happens (adding a bunch of sleep around buffer-change operations seems to fix the issue).
Solution:
Disable autoread plugin in undotree test.
Problem:
Listing individual "skipped" notices for each non-actionable or
NVIM_TEST_INTEG test is noisy and makes the "skipped" list overwhelming
and less meaningful.
Solution:
- Mark non-actionable tests with "N/A".
- Report NVIM_TEST_INTEG tests as a separate 1-line summary.
Problem:
A text edit positioned entirely past the last buffer line, with
newText ending in a newline, leaves a stray blank line: the
past-the-end path appends the trailing empty fragment produced by
vim.split() and does not set has_eol_text_edit, so the end-of-buffer
cleanup is skipped. Formatting servers emit such edits whenever
formatting moves text to the end of a document.
Regression from ec94014cd1 (#20137), which split the past-the-end
fast path off the clamp path that sets the flag.
Solution:
Set has_eol_text_edit in the past-the-end path, like the adjacent
path that clamps end_row.
Problem:
When a server sends workspace/codeLens/refresh while an automatic codelens
request is already scheduled, Nvim ignores the server refresh. This can leave
rendered codelens text stale until another buffer edit triggers a new request.
Solution:
Cancel the pending automatic request and send the server-requested refresh
immediately. This preserves request coalescing while giving explicit server
refreshes priority.
Problem:
PR #38340 prevented messages we receive with id:null from being
incorrectly classified as notifications, but caused us to ignore all
messages with id:null, including requests.
Solution:
Handle requests with id:null. When we receive a request, we only need to
respond based on the `method` and `param`.
(The original so-called `notification_received` in the test was actually
semantically `request_or_notification_received`.)
Problem:
The logic for generating the complete item `info` is spread across
`_lsp_to_complete_items`, the `CompleteChanged` event handler, and
`CompletionResolver:request`. This has previously caused the `info`
shown for resolved (via `completionItem/resolve`) and unresolved items
to differ.
Solution:
Centralise the logic in a new `complete_item_info` function which is now
solely responsible for determining:
1. The `info` to show.
2. The markup kind of the `info`.
3. Whether the `info` is complete.
This simplifies the interaction between the 3 functions mentioned in the
problem:
- `_lsp_to_complete_items` calls `complete_item_info` and passes along
the markup kind and whether the item needs resolving via the complete
item's `user_data`.
- The `CompleteChanged` consumes the markup kind and whether the
item needs resolving from the complete item's `user_data`.
- `CompletionResolver:request`, like `_lsp_to_complete_items` calls
`complete_item_info` again and updates the current `info` if it's
changed.
Problem:
`CompletionItem.detail` is only shown in the info popup if the server
supports `completionItem/resolve`.
Solution:
If the server doesn't support `completionItem/resolve`, prepend the
complete item `info` with `CompletionItem.detail` in a fenced codeblock,
same as we do when the server supports `completionItem/resolve`.
To ensure that completion items are displayed in the same way,
regardless of whether the server supports `completionItem/resolve`, i've
extracted out the test logic from the `selecting an item triggers
completionItem/resolve + (snippet) preview` case so that we can run the
same tests against a server which supports `completionItem/resolve` and
one which doesn't. Hopefully this should prevent the two behaviours
diverging again.
Problem:
If `CompletionItem.documentation` is populated but `detail` is not, then
`detail` is not resolved.
Solution:
Ensure that we resolve a `CompletionItem` if either `detail` or
`documentation` are not populated.
I've also removed `detail` from the popup menu since otherwise it will
be populated in both the popup menu and the info popup after the
`CompletionItem` has been resolved. I think the info popup is the best
place for it anyway as when there is a completion item with a long popup
menu entry (when `detail` is a medium/long function signature for
instance), the whole popup menu gets widened and this steals horizontal
space that could be used to display the `documentation`. Now with
`detail` and `documentation` in the info popup, they share the same
horizontal space. This also aligns with how VSCode, nvim-cmp, blink.cmp,
and mini.nvim display `detail`.
Problem:
When a resolved `CompletionItem` with kind `Snippet` populates
`textEdit` instead of `insertText`, the contents are not previewed.
Solution:
Generate the snippet preview from `textEdit.newText` as well.
