From 86ef062c8cb7a89eb9aa8b5d141027540d10826c Mon Sep 17 00:00:00 2001 From: bfredl Date: Mon, 16 Dec 2024 13:31:59 +0100 Subject: [PATCH] fix(wininfo): when freeing windows, free the lowest priority wininfo On master (and also before #31539) deleting a window could cause the used wininfo for a buffer to change. This is due to always removing the previous NULL wininfo when deleting a window, even if that wininfo had higher priority than the the deleted window's own wininfo. Instead delete the wininfo with lowest priority. This retains the memory saving efect while not affecting the effective value of window options and so on. --- src/nvim/window.c | 11 +++++--- test/functional/api/window_spec.lua | 42 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index b1c483547c..190ac500b0 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -5242,11 +5242,13 @@ void win_free(win_T *wp, tabpage_T *tp) // freed memory is re-used for another window. FOR_ALL_BUFFERS(buf) { WinInfo *wip_wp = NULL; + size_t pos_wip = kv_size(buf->b_wininfo); size_t pos_null = kv_size(buf->b_wininfo); for (size_t i = 0; i < kv_size(buf->b_wininfo); i++) { WinInfo *wip = kv_A(buf->b_wininfo, i); if (wip->wi_win == wp) { wip_wp = wip; + pos_wip = i; } else if (wip->wi_win == NULL) { pos_null = i; } @@ -5254,11 +5256,12 @@ void win_free(win_T *wp, tabpage_T *tp) if (wip_wp) { wip_wp->wi_win = NULL; - // If there already is an entry with "wi_win" set to NULL it - // must be removed, it would never be used. + // If there already is an entry with "wi_win" set to NULL, only + // the first entry with NULL will ever be used, delete the other one. if (pos_null < kv_size(buf->b_wininfo)) { - free_wininfo(kv_A(buf->b_wininfo, pos_null), buf); - kv_shift(buf->b_wininfo, pos_null, 1); + size_t pos_delete = MAX(pos_null, pos_wip); + free_wininfo(kv_A(buf->b_wininfo, pos_delete), buf); + kv_shift(buf->b_wininfo, pos_delete, 1); } } } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 92999f383a..4d5318a5dd 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -488,6 +488,48 @@ describe('API/win', function() assert_alive() end) + describe('after deleting', function() + local buf, win0, win1, win2 + + before_each(function() + win0 = api.nvim_get_current_win() + command('new') + buf = api.nvim_get_current_buf() + win1 = api.nvim_get_current_win() + command('set numberwidth=10') + command('split') + win2 = api.nvim_get_current_win() + command('set numberwidth=15') + command('enew') + api.nvim_set_current_win(win1) + command('normal ix') + command('enew') + api.nvim_set_current_win(win0) + eq(4, api.nvim_get_option_value('numberwidth', {})) + end) + + -- at this point buffer `buf` is current in no windows. deletion shouldn't affect its defaults + it('0 windows', function() + api.nvim_set_current_buf(buf) + eq(10, api.nvim_get_option_value('numberwidth', {})) + end) + + it('1 window', function() + api.nvim_win_close(win1, false) + + api.nvim_set_current_buf(buf) + eq(10, api.nvim_get_option_value('numberwidth', {})) + end) + + it('2 windows', function() + api.nvim_win_close(win1, false) + api.nvim_win_close(win2, false) + + api.nvim_set_current_buf(buf) + eq(10, api.nvim_get_option_value('numberwidth', {})) + end) + end) + it('returns values for unset local options', function() eq(-1, api.nvim_get_option_value('scrolloff', { win = 0, scope = 'local' })) end)