Problem: unnecessary and deprecated diagnostics use their own highlight
groups (`DiagnosticUnnecessary` and `DiagnosticDeprecated`) which
override the typical severity-based highlight groups (like
`DiagnosticUnderlineWarn`).
This can be misleading, since diagnostics about unused variables which
are warnings or errors, are shown like comments, since then only the
`DiagnosticUnnecessary` highlight group is used. Users do not see the
more eye-catching red/yellow highlight.
Solution: Instead of overriding the highlight group to
`DiagnosticUnnecessary` or `DiagnosticDeprecated`, set them in addition
to the normal severity-based highlights.
Problem: `nvim://` scheme feels more like a generalized interface that
may be requested externally, and it acts like CLI args (roughly).
This is how `vscode://` works.
Anything that behaves like an "app" or a "protocol" deserves its own
scheme. For such Nvim-owned things they will be called `nvim-xx://`.
Solution: Use `nvim-pack://confirm#<bufnr>` template for confirmation
buffer name instead of `nvim://pack-confirm#<bufnr>`.
Problem: Lockfile can become out of sync with what is actually installed
on disk when user performs (somewhat reasonable) manual actions like:
- Delete lockfile and expect it to regenerate.
- Delete plugin directory without `vim.pack.del()`.
- Manually edit lock data in a bad way.
Solution: Synchronize lockfile data with installed plugins on every
lockfile read. In particular:
1. Install immediately all missing plugins with valid lock data.
This helps with "manually delete plugin directory" case by
prompting user to figure out how to properly delete a plugin.
2. Repair lock data for properly installed plugins.
This helps with "manually deleted lockfile", "manually edited
lockfile in an unexpected way", "installation terminated due to
timeout" cases.
3. Remove unrepairable corrupted lock data and their plugins. This
includes bad lock data for missing plugins and any lock data
for corrupted plugins (right now this only means that plugin
path is not a directory, but can be built upon).
Step 1 also improves usability in case there are lazy loaded plugins
that are rarely loaded (like on `FileType` event, for example):
- Previously starting with config+lockfile on a new machine only
installs rare `vim.pack.add()` plugin after it is called (while
an entry in lockfile would still be present). This could be
problematic if there is no Internet connection, for example.
- Now all plugins from the lockfile are installed before actually
executing the first `vim.pack.add()` call in 'init.lua'. And later
they are only loaded on a rare `vim.pack.add()` call.
---
Synchronizing lockfile on its every read makes it work more robustly
if other `vim.pack` functions are called without any `vim.pack.add()`.
---
Performance for a regular startup (good lockfile, everything is
installed) is not affected and usually even increased. The bottleneck
in this area is figuring out which plugins need to be installed.
Previously the check was done by `vim.uv.fs_stat()` for every plugin
in `vim.pack.add()`. Now it is replaced with a single `vim.fs.dir()`
traversal during lockfile sync while later using lockfile data to
figure out if plugin needs to be installed.
The single `vim.fs.dir` approach scales better than `vim.uv.fs_stat`,
but might be less performant if there are many plugins that will be
not loaded via `vim.pack.add()` during startup.
Rough estimate of how long the same steps (read lockfile and normalize
plugin array) take with a single `vim.pack.add()` filled with 43
plugins benchmarking:
- Before commit: ~700 ms
- After commit: ~550 ms
Problem: Currently it is possible to have plugin in a "partial install"
state when `git clone` was successfull but `git checkout` was not.
This was done to not checkout default branch by default in these
situations (for security reasons).
The problem is that it adds complexity when both dealing with lockfile
(plugin's `rev` might be `nil`) and in how `src` and `version` are
treated (wrong `src` - no plugin on disk; wrong `version` - "partial"
plugin on disk).
Solution: Treat plugin as "installed" if both `git clone` and
`git checkout` are successful, while ensuring that not installed
plugins are not on disk and in lockfile.
This also means that if in 'init.lua' there is a `vim.pack.add()` with
bad `version`, for first install there will be an informative error
about it BUT next session will also try to install it. The solution is
the same - adjust `version` beforehand.
Problem: Installation confirmation has several usability issues:
- Choosing "No" results in a `vim.pack.add()` error. This was by
design to ensure that all later code that *might* reference
presumably installed plugin will not get executed. However, this
is often too restrictive since there might be no such code (like
if plugin's effects are automated in its 'plugin/' directory).
Instead the potential code using not installed plugin will throw
an error.
No error on "No" will also be useful for planned lockfile repair.
- List of soon-to-be-installed plugins doesn't mention plugin names.
This might be confusing if plugins are installed under different
name.
Solution: Silently drop installation step if user chose "No" and show
plugin names in confirmation text (together with their pretty aligned
sources).
Problem:
When running with `--headless --listen ./hello`, pressing Ctrl-C
doesn’t log anything and doesn’t clean up the socket file.
Solution:
handle SIGINT like SIGTERM.
Problem: completion: crash with fuzzy completion
(Christian Brabandt)
Solution: When completion candidates are gathered from a different
window, and when completing `<c-p>`, linked list should be
sorted only after all items are collected (Girish Palya).
fixes: vim/vim#18752closes: vim/vim#187566437997d83
Co-authored-by: Girish Palya <girishji@gmail.com>
Problem:
Some servers write log to stdout and there's no way to avoid it.
See https://github.com/neovim/neovim/pull/35743#pullrequestreview-3379705828
Solution:
We can extract `content-length` field byte by byte and skip invalid
lines via a simple state machine (name/colon/value/invalid), with minimal
performance impact.
I chose byte parsing here instead of pattern. Although it's a bit more complex,
it provides more stable performance and allows for more accurate error info when
needed.
Here is a bench result and script:
parse header1 by pattern: 59.52377ms 45
parse header1 by byte: 7.531128ms 45
parse header2 by pattern: 26.06936ms 45
parse header2 by byte: 5.235724ms 45
parse header3 by pattern: 9.348495ms 45
parse header3 by byte: 3.452389ms 45
parse header4 by pattern: 9.73156ms 45
parse header4 by byte: 3.638386ms 45
Script:
```lua
local strbuffer = require('string.buffer')
--- @param header string
local function get_content_length(header)
for line in header:gmatch('(.-)\r?\n') do
if line == '' then
break
end
local key, value = line:match('^%s*(%S+)%s*:%s*(%d+)%s*$')
if key and key:lower() == 'content-length' then
return assert(tonumber(value))
end
end
error('Content-Length not found in header: ' .. header)
end
--- @param header string
local function get_content_length_by_byte(header)
local state = 'name'
local i, len = 1, #header
local j, name = 1, 'content-length'
local buf = strbuffer.new()
local digit = true
while i <= len do
local c = header:byte(i)
if state == 'name' then
if c >= 65 and c <= 90 then -- lower case
c = c + 32
end
if (c == 32 or c == 9) and j == 1 then
-- skip OWS for compatibility only
elseif c == name:byte(j) then
j = j + 1
elseif c == 58 and j == 15 then
state = 'colon'
else
state = 'invalid'
end
elseif state == 'colon' then
if c ~= 32 and c ~= 9 then -- skip OWS normally
state = 'value'
i = i - 1
end
elseif state == 'value' then
if c == 13 and header:byte(i + 1) == 10 then -- must end with \r\n
local value = buf:get()
return assert(digit and tonumber(value), 'value of Content-Length is not number: ' .. value)
else
buf:put(string.char(c))
end
if c < 48 and c ~= 32 and c ~= 9 or c > 57 then
digit = false
end
elseif state == 'invalid' then
if c == 10 then -- reset for next line
state, j = 'name', 1
end
end
i = i + 1
end
error('Content-Length not found in header: ' .. header)
end
--- @param fn fun(header: string): number
local function bench(label, header, fn, count)
local start = vim.uv.hrtime()
local value --- @type number
for _ = 1, count do
value = fn(header)
end
local elapsed = (vim.uv.hrtime() - start) / 1e6
print(label .. ':', elapsed .. 'ms', value)
end
-- header starting with log lines
local header1 =
'WARN: no common words file defined for Khmer - this language might not be correctly auto-detected\nWARN: no common words file defined for Japanese - this language might not be correctly auto-detected\nContent-Length: 45 \r\n\r\n'
-- header starting with content-type
local header2 = 'Content-Type: application/json-rpc; charset=utf-8\r\nContent-Length: 45 \r\n'
-- regular header
local header3 = ' Content-Length: 45\r\n'
-- regular header ending with content-type
local header4 = ' Content-Length: 45 \r\nContent-Type: application/json-rpc; charset=utf-8\r\n'
local count = 10000
collectgarbage('collect')
bench('parse header1 by pattern', header1, get_content_length, count)
collectgarbage('collect')
bench('parse header1 by byte', header1, get_content_length_by_byte, count)
collectgarbage('collect')
bench('parse header2 by pattern', header2, get_content_length, count)
collectgarbage('collect')
bench('parse header2 by byte', header2, get_content_length_by_byte, count)
collectgarbage('collect')
bench('parse header3 by pattern', header3, get_content_length, count)
collectgarbage('collect')
bench('parse header3 by byte', header3, get_content_length_by_byte, count)
collectgarbage('collect')
bench('parse header4 by pattern', header4, get_content_length, count)
collectgarbage('collect')
bench('parse header4 by byte', header4, get_content_length_by_byte, count)
```
Also, I removed an outdated test
accd392f4d/test/functional/plugin/lsp_spec.lua (L1950)
and tweaked the boilerplate in two other tests for reusability while keeping the final assertions the same.
accd392f4d/test/functional/plugin/lsp_spec.lua (L5704)accd392f4d/test/functional/plugin/lsp_spec.lua (L5721)
Problem: Changing `src` of an existing plugin cleanly requires manual
`vim.pack.del()` prior to executing `vim.pack.add()` with a new `src`.
Solution: Autodetect `src` change for an existing plugin (by comparing
against lockfile data). If different - properly delete immediately and
treat this as new plugin installation.
Alternative solution might be to update `origin` remote in the
installed plugin after calling `vim.pack.update()`. Although, doable,
this 1) requires more code; and 2) works only for Git plugins (which
might be not the only type of plugins in the future). Automatic
"delete and clean install" feels more robust.
Problem: Plain `vim.pack.add()` calls (with default `opts.load`) does
not fully work if called inside 'plugin/' runtime directory. In
particular, 'plugin/' files of newly added plugins are not sourced.
This is because `opts.load` is `false` during the whole startup, which
means `:packadd!` is used (modify 'runtimepath' but not force source
newly added 'plugin/' files).
This use case is common due to users organizing their config as
separate files in '~/.config/nvim/plugin/'.
Solution: Use newly added `v:vim_did_init` to decide default `opts.load`
value instead of `v:vim_did_enter`.
Problem: WinEnter autocommand may confuse Vim when closing tabpage
(hokorobi)
Solution: Verify that curwin did not change in close_others()
fixes: vim/vim#18722closes: vim/vim#1873361b73b89a3
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem:
Nvim does not recognize URI scheme with numeric characters. While rare, there
are URIs that contain numbers (e.g. [ed2k://](https://en.wikipedia.org/wiki/Ed2k_URI_scheme))
and characters like `+` (e.g. `svn+ssh`). I use it in
[distant.nvim](https://github.com/chipsenkbeil/distant.nvim) to support
multiple, distinct connections using `distant+1234://` as the scheme.
Otherwise, if you open a file with the same name & path on two different
machines from the same Nvim instance, their buffer names will conflict
when just using `distant://`.
Solution:
Adds full support for detecting URI scheme per
[RFC3986](https://www.rfc-editor.org/rfc/rfc3986#section-3.1)
Problem: :breaklist accepts unprocessed arguments.
Solution: Remove EX_EXTRA flag from the Ex command definition.
(Doug Kearns)
The command should emit an "E488: Trailing characters" error rather than
silently accept arguments.
closes: vim/vim#18746de7049ede1
Co-authored-by: Doug Kearns <dougkearns@gmail.com>
Problem: runtime(netrw): LocalBrowseCheck() wipes unnamed buffers when
g:netrw_fastbrowse=0 (Carlos Falgueras García)
Solution: Check that bufname() is not empty
fixes: vim/vim#18740closes: vim/vim#18741384685fade
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: Error message with :unlet! and non-existing dictionary item
(Coacher)
Solution: Set GLV_QUIET when using unlet with bang attribute
fixes: vim/vim#18516closes: vim/vim#18734b8119920eb
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem: reuse_win will always jump to the first window containing the
target buffer rather even if the buffer is displayed in the current
window/tab
Solution: check to see if the buffer is already displayed in the
current window or any window of the current buffer
Problem:
- Exposing the raw config as table is a pattern not seen anywhere else
in the Nvim codebase.
- Old spellfile.vim docs still available, no new documentation
Solution:
- Exposing a `config()` function that both acts as "getter" and "setter"
is a much more common idiom (e.g. vim.lsp, vim.diagnostic).
- Add new documentation and link old docs to |spellfile.lua| instead of
|spellfile.vim|.
* feat(lua): `Range:is_empty()` to check vim.range emptiness
* fix(lsp): don't overlay insertion-style inline completions
**Problem:** Some servers commonly respond with an empty inline
completion range which acts as a position where text should be inserted.
However, the inline completion module assumes that all responses with a
range are deletions + insertions that thus require an `overlay` display
style. This causes an incorrect preview, because the virtual text should
have the `inline` display style (to reflect that this is purely an
insertion).
**Solution:** Only use `overlay` for non-empty replacement ranges.
Problem: Grid size check didn't account for border_width, causing
row index out of bounds when drawing bordered popup menu.
Solution: Check grid.rows against pum_height + border_width.
Problem: Resize events during startup may clear an active external
cmdline, which is then not redrawn.
UI2 VimResized autocommand does not work.
UI2 message appearance may be altered by inherited window
options. The message separator uses the wrong fillchar.
Solution: Unset cmdline_was_last_redrawn when clearing the screen, such
that cmdline_show is re-emitted.
Ensure set_pos function is called without arguments.
Ensure such options are unset. Use 'fillchars'->msgsep.
**Problem**:
Currently, whenever user get's on prompt-text area we move the user to
end of user-input area. As a result when left/c-left,home keys are
triggered at start of user-input the cursor get's placed at end of
user-input. But this behavior can be jarring and unintuitive also it's
different from previous behavior where it'd just stay at start of
input-area. Also, previously when insert-mode was triggered in
prompt-text with n_a for example then cursor was placed at start of
user-input area not at the end. So, that behavior was also broken.
**Solution:**
Restore previous behavior. Don't force user to end of user-input when
entering insert-mode from readonly section.
Problem: Confirmation buffer is named with `nvim-pack://` as scheme
prefix and uses buffer id (needed for in-process LSP) as one an entry
in the "hierarchical part".
Solution: Use `nvim://pack-confirm#<buf>` format with a more ubiquitous
`nvim://` prefix and buffer id at the end as the optional fragment.
Problem: In some areas plugin's revision is named "state". This might be
confusing for the users.
Solution: Consistently use "revision" to indicate "plugin's state on
disk".
Problem: Using abbreviated version of commit hashes might be unreliable
in the long term (although highly unlikely).
Solution: Use full hashes in lockfile and revision description (in
confirmation buffer and log). Keep abbreviated hashes when displaying
update changes (for brevity).
Problem: A plugin does not know when startup scripts were already
triggered. This is useful to determine if a function is
called inside vimrc or after (like when sourcing 'plugin/'
files).
Solution: Add the v:vim_did_init variable (Evgeni Chasnovski)
closes: vim/vim#18668294bce21ee
Nvim has two more steps between sourcing startup scripts and loading
plugins. Set this variable after these two steps.
Co-authored-by: Evgeni Chasnovski <evgeni.chasnovski@gmail.com>
Problem: In visual mode, g<End> does not move to the last non-blank
character when the end of a line is on the same line as the
cursor (after v9.0.1753)
Solution: Move the cursor back by one position if it lands after the
line (varsidry)
fixes: vim/vim#18657closes: vim/vim#18658adc85151f3
Problem:
In socket_connect(), if connecting to the given TCP address times out,
libuv is still trying to connect to the address, and connect_cb may be
called when running the libuv event loop after the `status` variable
referenced by `req.data` goes out of scope.
Solution:
Close the uv_tcp_t handle and wait for connect_cb to be called before
retrying or failing in socket_connect(). This also avoid leaking libuv
handles.
The tests added here only check that the non-timeout case still works,
as checking the timeout case is very hard without modifications to the
code. Removing the first LOOP_PROCESS_EVENT_UNTIL() in socket_connect()
(the one with the timeout) is a way to check that manually.
Also add a comment about the cause of the ASAN error in #34586.
Problem:
On MSWIN, file completion (CTRL-X CTRL-F) only works for the current
drive (so not for actual absolute paths), since drive letters are never
included in the completion pattern.
e.g. when completing "F:\Hello" Nvim currently completes "\Hello"
which is relative to the current drive/volume.
vim solves this by adding ':' to the default 'isfname' value on mswin,
but that causes issues as ':' is not a valid windows path char anywhere
_except_ after the drive letter.
Solution:
detect drive letters in front of the path when creating the completion
pattern.
Problem: %P in 'statusline' doesn't behave as documented
(after 9.1.1479).
Solution: Make the percentage 3-chars wide when not translated.
(zeertzjq)
fixes: vim/vim#18669closes: vim/vim#1867173a0de4a04
Co-authored-by: Christ van Willegen <cvwillegen@gmail.com>
Problem: regression when displaying localized percentage position
(after v9.1.1291)
Solution: calculate percentage first (Emir SARI)
Cleanups made in ec032de broke the Turkish percent display, failing to
prepend it properly in cases between 0 and 10. In Turkish, the percent
sign is prepended to the number, so it was displaying it as `% 5`
(should have been `%5`), while displaying numbers bigger than 9 properly.
related: vim/vim#175978fe9e55a7d
The test was unskipped in Vim in patch 9.1.1479 which added Turkish
translation for "%d%%". However, Nvim has had Turkish translation for
"%d%%" since 2023, so don't skip the test.
Co-authored-by: Emir SARI <emir_sari@icloud.com>
compile time features are hot again.
Note: this changes the &term value for builtin definition from
'builtin_xterm' to just 'xterm'. It's an xterm regardless of we use an
external definition or an internal. Prior to this commit the vast
majority of POSIX users will have used external terminfo, so plugins and
scripts are only going to have checked for &term == 'xterm' or 'tmux' or
whatever.
The status of external loading is still available in "nvim -V3" output.
