Problem:
As it turns out, when you have 17 different ways to "exit", you need to
plug 17 different holes.
Leak still reported by ASAN CI, even after 1eae512285f238788601.
ERROR: LeakSanitizer: detected memory leaks
strbuf_init src/cjson/strbuf.c:62:22
json_create_config src/cjson/lua_cjson.c:635:5
lua_cjson_new src/cjson/lua_cjson.c:2060:5
nlua_state_add_stdlib src/nvim/lua/stdlib.c:840:3
nlua_state_init src/nvim/lua/executor.c:973:3
Analysis:
`os_breakcheck()` may process a `exit_event` placed by
`exit_on_closed_chan()` during shutdown. This would re-enter
`preserve_exit()`, which then skips everything because it sees
`really_exiting=true`.
Solution:
Set `exiting` in `preserve_exit()`, not only in `getout()`.
Problem:
SIGTERM received during `os_exit()` can cause it to "nest":
os_exit src/nvim/main.c:720
preserve_exit src/nvim/main.c:910
deadly_signal src/nvim/os/signal.c:205
on_signal src/nvim/os/signal.c:240
signal_event src/nvim/event/signal.c:45
multiqueue_process_events src/nvim/event/multiqueue.c:159
event_teardown src/nvim/main.c:178
os_exit src/nvim/main.c:720
Solution:
Call `signal_reject_deadly()` at the start of `os_exit()` like
`preserve_exit()` already does.
Problem:
The object subsystem has an intermediate representation for no real
reason. Besides the code cost, this also adds an extra (api <=> OptVal)
conversion step, which is a (small) perf cost.
Solution:
We already have `Object`, so use it instead.
- drop `OptVal`, `OptValData`, `OptValType`, and related boilerplate.
- add `kObjectTypeUnset`.
Problem:
Want `gQ` for _le multicursor_.
Solution:
- Don't use `gQ` for exmode.
- Introduce `:exmode`.
- Introduce `[count]q:` as an alias to `:exmode`.
Problem:
POSIX-compatible Ex-mode requires special-cases all over the codebase to
match various quirks that don't actually matter to users.
- The main utility of *interactive* Ex-mode is its REPL behavior, and
that can be achieved with `cmdwin`, which also gains extra UX
benefits.
- The main utility of *non-interactive* `nvim -es` is for shell
scripting, where Ex-mode quirks are mostly unhelpful (e.g. the
"Entering Ex mode" message).
Solution:
- Reimplement *interactive* Ex-mode as a "persistent, insert-mode
cmdwin" in Lua.
- "nvim -e/-E" is simply an alias to "gQ".
- Reframe *non-interactive* Ex-mode (`nvim -es`) as "script mode".
- Drop POSIX Ex-mode quirks.
Improvements:
- "nvim -V1 -es" output ends with a final newline!
- "nvim -V1 -es" no longer shows the "Entering Ex mode" msg. (This was
pointless noise, unwanted for scripting purposes.)
- stdin is no longer typeahead. Scripts (":lua io.read()") can read
stdin as data.
- Empty line is a no-op: a stray blank line no longer moves the cursor
(deviates from POSIX ex "+1"), no longer exits 1 at EOF (E501).
Preserved behavior:
- cursor starts at "$"
- mode()=="cv" (for non-interactive)
- multiline commands (:append/:function/heredoc pull continuation lines)
- bare-range print
- :print=>stdout
- -V1=>stderr
- CRLF input
- continue-after-error and exit codes
Dropped (regressed) POSIX behavior (non-interactive):
- Event loop only ticks while/between commands, not while blocked
waiting for a stdin line.
- ":g/pat/visual...Q"
- input()/getchar()/":s/x/y/c" no longer consume stdin lines as
answers: Nvim stops at end-of-input, skipping the rest of the script,
exit 0. Use ":lua io.read()" instead.
- If users care about this they should use interactive Ex-mode (`gQ`).
- ":@r" stops at end of the register instead of continuing to read
cmdline input from stdin.
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: When a split window is created from the init script using nvim_open_win()
with enter=false, the current window focus is not preserved. The cursor
incorrectly moves into the newly created split during startup.
Solution: Save curwin before the iteration loop in create_windows(), and
restore it after the loop. also update edit_buffers() to use curwin instead
of firstwin.
Problem:
`getchar.c` is a quirky name and doesn't align with our existing
`os/input.c` and `tui/input.c` modules, which encompass the same topic
at different layers.
Solution:
Rename `getchar.c` => `input.c` for discoverability.
Introduce `dialog.c` for functionality related to showing confirm
prompts, dialogs, etc.
Problem:
cmdwin (the `:q` cmdline buffer) has various limitations which require
special-casing all over the codebase.
Besides complicating the code, it also breaks async plugins if they try
to create buffers/windows after some work is done, if the user happens
to open cmdwin at the wrong the moment:
Lua callback: …/guh.nvim/lua/guh/util.lua:531:
E11: Invalid in command-line window; <CR> executes, CTRL-C quits
stack traceback:
[C]: in function 'nvim_buf_delete'
…/guh.nvim/lua/guh/util.lua:531: in function <…/guh.nvim/lua/guh/util.lua:526>
Solution:
Just say no to "inception". Reimplement cmdwin as a normal buffer+window.
All of the cmdwin contortions (in both core, and innocent plugins) exist
literally only to support "inception": recursive
cmdwin-in-cmdline-things, like `<c-r>=`, `/`, search-during-substitute,
`:input()`, etc. So we just won't support that (though I have
a potential plan for that later, which I call "modal parking lot").
The benefit is that plugins, and core, no longer have to care about
cmdwin.
BONUS:
- mouse-drag on vertical separators works (it only worked for
horizontal/statusline before)
- inccommand-in-cmdwin now works correctly, for free (thus don't need
#40077).
POTENTIAL FOLLOWUPS
- Drop `CHECK_CMDWIN` ("E11: Invalid in command-line window"), allow chaos.
- Unify `BUFLOCK_OK` / `LOCK_OK` ?
DESIGN:
- Eliminate lots of C globals, `EX_CMDWIN`, etc.
- `text_locked()` no longer reports true for cmdwin.
- cmdwin = a normal window with 'winfixbuf', 'bufhidden=wipe',
'buftype=nofile'. Invariants come from those options rather than
special cases throughout the codebase.
- `nv_record` for q:/q//q? calls Lua
`nlua_call_vimfn("vim._core.cmdwin", …)`. No `K_CMDWIN`
/ cmdline-reader detour.
- `cedit_key` (`c_CTRL-F`) schedules a deferred event that calls
`vim._core.cmdwin.open(type, content, pos)` and returns `Ctrl_C` so
the in-flight cmdline cancels. Reader state is not serialized; instead
the captured `(type, line, col)` is replayed via
`nvim_feedkeys(type..line.."<CR>", "nt", …)` after user confirms.
- On confirm/cancel: `<CR>` / `<C-C>` calls into Lua which closes the
window and re-feeds the cmdline.
BREAKING CHANGES:
- Expression-register cmdline (`<C-R>=` from insert-mode) no longer
supports cmdwin. Same applies to `input()` / `inputlist()` (already
covered by `text_locked`).
- Usage of cmdwin in macros/mappings will probably break (assuming they
ever worked).
This was originally used because we needed a lua interpreter to run the
standalone _busted_ lua application (except we used fuckery behind the
scenes to fake its binary dependencies with our own binary
dependencies). Now we longer do that, we instead run `test.harness` as a
standard nvim -l script. as this mode is no longer used in tests it
risks going to bitrot anyway.
NB: -ll mode still had some theoretical benefits like quicker
initialization and more "native" printing behavior, etc.
But we can incrementally work on "nvim -l" mode instead
of maintaining a separate special thing.
Problem:
The Lua test harness still ran through standalone -ll mode, so tests
depended on the low-level Lua path instead of the regular Nvim Lua
environment. That also meant os.exit() coverage had to carry an ASAN
workaround because Lua's raw process exit skipped Nvim teardown and let
LeakSanitizer interfere with the observed exit code.
Solution:
Run the harness and related fixtures with nvim -l. Patch os.exit() in
the main Lua state to exit through getout(), so scripts observe normal
Nvim shutdown while standalone -ll remains available for generator-style
scripts. As a consequence, the startup test can assert os.exit() without
disabling leak detection.
AI-assisted: Codex
Problem:
followup to 55ceb314ca#39478
`:oldfiles` and swapfile `:recover` do not delegate to `vim.ui.select`.
Solution:
- Delegate to `vim.ui.select`.
- Fix a long-standing `recover_names` bug where `concat_fnames(dir_name,
files[i], true)` produced malformed `<dir>//<dir>/<file>` paths (also
fixes `swapfilelist()`).
Problem:
`v:starttime`, `:uptime` use a monotonic high-resolution timer. This
only works as long as the timer keeps running (if the computer is
suspended the timer is paused). This is somewhat unintuitive, and
doesn't match the behavior of the `uptime` shell command.
Solution:
Implement `os_realtime` to get the real time since the
epoch in nanoseconds.
Problem:
On Windows, path separators may become inconsistent for various reasons,
which makes normalization quite painful.
Solution:
Normalize paths to `/` at the entry boundaries and always use it
internally, converting back only in rare cases where `\` is really
needed (e.g. cmd.exe/bat scripts?).
This is the first commit in a series of incremental steps.
Note:
* some funcs won't respect shellslash. e.g. `expand/fnamemodify`
* some funcs still respect shellslash, but will be updated in a follow
PR. e.g. `ex_pwd/f_chdir/f_getcwd`
* uv's built-in funcs always return `\`. e.g. `uv.cwd/uv.exepath`
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Problem:
- The `ZR` feature makes it more obvious that we need some sort of flag so that
an `ExitPre` / `QuitPre` / `VimLeave` handler can handle restarts differently
than a normal exit. For example, it's common that users want `:mksession` on
restart, but perhaps not on a normal exit.
- Nvim has no way to report its "uptime".
Solution:
- Introduce `v:starttime`
- Introduce `v:exitreason`
Problem: too many strlen() calls
Solution: refactor concat_fname() and remove calls to strlen()
(John Marriott)
Function `concat_fnames()` can make up to 5 calls to `STRLEN()` (either
directly or indirectly via `STRCAT()`). In many cases the lengths of
arguments `fname1` and/or `fname2` are either known or can simply be
calculated.
This Commit refactors this function to accept the lengths of arguments
`fname1` and `fname2` as arguments. It also adds new argument `ret` to
return the resulting string as a `string_T`.
Additionally:
- function `add_pack_dir_to_rtp()` in `scriptfile.c`:
Use a `string_T` to store local variables `new_rtp` and `afterdir`.
Replace calls to `STRCAT()` with calls to `STRCPY()`.
Change type of variable `keep` to `size_t` for consistency with
other lengths.
- function `qf_get_fnum()` in `quickfix.c`:
Use a `string_T` to store local variables `ptr` and `bufname`
- function `qf_push_dir()` in `quickfix.c`:
Use a `string_T` to store local variable `dirname`.
Replace call to `vim_strsave()` with `vim_strnsave()`.
- function `qf_guess_filepath()` in `quickfix.c`:
Use a `string_T` to store local variable `fullname`.
- function `make_percent_swname()` in `memline.c`:
Rename some variables to better reflect their use.
Use a `string_T` to store local variables `d` and `fixed_name`.
Slightly refactor to remove need to create an extra string.
- function `get_file_in_dir()` in `memline.c`:
Use a `string_T` to store local variables `tail` and `retval`.
Move some variables closer to where they are used.
- function `cs_resolve_file()` in `if_cscope.c`:
Use a `string_T` to store local variable `csdir`.
Remove one call to `STRLEN()`.
- function `add_pathsep()` in `filepath.c`:
Refactor and remove 1 call to `STRLEN()`
- function `set_init_xdg_rtp()` in `option.c`:
Use a `string_T` to store local variable `vimrc_xdg`.
closes: vim/vim#19854cb51add7ae
Co-authored-by: John Marriott <basilisk@internode.on.net>
Co-authored-by: Christian Brabandt <cb@256bit.org>
Problem:
The "restart" event has some problems:
- all UI clients must implement a somewhat complex set of setups
- UI must be on the same machine as the server
- only works for the "current" UI
- race/edge case: If the user config has errors / waiting for input, are
all UIs able to attach while Nvim is waiting for input?
Solution:
- Perform the restart on the server, not the client.
- Pass listen address (instead of CLI args) in the UI event.
- Simplifies UI logic: they only need to attach to new address.
- Opens the door for more enhancements in the future, such as allowing
all UIs to reattach instead of only the "current" UI.
Co-authored-by: zeertzjq <zeertzjq@outlook.com>
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Problem: String handling can be improved
Solution: Pass string length where it is known to avoid strlen() calls,
do a few minor refactors (John Marriott).
This commit changes some calls to function `set_vim_var_string()` to pass
the string length where it is known or can be easily calculated.
In addition:
In `evalvars.c`:
* In function `set_reg_var()` turn variable `regname` into a C string
because that is how it used.
* Small cosmetics.
In `option.c`:
* Slightly refactor function `apply_optionset_autocmd()` to move some
variables closer to where they are used.
In `getchar.c`:
* Slightly refactor function `do_key_input_pre()`:
-> change call to `dict_add_string()` to `dict_add_string_len()` and
pass it the length of `buf`.
-> only call `get_vim_var_string()` once.
In `message.c`:
* Use a `string_T` to store local variable `p`.
In `normal.c`:
* Move some variables closer to where they are used.
closes: vim/vim#19618727f6e2686
Co-authored-by: John Marriott <basilisk@internode.on.net>
This reverts commit ab8371a26c.
Need to think of a different solution, which may require adding new
flags to nvim_ui_attach() (e.g. passing stdout or stderr fd).
Problem:
On FreeBSD, output written to TTY may be lost on exit.
Example test failure:
FAILED
test/functional/terminal/tui_spec.lua @
2521:
TUI no assert failure on deadly signal #21896
test/functional/terminal/tui_spec.lua:2523: Row 1 did not match.
Expected:
|*Nvim: Caught deadly signal 'SIGTERM' |
|* |
|*[Process exited 1]^ |
|* |
|* |
| |
|{5:-- TERMINAL --} |
Actual:
|* |
|*[Process exited 1]{100:^ }|
|*{100:~ }|
|*{100:~ }|
|*{3:[No Name] }|
| |
|{5:-- TERMINAL --} |
To print the expect() call that would assert the current screen state, use
screen:snapshot_util(). In case of non-deterministic failures, use
screen:redraw_debug() to show all intermediate screen states.
Snapshot:
screen:expect([[
|
[Process exited 1]{100:^ }|
{100:~ }|*2
{3:[No Name] }|
|
{5:-- TERMINAL --} |
]])
stack traceback:
test/functional/ui/screen.lua:909: in function '_wait'
test/functional/ui/screen.lua:537: in function 'expect'
test/functional/terminal/tui_spec.lua:2523: in function <test/functional/terminal/tui_spec.lua:2521>
Solution:
Call tcdrain() on stdout and stderr on exit.
This problem is only observed on FreeBSD, but it probably doesn't hurt
to do this on all platforms with termios.h. In fact using tcdrain() on
PTY slave is no-op on Linux according to Linux kernel source code.
Problem: If Nvim server fails to --listen and prints error before the
TUI enters alternate screen, the error isn't visible.
Solution: Forward server stderr using client side stderr handler instead
of having the server inherit client stderr file descriptor.
This does mean that `stderr_isatty` will be `false` in the server, but
that value doesn't matter in embedded mode.
Always pass `stdin_fd` to embedded server to avoid a hang when reading
from stdin when it's a TTY (not sure why one wants to do that, perhaps
by mistake), because if `stdin_fd` isn't passed, the server will try to
use stderr as stdin.
Example test failure on CI:
FAILED test/functional/terminal/tui_spec.lua @ 41: TUI exit status 1 and error message with server --listen error #34365
test/functional/terminal\tui_spec.lua:55: Failed to match any screen lines.
Expected (anywhere): "nvim%.exe: Failed to %-%-listen: address already in use:"
Actual:
|{114:nvim.exe -h"} |
| |
|[Process exited 1]^ |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
|{5:-- TERMINAL --} |
Snapshot:
screen:expect([[
{114:nvim.exe -h"} |
|
[Process exited 1]^ |
|*13
{5:-- TERMINAL --} |
]])
stack traceback:
test\functional\ui\screen.lua:909: in function '_wait'
test\functional\ui\screen.lua:537: in function 'expect'
test/functional/terminal\tui_spec.lua:55: in function <test/functional/terminal\tui_spec.lua:41>
In this case, it appears that the client entered alternate screen in the
middle of the server's print_mainerr().
fix: allocate hidden console for detached server
Starting the server with UV_PROCESS_DETACHED results in DETACHED_PROCESS, leaving the child without a console. Without a console:
CONIN$ / CONOUT$ cannot resolve, causing channel_from_stdio to fail.
ConPTY cannot attach, breaking :terminal.
This patch allocates a hidden console via AllocConsole() when the server has none, restoring working stdio and enabling ConPTY.
Also updates os_set_cloexec to clear HANDLE_FLAG_INHERIT on the RPC pipe
handles, matching the Unix F_DUPFD_CLOEXEC behavior.
Problem:
- `:args` and `argv()` can change after startup.
- `v:arg` includes options/commands, not just files.
- Plugins (e.g. Oil) may rewrite directory args.
Solution:
- New read-only var `v:argf`: snapshot of file/dir args at startup.
- Unaffected by `:args` or plugins.
- Unlike `v:argv`, excludes options/commands.
- Paths are resolved to absolute paths when possible
Example:
nvim file1.txt dir1 file2.txt
:echo v:argf
" ['/home/user/project/file1.txt', '/home/user/project/dir1', '/home/user/project/file2.txt']
Problem:
$NVIM_APPNAME was not respected when searching $XDG_CONFIG_DIRS for
config files. Nvim hardcoded "nvim" when constructing paths like
`$XDG_CONFIG_DIRS/nvim/init.lua`, ignoring the $NVIM_APPNAME environment
variable.
This meant that config files like `$XDG_CONFIG_DIRS/myapp/init.lua` were
not loaded, even though $NVIM_APPNAME was set to "myapp".
Solution:
Use `get_appname()` instead of hardcoded "nvim" for $XDG_CONFIG_DIRS
paths in `do_system_initialization()` and `do_user_initialization()`.
This makes $XDG_CONFIG_DIRS behave consistently with $XDG_CONFIG_HOME,
which already respected $NVIM_APPNAME.
As documented in `runtime/doc/starting.txt` (L1440-L1441):
"In the help wherever `$XDG_CONFIG_…/nvim` is mentioned it is understood
as `$XDG_CONFIG_…/$NVIM_APPNAME`."
See:
43339dee40/runtime/doc/starting.txt (L1440-L1441)
Relates to #37405
Problem:
The E5422 error message was duplicated in two places in main.c, and
hardcoded error strings should be centralized in errors.h for consistency.
Solution:
Add e_conflicting_configs to errors.h and replace both inline error
strings with the centralized constant.
Problem:
`init.lua` files in `$XDG_CONFIG_DIRS` directories were not being sourced during startup, even though the documentation states they should be searched alongside `init.vim`.
See:
e51f5e17e1/runtime/doc/starting.txt (L495-L496)
Solution:
Modify `do_user_initialization()` to search for `init.lua` in each `$XDG_CONFIG_DIRS` directory before falling back to `init.vim`, matching the behavior for `$XDG_CONFIG_HOME`. Also show `E5422` error if both `init.lua` and `init.vim` exist in the same directory.
Fixes#37405
Problem: Exe stack length can be wrong without being detected.
Solution: Add a check when ABORT_ON_INTERNAL_ERROR is defined.
e31ee86859
vim-patch:8.2.3262: build failure when ABORT_ON_INTERNAL_ERROR is defined
Port patch 9.0.1454 for "make formatc".
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem:
Tests that trigger `os_delay` messages may take 1-3 seconds, wasting
build/CI time, since this serves no purpose in tests.
Solution:
- Introduce `msg_delay` for cases where `os_delay` is being used as
a "UI feature".
- Skip `msg_delay` in tests.
Problem:
The execution of startup scripts in parent directories are too late
compared to scripts in current direcctory.
Solution:
Execute all startup scripts with `lua/_core/exrc.lua`.
closes: #35147
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: Old Coverity warning for not checking ftell() return value.
Solution: Check return value of fseek() and ftell().
3df8f6e353
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: The ops.c file is too big.
Solution: Move code for dealing with registers to a new file. (Yegappan
Lakshmanan, closesvim/vim#4982)
4aea03eb87
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem:
Crash on startup in some situations due to call to `set_curbuf`
with the current value of `cur_buf`.
Solution:
Switch buffers before doing the wipeout of the stdin buffer.
Use cmdline cmds instead of raw buffer pointers to avoid lifetime issues.
Problem:
All of Nvim’s threads are clamped to the Default QoS class. This means
that Nvim is forced to compete for CPU time with compilers and other
batch work, and is even prioritized beneath user-initiated work in GUI
apps like e.g. file imports. This significantly harms responsiveness.
Solution:
Tell the kernel that the Nvim process takes part in rendering a UI.
Implementation:
Remove the process-wide QoS clamp. This doesn’t directly do anything to
the main thread, but rather has the side-effect of letting the main
thread run at its actual QoS (User Interactive QoS).
Problem: When stdin follows files, set_curbuf() initializes buffer via
enter_buffer(), then open_buffer() initializes again, causing memory leaks.
Solution: Use readfile() directly for new buffers already initialized by
enter_buffer(), only call open_buffer() for original buffer case.
Problem:
Running `echo dummy | nvim file1 file2` closes the file1 buffer if
file1 doesn't exist.
Solution:
Logic changed in 43e8ec9 such that stdin buffer may now be created *after*
file args. Handle that case.
