Problem: close_buffer() callers incorrectly handle b_nwindows,
especially after nasty autocmds, allowing it to go
out-of-sync. May lead to buffers that can't be unloaded, or
buffers that are prematurely freed whilst displayed.
Solution: Modify close_buffer() and review its callers; let them
decrement b_nwindows if it didn't unload the buffer. Remove
some now unneeded workarounds like 8.2.2354, 9.1.0143,
9.1.0764, which didn't always work (Sean Dewar)
(endless yapping omitted)
related: vim/vim#19728bf21df1c7b
b_nwindows = 0 change for free_all_mem() was already ported.
Originally Nvim returned true when b_nwindows was decremented before the end was
reached (to better indicate the decrement). That's not needed anymore, so just
return true only at the end, like Vim. (retval isn't used anywhere now anyways)
Set textlock for dict watchers at the end of close_buffer() to prevent them from
switching windows, as that can leave a window with a NULL buffer. (possible
before this PR, but the new assert catches it; added a test)
Despite textlock, things still aren't ideal, as watchers may observe the buffer
as unloaded and hidden (b_nwindows was decremented), yet still in a window...
Likewise, for Nvim, wipe_qf_buffer()'s comment may not be entirely accurate;
autocmds are blocked, but on_detach callbacks (textlocked) and dict watchers may
still run. Might be problematic, but those aren't new issues.
Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Problem
Unlike inlay hints, code lenses are closely related to running commands;
a significant number of code lenses are used to execute a command (such
as running tests). Therefore, it is necessary to provide a default
mapping for them.
Solution
Add a new default mapping "grx" (mnemonic: "eXecute", like "gx").
Problem:
Right-click menu fails with E335 when using V in Insert mode (after
i_ctrl-o). The mode detection checks restart_edit before VIsual_active,
incorrectly selecting Insert mode binding even when Visual mode is
active.
Solution:
Check Visual mode before Insert mode, to match get_menu_mode() priority
order.
Problem:
"Sorry" in a message (1) is noise, and (2) actually reduces the clarity
of the message because the titlecasing of "Sorry" distracts from the
actually important part of the message.
Solution:
Drop "Sorry" from messages.
Problem:
`K` in help files may fail in some noisy text. Example:
(`fun(config: vim.lsp.ClientConfig): boolean`)
^cursor
Solution:
- `:help!` (bang, no args) activates DWIM behavior: tries `<cWORD>`,
then trims punctuation until a valid tag is found.
- Set `keywordprg=:help!` by default.
- Does not affect `CTRL-]`, that is still fully "tags" based.
Problem: :terminal CWD restoration test may lead to an error log if
after_each() runs before the PTY process calls chdir().
Solution: Wait for some time before wiping the buffer, which can also
prevent SIGHUP being sent to the parent.
Problem:
When a terminal process exits, "[Process Exited]" text is added
to the buffer contents.
Solution:
- Return `exitcode` field from `nvim_get_chan_info`.
- Show it in the default 'statusline'.
- Show exitcode as virtual text in the terminal buffer.
Define a CMake target for every subdirectory of test/functional that
contains functional tests, and a functionaltest_parallel target that
depends on all those targets, allowing multiple test runners to run in
parallel.
On CI, use at most 2 parallel test runners, as using more may increase
system load and make tests unstable.
Problem:
bw_rest was used as an extra buffer to save incomplete byte sequences
between calls to buf_write_bytes. Besides being unnecessarily
complicated, this introduced a number of issues:
1) The bytes stored in bw_rest could still be there at the end of
writing the file, never having been written, thus losing some of the
file content on write.
2) bw_rest was not cleared out after the "checking_conversion" phase,
leaving them to affect the written file content during the writing
phase, corrupting the file.
3) bw_rest could contain extra bytes that need to be written to the
output buffer during a buf_write_convert call, potentially before any
bytes are consumed. But some conversions are in-place, without a
separate output buffer. Writing bytes from bw_rest to the "output"
buffer actually overwrote bytes from the input buffer before they were
read, corrupting the data to be written.
4) The extra bytes in bw_rest that need to be written to the conversion
output buffer were not originally accounted for in the size calculation
for the output buffer, causing a buffer overflow (previously fixed in
Vim patch 9.1.2028).
Solution:
Rather than maintaining a separate buffer, the unconverted bytes at the
end of the buffer can just be shifted to the beginning of the buffer,
and the buffer size updated. This requires a bit of refactoring, and
buf_write_convert and buf_write_convert_with_iconv need to report the
number of bytes they consumed so that buf_write_bytes can handle the
remaining bytes.
Following conversion, bw_buf can be checked for any remaining bytes.
Leftover bytes in this case result in a conversion error, which is
better than silently dropping them.
A short section of dead code was removed from buf_write_convert, for
converting a non-UTF-8 buffer to UTF-8. Neovim buffers are always UTF-8.
A few additional tests for iconv conversions have been added. Vim's
iconv tests are disabled in Neovim because they use unsupported values
for 'encoding'.
TermCursor already has cterm=reverse. Additionally, now that terminal
buffers have a real cursor, the cterm=reverse in TermCursor no longer
shows up in the screen state.
Problem:
Escaping logic for {subject} in ex cmd `:help {subject}` is done in a
messy 200+ lines C function which is hard to maintain and improve.
Solution:
Rewrite in Lua. Use `string.gsub()` instead of looping over characters
to improve clarity and add many more tests to be able to confidently
improve current code later on.
Problem: Newlines intended to write messages below the cmdline or to
mark the start of a new message on message grid are emitted
through ext_messages. This results in unnecessary newlines for
a UI that has decoupled its message area from the cmdline.
msg_col is set directly in some places which is not transmitted
to msg_show events.
Various missing message kind for list commands.
Trailing newlines on various list commands.
Solution: Only emit such newlines without ext_messages enabled.
Use msg_advance() instead of setting msg_col directly.
Assign them the "list_cmd" kind.
Ensure no trailing newline is printed.
Problem:
Temporary files from /tmp/ and /private/ paths clutter :oldfiles list.
Additionally, the documented Windows default (rA:,rB:) was never applied
due to a missing platform condition.
Solution:
Drop platform-specific shada differences and default to excluding
/tmp/ and /private/ paths.
Problem:
On Windows, writing to a pipe doesn't work if the pipe isn't connected
yet. This causes an RPC request to a session newly created by connect()
to hang, as it's waiting for a response to a request that never reaches
the server.
Solution:
Wait for uv.pipe_connect() callback to be called when using connect().
Problem:
Can't use `:source` to run a Lua codeblock (treesitter injection) in
a help (vimdoc) file.
Solution:
Use treesitter to parse the range and treat it as Lua if detected as
such.
Problem:
`:lsp restart` detects when a client has exited by using the `LspDetach`
autocommand. This works correctly in common cases, but breaks when
restarting a client which is not attached to any buffer. It also breaks
if a client is detached in between `:lsp restart` and the actual
stopping of the client.
Solution:
Move restart logic into `vim/lsp/client.lua`, so it can hook in to
`_on_exit()`. The public `on_exit` callback cannot be used for this, as
`:lsp restart` needs to ensure the restart only happens once, even if
the command is run multiple times on the same client.
Problem:
We want to encourage implementing core features in Lua instead of C, but
it's clumsy because:
- Core Lua code (built into `nvim` so it is available even if VIMRUNTIME
is missing/invalid) requires manually updating CMakeLists.txt, or
stuffing it into `_editor.lua`.
- Core Lua modules are not organized similar to C modules, `_editor.lua`
is getting too big.
Solution:
- Introduce `_core/` where core Lua code can live. All Lua modules added
there will automatically be included as bytecode in the `nvim` binary.
- Move these core modules into `_core/*`:
```
_defaults.lua
_editor.lua
_options.lua
_system.lua
shared.lua
```
TODO:
- Move `_extui/ => _core/ui2/`
Problem: It is desirable for the default statusline to contain colored
diagnostics information. However, current `StatusLine` group is
purposefully defined as almost inverted `Normal` to "make current
window obvious". This makes diagnostic information from
`vim.diagnostic.status()` barely visible: it uses established
`DiagnosticSignXxx` groups which have colored foreground with
lightness close to `StatusLine` background.
Also the `StatusLineNC` group is fairly different from `Normal` in
order to both "makes window separators clear" and "be different from
`CursorLine`". But not as mush different as `StatusLine` because
"`StatusLine` and `StatusLineNC` should be clearly different".
Solution: Make both `StatusLine` and `StatusLineNC` be slightly closer
in lightness to `Normal`. This makes `StatusLine` and `StatusLineNC`
groups satisfy their conditions in the following way:
- `vim.diagnostic.count()` is readable on `StatusLine` - yes.
- `vim.diagnostic.count()` is readable on `StatusLineNC` - yes.
- `StatusLine` makes current window obvious - I'd say yes.
- `StatusLine` and `StatusLineNC` are clearly different - it depends
on the eyes and monitor. The current is clearly better, but the new
ones I'd say are still visibly different.
- `StatusLineNC` makes window separators clear - I'd say yes, but
depends on the eyes and monitor.
- `StatuslineNC` is different from `CursorLine` - NO, they are same.
Another approach to solve this would be to introduce dedicated
`DiagnosticStatuslineXxx` groups to use in `vim.diagnostics.status()`.
They can be defined using foreground colors from the same lightness as
`Normal`. This would make them readable in `StatusLine`. But not
`StatusLineNC`, though.
Problem: MS-Windows: Relative import in a script sourced from a buffer
doesn't work (Ernie Rael)
Solution: Set a filename, so that we are not trying to use
script-relative filename (Yegappan Lakshmanan)
When a script is sourced from a buffer, the file name is set to ":source
buffer=". In MS-Windows, the ":" is a path separator character (used
after a drive letter). This results in the code trying to use the ":"
prefix to import the script on MS-Windows. To fix this, when importing a
script from a script sourced from a buffer with nofile, don't use
a script relative path name.
fixesvim/vim#14588closes: vim/vim#14603f135fa28e4
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Problem: :trust is executed even when inside false condition.
Solution: Make skip_cmd() return true for CMD_trust, as ex_trust() does
not handle eap->skip itself.
Problem: :update should write new file buffers, but previous fix
affected special buffer types (acwrite, nofile, etc.).
Solution: Add bt_nofilename() check to only write new files for
buffers representing real filesystem paths.
Problem: update command does not write new buffers that have
filenames but no corresponding file on disk, even when using ++p flag.
Solution: allow update to write when buffer has filename but file
doesn't exist.
Use only a single clear() call in some test/functional/vimscript/ test
files whose test cases have very little side effect.
A downside of using a single clear() is that if a crash happens in one
test case, all following test cases in the same file will also fail, but
these functionalities and tests don't change very often.
Problem: The swapfile attention message is not repeated after clearing
the screen.
After clearing the screen `msg_scrolled` is reset without
clearing other related variables, causing an assert.
Solution: Make the attention message part of the confirm prompt.
Call `msg_reset_scroll()`.
Problem: Listing submenus with :menu doesn't work.
Solution: Don't go to the parent of the return value of find_menu(), and
handle empty path at the caller.
Related #8194, which actually only fixed the problem for menu_get(), not
for :menu Ex command.
NEW BUILD SYSTEM!
This is a MVP implementation which supports building the "nvim" binary,
including cross-compilation for some targets.
As an example, you can build a aarch64-macos binary from
an x86-64-linux-gnu host, or vice versa
Add CI target for build.zig currently for functionaltests on linux
x86_64 only
Follow up items:
- praxis for version and dependency bumping
- windows 💀
- full integration of libintl and gettext (or a desicion not to)
- update help and API metadata files
- installation into a $PREFIX
- more tests and linters
