From 6b4970f6e0ac36021b2a8bd0533f5078040d31f7 Mon Sep 17 00:00:00 2001 From: Sean Dewar Date: Sun, 23 Jul 2023 19:50:20 +0100 Subject: [PATCH] feat(api): allow open_win/win_set_buf in the cmdwin in some cases Problem: As discussed on Matrix, there was some interest in having `nvim_open_win` again be able to open floats in the cmdwin (e.g: displaying a hover doc related to what's in the cmdwin). After #23228, this was disallowed. Solution: Allow `nvim_open_win` in the cmdwin as long as `!enter` and `buffer != curbuf` (the former can cause all sorts of issues, and the latter can crash Nvim after closing cmdwin). Also allow `nvim_win_set_buf` in a similar fashion. Note that we're not *entirely* sure if this is 100% safe (cmdwin is a global-state-using-main-loop-calling beast), but this seems to work OK..? Also: - Check the buffer argument of `nvim_open_win` earlier, and abort if it's invalid (it used to still open a window in this case). - Untranslate `e_cmdwin` errors in the API (other errors in the API are not translated: although not detailed in the API contract yet, errors are supposed to be stable). --- runtime/doc/api.txt | 4 +-- src/nvim/api/vim.c | 2 +- src/nvim/api/win_config.c | 13 +++++++-- src/nvim/api/window.c | 15 ++++++++--- src/nvim/window.c | 14 +++------- test/functional/api/window_spec.lua | 41 +++++++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 18 deletions(-) diff --git a/runtime/doc/api.txt b/runtime/doc/api.txt index 5b6a901970..1e5b6b0b40 100644 --- a/runtime/doc/api.txt +++ b/runtime/doc/api.txt @@ -2915,7 +2915,7 @@ nvim_win_set_buf({window}, {buffer}) *nvim_win_set_buf()* Sets the current buffer in a window, without side effects Attributes: ~ - not allowed when |textlock| is active or in the |cmdwin| + not allowed when |textlock| is active Parameters: ~ • {window} Window handle, or 0 for current window @@ -3036,7 +3036,7 @@ nvim_open_win({buffer}, {enter}, {*config}) *nvim_open_win()* < Attributes: ~ - not allowed when |textlock| is active or in the |cmdwin| + not allowed when |textlock| is active Parameters: ~ • {buffer} Buffer to display, or 0 for current buffer diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 8738b3e38e..b4a6fa718b 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -967,7 +967,7 @@ Integer nvim_open_term(Buffer buffer, DictionaryOf(LuaRef) opts, Error *err) } if (cmdwin_type != 0 && buf == curbuf) { - api_set_error(err, kErrorTypeException, "%s", _(e_cmdwin)); + api_set_error(err, kErrorTypeException, "%s", e_cmdwin); return 0; } diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 6ca36a0daf..81a239d913 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -158,8 +158,17 @@ /// @return Window handle, or 0 on error Window nvim_open_win(Buffer buffer, Boolean enter, Dict(float_config) *config, Error *err) FUNC_API_SINCE(6) - FUNC_API_TEXTLOCK + FUNC_API_TEXTLOCK_ALLOW_CMDWIN { + buf_T *buf = find_buffer_by_handle(buffer, err); + if (!buf) { + return 0; + } + if (cmdwin_type != 0 && (enter || buf == curbuf)) { + api_set_error(err, kErrorTypeException, "%s", e_cmdwin); + return 0; + } + FloatConfig fconfig = FLOAT_CONFIG_INIT; if (!parse_float_config(config, &fconfig, false, true, err)) { return 0; @@ -173,7 +182,7 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(float_config) *config, E } // autocmds in win_enter or win_set_buf below may close the window if (win_valid(wp) && buffer > 0) { - win_set_buf(wp->handle, buffer, fconfig.noautocmd, err); + win_set_buf(wp, buf, fconfig.noautocmd, err); } if (!win_valid(wp)) { api_set_error(err, kErrorTypeException, "Window was closed immediately"); diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index 5480584aa5..666f09e890 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -50,9 +50,18 @@ Buffer nvim_win_get_buf(Window window, Error *err) /// @param[out] err Error details, if any void nvim_win_set_buf(Window window, Buffer buffer, Error *err) FUNC_API_SINCE(5) - FUNC_API_TEXTLOCK + FUNC_API_TEXTLOCK_ALLOW_CMDWIN { - win_set_buf(window, buffer, false, err); + win_T *win = find_window_by_handle(window, err); + buf_T *buf = find_buffer_by_handle(buffer, err); + if (!win || !buf) { + return; + } + if (cmdwin_type != 0 && (win == curwin || buf == curbuf)) { + api_set_error(err, kErrorTypeException, "%s", e_cmdwin); + return; + } + win_set_buf(win, buf, false, err); } /// Gets the (1,0)-indexed, buffer-relative cursor position for a given window @@ -396,7 +405,7 @@ void nvim_win_close(Window window, Boolean force, Error *err) if (win == curwin) { cmdwin_result = Ctrl_C; } else { - api_set_error(err, kErrorTypeException, "%s", _(e_cmdwin)); + api_set_error(err, kErrorTypeException, "%s", e_cmdwin); } return; } diff --git a/src/nvim/window.c b/src/nvim/window.c index e230bde95c..fa7ca53b08 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -698,15 +698,9 @@ static void cmd_with_count(char *cmd, char *bufp, size_t bufsize, int64_t Prenum } } -void win_set_buf(Window window, Buffer buffer, bool noautocmd, Error *err) +void win_set_buf(win_T *win, buf_T *buf, bool noautocmd, Error *err) + FUNC_ATTR_NONNULL_ALL { - win_T *win = find_window_by_handle(window, err); - buf_T *buf = find_buffer_by_handle(buffer, err); - - if (!win || !buf) { - return; - } - tabpage_T *tab = win_find_tabpage(win); // no redrawing and don't set the window title @@ -720,7 +714,7 @@ void win_set_buf(Window window, Buffer buffer, bool noautocmd, Error *err) api_set_error(err, kErrorTypeException, "Failed to switch to window %d", - window); + win->handle); } try_start(); @@ -729,7 +723,7 @@ void win_set_buf(Window window, Buffer buffer, bool noautocmd, Error *err) api_set_error(err, kErrorTypeException, "Failed to set buffer %d", - buffer); + buf->handle); } // If window is not current, state logic will not validate its cursor. diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 19511f30f1..e7e767817b 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -43,6 +43,22 @@ describe('API/win', function() eq('Invalid buffer id: 23', pcall_err(window, 'set_buf', nvim('get_current_win'), 23)) eq('Invalid window id: 23', pcall_err(window, 'set_buf', 23, nvim('get_current_buf'))) end) + + it('disallowed in cmdwin if win=curwin or buf=curbuf', function() + local new_buf = meths.create_buf(true, true) + local new_win = meths.open_win(new_buf, false, { + relative='editor', row=10, col=10, width=50, height=10, + }) + feed('q:') + eq('E11: Invalid in command-line window; executes, CTRL-C quits', + pcall_err(meths.win_set_buf, 0, new_buf)) + eq('E11: Invalid in command-line window; executes, CTRL-C quits', + pcall_err(meths.win_set_buf, new_win, 0)) + + local next_buf = meths.create_buf(true, true) + meths.win_set_buf(new_win, next_buf) + eq(next_buf, meths.win_get_buf(new_win)) + end) end) describe('{get,set}_cursor', function() @@ -821,6 +837,31 @@ describe('API/win', function() }) eq(1, funcs.exists('g:fired')) end) + + it('disallowed in cmdwin if enter=true or buf=curbuf', function() + local new_buf = meths.create_buf(true, true) + feed('q:') + eq('E11: Invalid in command-line window; executes, CTRL-C quits', + pcall_err(meths.open_win, new_buf, true, { + relative='editor', row=5, col=5, width=5, height=5, + })) + eq('E11: Invalid in command-line window; executes, CTRL-C quits', + pcall_err(meths.open_win, 0, false, { + relative='editor', row=5, col=5, width=5, height=5, + })) + + eq(new_buf, meths.win_get_buf(meths.open_win(new_buf, false, { + relative='editor', row=5, col=5, width=5, height=5, + }))) + end) + + it('aborts if buffer is invalid', function() + local wins_before = meths.list_wins() + eq('Invalid buffer id: 1337', pcall_err(meths.open_win, 1337, false, { + relative='editor', row=5, col=5, width=5, height=5, + })) + eq(wins_before, meths.list_wins()) + end) end) describe('get_config', function()