Problem: using int as bool
Solution: refactor: use bool type for internal flags in buf_T
(Hirohito Higashi)
Change the type of 23 internal state flag fields in buf_T from int
to bool for improved type clarity and code readability.
These fields are pure boolean flags that are never accessed via the
option system's varp (which uses *(int *)varp = value), never compared
with int fields holding non-0/1 values, and never use tristate values.
Converted fields:
- State flags: b_dev_valid, b_saving, b_mod_set, b_new_change,
b_marks_read, b_modified_was_set, b_did_filetype, b_keep_filetype,
b_au_did_filetype, b_u_synced, b_scanned, b_p_initialized
- Characteristic flags: b_has_textprop, b_may_swap, b_did_warn,
b_help, b_spell, b_shortname, b_has_sign_column, b_netbeans_file,
b_was_netbeans_file, b_write_to_channel, b_diff_failed
All TRUE/FALSE assignments to these fields have been updated to
true/false accordingly. The type of temporary save variables
(e.g. help_save in tag.c) has also been adjusted to bool.
Option value fields (b_p_XXX) are kept as int because they are
accessed via the option system and some use tristate (-1) semantics.
Fields compared with int option values (b_start_eof, b_start_eol,
b_start_bomb) are also kept as int to preserve comparison integrity.
closes: #20020
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
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: #19854
Co-authored-by: Christian Brabandt <cb@256bit.org>
Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Unnecessary memory allocation in set_callback(); after
set_callback(), callers must manually free the source
callback's name if cb_free_name is set.
Solution: Refactor set_callback() to re-use the callback name when
possible to avoid extra memory allocations and clean up so the
callers do not have to take care themselves (Shane Harper).
closes: #19831
Signed-off-by: Shane Harper <shane@shaneharper.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>
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)
close_buffer() now doesn't decrement b_nwindows when not unloading buf, or when
buf isn't w_buffer after autocmds (they would've already decremented it).
Callers are now expected to decrement b_nwindows if w_buffer is not NULL after
close_buffer(), and when still intending to switch buffers or close win, for two
reasons:
- close_buffer() autocmds may have switched buffers. The new w_buffer's
b_nwindows would also need decrementing.
- After close_buffer(), callers may opt to not switch w_buffer or close win.
b_nwindows would need to be incremented again. (unless w_buffer is NULL from
being unloaded; callers are already forced to find a new buffer then)
These were the main causes of b_nwindows bugs, as these cases could not be
reliably detected, and went largely unhandled.
NOTE: if close_buffer() autocmds switch buffers, close_buffer() is not called
for that new buffer before decrementing b_nwindows. This may skip side-effects
like from 'bufhidden', but I think it's mostly harmless, and was already
happening in other places.
Let's see how this goes... Other details: (I have lots to say!)
It's OK to pass a win to close_buffer() that isn't showing buf (used by
set_curbuf()). In that case, we skip some side-effects and don't decrement
b_nwindows, but may still unload buf if hidden.
buf_freeall() now returns whether it freed anything. Removes some repeated
checks in close_buffer().
Preserve close_buffer()'s behaviour when called by win_free_popup() after its
popup was already removed from the window list. This made win_valid_any_tab()
return FALSE, so we skip things that originally checked it in that case.
Add "set_context" to close_buffer() to preserve do_ecmd()'s behaviour of only
setting b_last_cursor and/or calling buflist_setfpos() when not splitting
(see 7.2.041: https://groups.google.com/g/vim_dev/c/ZGgNvaylNzI/m/WHxjhnuxqB0J)
Without this, Test_marks_cmd() fails from its ' mark differing. Don't use
oldwin though; it's not always the window with the closed buf, especially
after BufLeave autocmds in do_ecmd(). Also, only set context if win is really
displaying buf.
Don't bail in do_ecmd() if buf was deleted but curwin->w_buffer is NULL; that
leaves curwin open to a NULL buffer! Use lastbuf instead, like set_curbuf().
I don't think it's possible for buf to be deleted by close_buffer() anyway, as
b_locked was set (which I can't see a way to bypass, unlike b_locked_split).
Maybe such checks can be removed, but I'd rather not risk that here.
Don't set curwin to previouswin in set_curbuf(); shouldn't be needed, otherwise
may lead to curbuf != curwin->w_buffer if autocmds switch to a window showing
buf, as that skips enter_buffer()? Was introduced back in 7.3.557 to avoid
cases where autocmds switch windows, possibly leaving previouswin with a NULL
buffer. Since 7.4.2312 and 7.4.2328, close_buffer() and buf_freeall() already
handles this. I've added an assert() as a sanity check anyway.
In free_all_mem(), set b_nwindows to 0 before close_buffer() so buffers can be
wiped if still in a window before win_free_all(). Needed as close_buffer() now
skips unloading buffers that aren't hidden if win is NULL. If it's possible for
free_all_mem()'s :tabonly! and :only! to not close all windows before freeing,
then this issue was also previously possible if b_nwindows > 1.
related: #19728
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Crash when reading quickfix line (Kaiyu Xie)
Solution: Make sure line is terminated by NUL
closes: #19667
Supported by AI
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Memory leak in qf_push_dir() (after v9.2.0091)
Problem: free dirname, if it is not a directory.
closes: #19552
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Inefficient use of ga_concat()
Solution: Use ga_concat_len() when the length is already known to avoid
use of strlen() (John Marriott).
Additionally the following changes are done:
os_unix.c:
- in function `socket_server_list_sockets()` use a `string_T` for the
strings `buf` and `path` for use in `ga_concat_len()`
and drop un-needed variable `dir`.
quickfix.c:
- in function `qf_jump_print_msg()` use a `string_T` for the string
`IObuff` for use in `ga_concat_len()`.
- in function `qf_range_text()` use a `string_T` for the string `buf`
for use in `ga_concat_len()`.
register.c:
- simplify function `execreg_line_continuation()`.
terminal.c:
- in function `read_dump_file()` use a `string_T` for the
string `prev_char` for use in `ga_concat_len()`.
tuple.c:
- in function `tuple_join_inner()` use a `string_T` for the
string `s` for use in `ga_concat_len()`. Also, change local struct
`join_T` to use `string_T`.
vim9type.c:
- in functions `type_name_tuple()` and `type_name_func()`
use a `string_T` for the string `arg_type` for use in
`ga_concat_len()`.
closes: #19038
Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: After :botright copen and closing the quikfix window, the
cursor ends up in the wrong window. The problem is fr_child
always points to the first (leftmost for FR_ROW, topmost for
FR_COL) child frame. When do :vsplit, the new window is
created on the left, and frame_insert() updates the parent's
fr_child to point to this new left window.
Solution: Create a snapshot before open the quickfix window and restore
it when close it (glepnir).
closes: #18961
Signed-off-by: glepnir <glephunter@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Generating prototype files does not work on all platforms
Solution: Rework prototypes generation using python instead of cproto,
enable it in CI to test it for each PR (Hirohito Higashi).
closes: #18045
Signed-off-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: fuzzy-matching can be improved
Solution: Implement a better fuzzy matching algorithm
(Girish Palya)
Replace fuzzy matching algorithm with improved fzy-based implementation
The
[current](https://www.forrestthewoods.com/blog/reverse_engineering_sublime_texts_fuzzy_match/)
fuzzy matching algorithm has several accuracy issues:
* It struggles with CamelCase
* It fails to prioritize matches at the beginning of strings, often
ranking middle matches higher.
After evaluating alternatives (see my comments
[here](https://github.com/vim/vim/issues/17531#issuecomment-3112046897)
and
[here](https://github.com/vim/vim/issues/17531#issuecomment-3121593900)),
I chose to adopt the [fzy](https://github.com/jhawthorn/fzy) algorithm,
which:
* Resolves the aforementioned issues.
* Performs better.
Implementation details
This version is based on the original fzy
[algorithm](https://github.com/jhawthorn/fzy/blob/master/src/match.c),
with one key enhancement: **multibyte character support**.
* The original implementation supports only ASCII.
* This patch replaces ascii lookup tables with function calls, making it
compatible with multibyte character sets.
* Core logic (`match_row()` and `match_positions()`) remains faithful to
the original, but now operates on codepoints rather than single-byte
characters.
Performance
Tested against a dataset of **90,000 Linux kernel filenames**. Results
(in milliseconds) show a **\~2x performance improvement** over the
current fuzzy matching algorithm.
```
Search String Current Algo FZY Algo
-------------------------------------------------
init 131.759 66.916
main 83.688 40.861
sig 98.348 39.699
index 109.222 30.738
ab 72.222 44.357
cd 83.036 54.739
a 58.94 62.242
b 43.612 43.442
c 64.39 67.442
k 40.585 36.371
z 34.708 22.781
w 38.033 30.109
cpa 82.596 38.116
arz 84.251 23.964
zzzz 35.823 22.75
dimag 110.686 29.646
xa 43.188 29.199
nha 73.953 31.001
nedax 94.775 29.568
dbue 79.846 25.902
fp 46.826 31.641
tr 90.951 55.883
kw 38.875 23.194
rp 101.575 55.775
kkkkkkkkkkkkkkkkkkkkkkkkkkkkk 48.519 30.921
```
```vim
vim9script
var haystack = readfile('/Users/gp/linux.files')
var needles = ['init', 'main', 'sig', 'index', 'ab', 'cd', 'a', 'b',
'c', 'k',
'z', 'w', 'cpa', 'arz', 'zzzz', 'dimag', 'xa', 'nha', 'nedax',
'dbue',
'fp', 'tr', 'kw', 'rp', 'kkkkkkkkkkkkkkkkkkkkkkkkkkkkk']
for needle in needles
var start = reltime()
var tmp = matchfuzzy(haystack, needle)
echom $'{needle}' (start->reltime()->reltimefloat() * 1000)
endfor
```
Additional changes
* Removed the "camelcase" option from both matchfuzzy() and
matchfuzzypos(), as it's now obsolete with the improved algorithm.
related: neovim/neovim#34101
fixes#17531closes: #17900
Signed-off-by: Girish Palya <girishji@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: The 'grepformat' option is global option, but it would be
useful to have it buffer-local, similar to 'errorformat' and
other quickfix related options (Dani Dickstein)
Solution: Add the necessary code to support global-local 'grepformat',
allowing different buffers to parse different grep output
formats (glepnir)
fixes: #17316closes: #17315
Signed-off-by: glepnir <glephunter@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: style: more wrong indentation
Solution: reformat a few more places
(Yegappan Lakshmanan)
closes: #17309
Signed-off-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: when failing to wipeout a quickfix dummy buffer, it will
remain as a dummy buffer, despite being kept.
Solution: clear its dummy BF_DUMMY flag in this case (Sean Dewar).
closes: #17283
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: heap use-after-free possible when autocommands switch away from the
quickfix dummy buffer, but leave it open in a window.
Solution: close its windows first before attempting the wipe.
(Sean Dewar)
related: #17283
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Various typos in the code, redundant and strange use of
:execute in test_ins_complete.vim (after 9.1.1315).
Solution: Fix typos in the code and in the documentation, use the
executed command directly (zeertzjq).
closes: #17143
Co-authored-by: Christ van Willegen <cvwillegen@gmail.com>
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Using wrong window in ll_resize_stack()
(after v9.1.1287)
Solution: Use "wp" instead of "curwin", even though they are always the
same value. Fix typos in documentation (zeertzjq).
closes: #17080
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: quickfix and location-list stack is limited to 10 items
Solution: add the 'chistory' and 'lhistory' options to configure a
larger quickfix/location list stack
(64-bitman)
closes: #16920
Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: 64-bitman <60551350+64-bitman@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: If win_close() is called with a window that has quickfix stack
attached to it, the corresponding quickfix buffer will be
closed and freed after the buffer was already closed. At that
time curwin->w_buffer points to NULL, which the CHECK_CURBUF
will catch and abort if ABORT_ON_ERROR is defined
Solution: in wipe_qf_buffer() temporarily point curwin->w_buffer back to
curbuf, the window will be closed anyhow, so it shouldn't
matter that curbuf->b_nwindows isn't incremented.
closes: #16993closes: #16985
Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Vim script is missing the tuple data type
Solution: Add support for the tuple data type
(Yegappan Lakshmanan)
closes: #16776
Signed-off-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: When searching for "Cur", CamelCase matches like "lCursor" score
higher than exact prefix matches like Cursor, which is
counter-intuitive (Maxim Kim).
Solution: Add a 'camelcase' option to matchfuzzy() that lets users disable
CamelCase bonuses when needed, making prefix matches rank higher.
(glepnir)
fixes: #16504closes: #16797
Signed-off-by: glepnir <glephunter@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: cannot preserve error position when setting quickfix lists
Solution: Add the 'u' action for setqflist()/setloclist() and try
to keep the closes target position (Jeremy Fleischman)
fixes: #15839closes: #15841
Signed-off-by: Jeremy Fleischman <jeremyfleischman@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: quickfix list does not handle hardlinks well
Solution: store original file name with quickfix entry
(Austin Chang)
Quickfix list shows entries with duplicate name if the file is opened
with the path of hard links.
The major cause is that qflist assumed that the filename passed into
`qf_add_entry` matches the filename opened with the buffer.
This patch handles this case by introduce a `qf_fname` into `qfline_S`
structure. It stores the filename from `qf_add_entry` for each quickfix
line.
closes: #15687
Signed-off-by: Austin Chang <austin880625@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: heap-use-after-free in garbage collection with location list
user data.
Solution: Mark user data as in use when no other window is referencing
the location list (zeertzjq)
fixes: neovim/neovim#30371
closes: #15683
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: too many strlen() calls in search.c
Solution: refactor code and remove more strlen() calls,
use explicit variable to remember strlen
(John Marriott)
closes: #14796
Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Filetype may be undetected when a SwapExists autocommand sets
filetype in another buffer.
Solution: Make filetype detection state buffer-specific. Also fix a
similar problem for 'modified' (zeertzjq).
closes: #14344
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Coverity reports dead code.
Solution: Remove the dead code. Also fix a mistake in ml_get_pos_len()
and update some comments (zeertzjq).
closes: #14189
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: More code can use ml_get_buf_len() instead of STRLEN().
Solution: Change more STRLEN() calls to ml_get_buf_len(). Also do not
set ml_line_textlen in ml_replace_len() if "has_props" is set,
because "len_arg" also includes the size of text properties in
that case. (zeertzjq)
closes: #14183
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: may not be clear why failing to split causes an ":Xdo" command
to abort if 'wfb' is set.
Solution: do not return immediately if win_split fails, so E1513 is
still given. Expect both errors in the test. Also fix tests to
pass CI.
(Sean Dewar)
closes: #14152
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: several minor 'winfixbuf' issues exist, mostly relating to the
quickfix list
Solution: address them and adjust tests. Retab and reflow a few things too.
(Sean Dewar)
Things touched include:
- Replace the semsgs with gettext'd emsgs.
- Handle window switching in ex_listdo properly, so curbuf and curwin
are kept in-sync and trigger autocommands; handle those properly.
- Don't change the list entry index in qf_jump_edit_buffer if we fail
due to 'wfb' (achieved by returning FAIL; QF_ABORT should only be used
if the list was changed).
- Make qf_jump_edit_buffer actually switch to prevwin when using `:cXX`
commands **outside** of the list window if 'wfb' is set in curwin.
Handle autocommands properly in case they mess with the list.
NOTE: previously, it seemed to split if 'wfb' was set, but do nothing
and fail if prevwin is *valid*. This behaviour seemed strange, and maybe
unintentional? Now it aligns more with what's described for the `:cXX`
commands in the original PR description when used outside a list window,
I think.
- In both functions, only consider prevwin if 'wfb' isn't set for it;
fallback to splitting otherwise.
- Use win_split to split. Not sure if there was a specific reason for
using ex_splitview. win_split is simpler and respects modifiers like
:vertical that may have been used. Plus, its return value can be checked
for setting opened_window in qf code (technically win_split_ins autocmds
could immediately close it or change windows, in which the qf code might
close some other window on failure; it's already the case elsewhere,
though).
closes: #14142
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: qf_goto_win_with_qfl_file may check if prevwin has 'winfixbuf'
set without checking if it's valid first.
Solution: Reverse the condition. Add a test, a modeline, and a missing
CheckFeature. (Searn Dewar)
closes: #14140
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: Cannot keep a buffer focused in a window
(Amit Levy)
Solution: Add the 'winfixbuf' window-local option
(Colin Kennedy)
fixes: #6445closes: #13903
Signed-off-by: Colin Kennedy <colinvfx@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: duplicate code when cleaning undo stack
Solution: refactor undo cleanup into a single public function
related: #13928
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: When the quickfix buffer has been modified an autocommand
may invalidate the undo stack (kawarimidoll)
Solution: When clearing the quickfix buffer, also wipe the undo stack
fixes: #13905closes: #13928
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: [security] use-after-free in qf_free_items
Solution: only access qfpnext, if it hasn't been freed
Coverity discovered a possible use-after-free in qf_free_items. When
freeing the qfline items, we may access freed memory, when qfp ==
qfpnext.
So only access qfpnext, when it hasn't been freed.
Signed-off-by: Christian Brabandt <cb@256bit.org>
Problem: cannot use buffer-number for errorformat
Solution: add support for parsing a buffer number using '%b' in
'errorformat'
closes: #13419
Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
Problem: "clear" macros are not always used.
Solution: Use ALLOC_ONE, VIM_CLEAR, CLEAR_POINTER and CLEAR_FIELD in more
places. (Yegappan Lakshmanan, closes#12104)
Hirohito Higashi
John Marriott
Shane Harper
Sean Dewar
Christian Brabandt
glepnir
Girish Palya
Yegappan Lakshmanan
Naruhiko Nishino
zeertzjq
64-bitman
Jeremy Fleischman
Austin Chang
Colin Kennedy
Tom Praschan
Bram Moolenaar