From c924c2a7b316afb982cd7786923446517c6fec6b Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 10 Mar 2026 10:06:20 +0000 Subject: [PATCH] fix(api): win_config_float_tp grid removal, redraw Problem: when nvim_win_set_config moves a floatwin between tabpages, its grid may remain if temporarily inside another tabpage. Also, things like tablines aren't redrawn. Solution: always remove its grid. Set must_redraw so things are redrawn even if w_redr_type was already set for the old tabpage. I don't think it's necessary to do anything extra here when removing the grid: - win_ui_flush calls ui_call_win_hide anyway, and calling it manually ends up sending two win_hide events. - ui_comp_remove_grid safely does nothing if the grid doesn't exist. - w_pos_changed is set by win_config_float later, if that's needed. I think the pending_comp_index_update set by ui_comp_remove_grid is enough anyway, at least for making sure win_ui_flush sends win_hide. Added test fails with the prior approach of checking `parent_tp != curtab`, but also `win_tp == curtab`. (which is a better, but still flawed alternative) The added redrawing here also supersedes setting w_hl_needs_update, and also redraws stuff like the tabline to pass the new test. --- src/nvim/api/win_config.c | 17 +-- test/functional/ui/float_spec.lua | 214 ++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+), 8 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 34c3e56e8b..1f9550a187 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -696,14 +696,15 @@ static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, win_tp->tp_curwin = win_float_find_altwin(win, win_tp); } - if (parent_tp != curtab) { - if (ui_has(kUIMultigrid)) { - ui_call_win_hide(win->w_grid_alloc.handle); - } - ui_comp_remove_grid(&win->w_grid_alloc); - } else { - win->w_hl_needs_update = true; - } + // Remove grid if present. More reliable than checking curtab, as tabpage_check_windows may + // not run when temporarily switching tabpages, meaning grids may be stale from another + // tabpage! (e.g: switch_win_noblock with no_display=true) + ui_comp_remove_grid(&win->w_grid_alloc); + + // Redraw tabline, update window's hl attribs, etc. Set must_redraw here, as redraw_later + // might not if w_redr_type >= UPD_NOT_VALID was set in the old tabpage. + redraw_later(win, UPD_NOT_VALID); + set_must_redraw(UPD_NOT_VALID); } } diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 15b7004f7e..0369e55a30 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -11869,6 +11869,220 @@ describe('float window', function() ]]) end end) + + it('redrawn after moving tabpages via nvim_win_set_config()', function() + local tab1_win = api.nvim_get_current_win() + fn.setline(1, 'hello') + command('tab split') + local tab2_win = api.nvim_get_current_win() + -- Schedule an UPD_NOT_VALID redraw, but in one event move the float out of curtab before it's + -- handled. Do not flush before then. + local float = exec_lua(function() + local float = vim.api.nvim_open_win(0, true, { relative = 'editor', width = 10, height = 5, row = 0, col = 0 }) + vim.api.nvim__redraw({ valid = false, flush = false }) + vim.api.nvim_win_set_config(float, { relative = 'win', win = tab1_win, row = 1, col = 1 }) + return float + end) + + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + ^hello | + {0:~ }|*4 + | + ]]) + end + + -- Importantly, want tabline redrawn and float's hl attribs to be correct here. + api.nvim_win_set_config(float, { relative = 'win', win = 0, row = 1, col = 1 }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: + [No Name] }{3: }{11:2}{3:+ [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ]], + float_pos = { + [5] = { 1002, 'NW', 4, 1, 1, true, 50, 1, 1, 1 }, + }, + }) + else + screen:expect([[ + {9: + [No Name] }{3: }{11:2}{3:+ [No Name] }{5: }{9:X}| + ^h{1:hello } | + {0:~}{2:~ }{0: }|*4 + | + ]]) + end + + -- Autocommand runs within the first tabpage, but won't refresh grids when switching to it due + -- to switch_win_noblock having no_display set. + command( + ('autocmd OptionSet rightleft ++once call nvim_win_set_config(%d, #{relative: "win", win: %d, row: 0, col: 0})'):format( + float, + tab1_win + ) + ) + api.nvim_set_option_value('rightleft', true, { win = tab1_win }) + -- Stale grids should not have caused issues with removing the float's grid. + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + ^hello | + {0:~ }|*4 + | + ]]) + end + + command('tabfirst') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {3: }{11:2}{3:+ [No Name] }{9: + [No Name] }{5: }{9:X}| + [2:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 + olle^h| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ]], + float_pos = { + [5] = { 1002, 'NW', 2, 0, 0, true, 50, 1, 1, 0 }, + }, + }) + else + screen:expect([[ + {3: }{11:2}{3:+ [No Name] }{9: + [No Name] }{5: }{9:X}| + {1:hello } olle^h| + {2:~ }{0: ~}|*4 + | + ]]) + end + + -- Check tablines are redrawn even when moving floats between two non-current tabpages. + command('tabnew') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: + [No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0: ~}|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: + [No Name] }{5: }{9:X}| + ^ | + {0: ~}|*4 + | + ]]) + end + + api.nvim_win_set_config(float, { relative = 'win', win = tab2_win, row = 1, col = 1 }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: + [No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0: ~}|*4 + ]], + }) + else + screen:expect([[ + {9: + [No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + ^ | + {0: ~}|*4 + | + ]]) + end + end) end describe('with ext_multigrid and actual mouse grid', function()