Before, we needed to always pack an entire msgpack_rpc Object to
a continous memory buffer before sending it out to a channel.
But this is generally wasteful. it is better to just flush
whatever is in the buffer and then continue packing to a new buffer.
This is also done for the UI event packer where there are some extra logic
to "finish" of an existing batch of nevents/ncalls. This doesn't really
stop us from flushing the buffer, just that we need to update the state
machine accordingly so the next call to prepare_call() always will
start with a new event (even though the buffer might contain overflow
data from a large event).
Then we can just load metadata in C as a single msgpack blob. Which also
can be used directly as binarly data, instead of first unpacking all the
functions and ui_events metadata to immediately pack it again, which was
a bit of a silly walk (and one extra usecase of `msgpack_rpc_from_object`
which will get yak shaved in the next PR)
Functions like file_open_new() and file_open_fd_new() which just is a
wrapper around the real functions but with an extra xmalloc/xfree around
is an anti-pattern. If the caller really needs to allocate a
FileDescriptor as a heap object, it can do that directly.
FileDescriptor by itself is pretty much a pointer, or rather two:
the OS fd index and a pointer to a buffer. So most of the time an extra
pointer layer is just wasteful.
In the case of scriptin[curscript] in getchar.c, curscript used
to mean in practice:
N+1 open scripts when curscript>0
zero or one open scripts when curscript==0
Which means scriptin[0] had to be compared to NULL to disambiguate the
curscript=0 case.
Instead, use curscript==-1 to mean that are no script,
then all pointer comparisons dissappear and we can just use an array of
structs without extra pointers.
Note: this contains two _temporary_ changes which can be reverted
once the Arena vs no-Arena distinction in API wrappers has been removed.
Both nlua_push_Object and object_to_vim_take_luaref() has been changed
to take the object argument as a pointer. This is not going to be
necessary once these are only used with arena (or not at all) allocated
Objects.
The object_to_vim() variant which leaves luaref untouched might need to
stay for a little longer.
and for return value of nlua_exec/nlua_call_ref, as this uses
the same family of functions.
NB: the handling of luaref:s is a bit of a mess.
add api_luarefs_free_XX functions as a stop-gap as refactoring
luarefs is a can of worms for another PR:s.
as a minor feature/bug-fix, nvim_buf_call and nvim_win_call now preserves
arbitrary return values.
Problem:
Since 2448816956, the --startuptime report shows
two blocks of data. The TUI process and its embedded nvim process write to the
file concurrently, which may interleave the two startup sequences into the same
timeline.
Solution:
Report each process as a separate section in the same file.
1. Each process buffers the full report.
2. After startup is finished, the buffer is flushed (appended) to the file.
Fix#23036
Sample report:
--- Startup times for process: Primary/TUI ---
times in msec
clock self+sourced self: sourced script
clock elapsed: other lines
000.006 000.006: --- NVIM STARTING ---
000.428 000.422: event init
000.728 000.301: early init
...
005.880 000.713: init highlight
005.882 000.002: --- NVIM STARTED ---
--- Startup times for process: Embedded ---
times in msec
clock self+sourced self: sourced script
clock elapsed: other lines
000.006 000.006: --- NVIM STARTING ---
000.409 000.403: event init
000.557 000.148: early init
000.633 000.077: locale set
...
014.383 000.430: first screen update
014.387 000.003: --- NVIM STARTED ---
Remove `export` pramgas from defs headers as it causes IWYU to believe
that the definitions from the defs headers comes from main header, which
is not what we really want.
Problem: Many places in the code use `findoption()` to access an option using its name, even if the option index is available. This is very slow because it requires looping through the options array over and over.
Solution: Use option index instead of name wherever possible. Also introduce an `OptIndex` enum which contains the index for every option as enum constants, this eliminates the need to pass static option names as strings.
FUNC_ATTR_* should only be used in .c files with generated headers.
Defining FUNC_ATTR_* as empty in headers causes misuses of them to be
silently ignored. Instead don't define them by default, and only define
them as empty after a .c file has included its generated header.
Enable all clang-tidy warnings by default instead of disabling them.
This ensures that we don't miss useful warnings on each clang-tidy
version upgrade. A drawback of this is that it will force us to either
fix or adjust the warnings as soon as possible.
Problem: The legacy signlist data structures and associated functions are
redundant since the introduction of extmark signs.
Solution: Store signs defined through the legacy commands in a hashmap, placed
signs in the extmark tree. Replace signlist associated functions.
Usage of the legacy sign commands should yield no change in behavior with the
exception of:
- "orphaned signs" are now always removed when the line it is placed on is
deleted. This used to depend on the value of 'signcolumn'.
- It is no longer possible to place multiple signs with the same identifier
in a single group on multiple lines. This will now move the sign instead.
Moreover, both signs placed through the legacy sign commands and through
|nvim_buf_set_extmark()|:
- Will show up in both |sign-place| and |nvim_buf_get_extmarks()|.
- Are displayed by increasing sign identifier, left to right.
Extmark signs used to be ordered decreasingly as opposed to legacy signs.
Problem: heap-use-after-free in is_qf_win()
Solution: Check buffer is valid before accessing it
fc68299d43
Co-authored-by: Christian Brabandt <cb@256bit.org>
We already have an extensive suite of static analysis tools we use,
which causes a fair bit of redundancy as we get duplicate warnings. PVS
is also prone to give false warnings which creates a lot of work to
identify and disable.
long is 32 bits on windows, while it is 64 bits on other architectures.
This makes the type suboptimal for a codebase meant to be
cross-platform. Replace it with more appropriate integer types.
connection from any channel or stdio will unblock
remote_ui_wait_for_attach. Wait on stdio only if
only —embed specified, if both —embed and
—listen then wait on any channel.
- Move vimoption_T to option.h
- option_defs.h is for option-related types
- option_vars.h corresponds to Vim's option.h
- option_defs.h and option_vars.h don't include each other
Problem:
NVIM_APPNAME does not allow path separators in the name, so relative
paths can't be used:
NVIM_APPNAME="neovim-configs/first-config" nvim
NVIM_APPNAME="neovim-configs/second-config" nvim
Solution:
Let NVIM_APPNAME be a relative path. Absolute paths are not supported.
fix#23056fix#24966
