From f8795365deb88ab4e108858c563284b1082d06d4 Mon Sep 17 00:00:00 2001 From: Evgeni Chasnovski Date: Fri, 21 Jun 2024 16:23:02 +0300 Subject: test(lua): cover `vim._with()` with tests Problem: `vim._with()` has many different use cases which are not covered with tests. Solution: cover with tests. Some (many) test cases are intentionally marked as "pending" because they cover cases which don't work as expected at the moment (and fixing them requires specific knowledge of C codebase). Use them as a reference for future fixes. Also some of "can be nested" tests currently might pass only because the tested context doesn't work. --- runtime/lua/vim/shared.lua | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'runtime/lua/vim') diff --git a/runtime/lua/vim/shared.lua b/runtime/lua/vim/shared.lua index 7fd29d5f7b..79e614aaaa 100644 --- a/runtime/lua/vim/shared.lua +++ b/runtime/lua/vim/shared.lua @@ -1144,7 +1144,6 @@ end --- @field buf? integer --- @field emsg_silent? boolean --- @field hide? boolean ---- @field horizontal? boolean --- @field keepalt? boolean --- @field keepjumps? boolean --- @field keepmarks? boolean @@ -1159,6 +1158,15 @@ end --- Executes function `f` with the given context specification. --- +--- Notes: +--- - Context `{ buf = buf }` has no guarantees about current window when +--- inside context. +--- - Context `{ buf = buf, win = win }` is yet not allowed, but this seems +--- to be an implementation detail. +--- - There should be no way to revert currently set `context.sandbox = true` +--- (like with nested `vim._with()` calls). Otherwise it kind of breaks the +--- whole purpose of sandbox execution. +--- --- @param context vim.context.mods function vim._with(context, f) vim.validate('context', context, 'table') @@ -1167,7 +1175,6 @@ function vim._with(context, f) vim.validate('context.buf', context.buf, 'number', true) vim.validate('context.emsg_silent', context.emsg_silent, 'boolean', true) vim.validate('context.hide', context.hide, 'boolean', true) - vim.validate('context.horizontal', context.horizontal, 'boolean', true) vim.validate('context.keepalt', context.keepalt, 'boolean', true) vim.validate('context.keepjumps', context.keepjumps, 'boolean', true) vim.validate('context.keepmarks', context.keepmarks, 'boolean', true) @@ -1192,6 +1199,10 @@ function vim._with(context, f) if not vim.api.nvim_win_is_valid(context.win) then error('Invalid window id: ' .. context.win) end + -- TODO: Maybe allow it? + if context.buf and vim.api.nvim_win_get_buf(context.win) ~= context.buf then + error('Can not set both `buf` and `win` context.') + end end -- Store original options @@ -1214,7 +1225,7 @@ function vim._with(context, f) end end - return unpack(retval) + return unpack(retval, 1, table.maxn(retval)) end return vim -- cgit From 07cc559cdf11acb3031b6b7ba53e65285a6f4b3f Mon Sep 17 00:00:00 2001 From: Evgeni Chasnovski Date: Fri, 21 Jun 2024 16:23:05 +0300 Subject: feat(lua): update `vim._with` to allow more granular option contexts Problem: with a single `context.options` there is no way for user to force which scope (local, global, both) is being temporarily set and later restored. Solution: replace single `options` context with `bo`, `go`, `wo`, and `o`. Naming and implementation follows how options can be set directly with `vim.*` (like `vim.bo`, etc.). Options are set for possible target `win` or `buf` context. --- runtime/lua/vim/shared.lua | 99 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 82 insertions(+), 17 deletions(-) (limited to 'runtime/lua/vim') diff --git a/runtime/lua/vim/shared.lua b/runtime/lua/vim/shared.lua index 79e614aaaa..0c20248d8e 100644 --- a/runtime/lua/vim/shared.lua +++ b/runtime/lua/vim/shared.lua @@ -1141,8 +1141,10 @@ end --- @nodoc --- @class vim.context.mods +--- @field bo? table --- @field buf? integer --- @field emsg_silent? boolean +--- @field go? table --- @field hide? boolean --- @field keepalt? boolean --- @field keepjumps? boolean @@ -1150,11 +1152,47 @@ end --- @field keeppatterns? boolean --- @field lockmarks? boolean --- @field noautocmd? boolean ---- @field options? table +--- @field o? table --- @field sandbox? boolean --- @field silent? boolean --- @field unsilent? boolean --- @field win? integer +--- @field wo? table + +--- @nodoc +--- @class vim.context.state +--- @field bo? table +--- @field go? table +--- @field wo? table + +local scope_map = { buf = 'bo', global = 'go', win = 'wo' } +local scope_order = { 'o', 'wo', 'bo', 'go' } +local state_restore_order = { 'bo', 'wo', 'go' } + +--- Gets data about current state, enough to properly restore specified options/env/etc. +--- @param context vim.context.mods +--- @return vim.context.state +local get_context_state = function(context) + local res = { bo = {}, go = {}, wo = {} } + + -- Use specific order from possibly most to least intrusive + for _, scope in ipairs(scope_order) do + for name, _ in pairs(context[scope] or {}) do + local sc = scope == 'o' and scope_map[vim.api.nvim_get_option_info2(name, {}).scope] or scope + + -- Do not override already set state and fall back to `vim.NIL` for + -- state `nil` values (which still needs restoring later) + res[sc][name] = res[sc][name] or vim[sc][name] or vim.NIL + + -- Always track global option value to properly restore later. + -- This matters for at least `o` and `wo` (which might set either/both + -- local and global option values). + res.go[name] = res.go[name] or vim.go[name] + end + end + + return res +end --- Executes function `f` with the given context specification. --- @@ -1166,14 +1204,24 @@ end --- - There should be no way to revert currently set `context.sandbox = true` --- (like with nested `vim._with()` calls). Otherwise it kind of breaks the --- whole purpose of sandbox execution. +--- - Saving and restoring option contexts (`bo`, `go`, `o`, `wo`) trigger +--- `OptionSet` events. This is an implementation issue because not doing it +--- seems to mean using either 'eventignore' option or extra nesting with +--- `{ noautocmd = true }` (which itself is a wrapper for 'eventignore'). +--- As `{ go = { eventignore = '...' } }` is a valid context which should be +--- properly set and restored, this is not a good approach. +--- Not triggering `OptionSet` seems to be a good idea, though. So probably +--- only moving context save and restore to lower level might resolve this. --- --- @param context vim.context.mods function vim._with(context, f) vim.validate('context', context, 'table') vim.validate('f', f, 'function') + vim.validate('context.bo', context.bo, 'table', true) vim.validate('context.buf', context.buf, 'number', true) vim.validate('context.emsg_silent', context.emsg_silent, 'boolean', true) + vim.validate('context.go', context.go, 'table', true) vim.validate('context.hide', context.hide, 'boolean', true) vim.validate('context.keepalt', context.keepalt, 'boolean', true) vim.validate('context.keepjumps', context.keepjumps, 'boolean', true) @@ -1181,11 +1229,12 @@ function vim._with(context, f) vim.validate('context.keeppatterns', context.keeppatterns, 'boolean', true) vim.validate('context.lockmarks', context.lockmarks, 'boolean', true) vim.validate('context.noautocmd', context.noautocmd, 'boolean', true) - vim.validate('context.options', context.options, 'table', true) + vim.validate('context.o', context.o, 'table', true) vim.validate('context.sandbox', context.sandbox, 'boolean', true) vim.validate('context.silent', context.silent, 'boolean', true) vim.validate('context.unsilent', context.unsilent, 'boolean', true) vim.validate('context.win', context.win, 'number', true) + vim.validate('context.wo', context.wo, 'table', true) -- Check buffer exists if context.buf then @@ -1205,27 +1254,43 @@ function vim._with(context, f) end end - -- Store original options - local previous_options ---@type table - if context.options then - previous_options = {} - for k, v in pairs(context.options) do - previous_options[k] = - vim.api.nvim_get_option_value(k, { win = context.win, buf = context.buf }) - vim.api.nvim_set_option_value(k, v, { win = context.win, buf = context.buf }) + -- Decorate so that save-set-restore options is done in correct window-buffer + local callback = function() + -- Cache current values to be changed by context + -- Abort early in case of bad context value + local ok, state = pcall(get_context_state, context) + if not ok then + error(state, 0) end - end - local retval = { vim._with_c(context, f) } + -- Apply some parts of the context in specific order + -- NOTE: triggers `OptionSet` event + for _, scope in ipairs(scope_order) do + for name, context_value in pairs(context[scope] or {}) do + vim[scope][name] = context_value + end + end + + -- Execute + local res = { pcall(f) } + + -- Restore relevant cached values in specific order, global scope last + -- NOTE: triggers `OptionSet` event + for _, scope in ipairs(state_restore_order) do + for name, cached_value in pairs(state[scope]) do + vim[scope][name] = cached_value + end + end - -- Restore original options - if previous_options then - for k, v in pairs(previous_options) do - vim.api.nvim_set_option_value(k, v, { win = context.win, buf = context.buf }) + -- Return + if not res[1] then + error(res[2], 0) end + table.remove(res, 1) + return unpack(res, 1, table.maxn(res)) end - return unpack(retval, 1, table.maxn(retval)) + return vim._with_c(context, callback) end return vim -- cgit From cd53db2157f0cd27877451a6b00d66e9bed74e73 Mon Sep 17 00:00:00 2001 From: Evgeni Chasnovski Date: Sun, 23 Jun 2024 17:13:06 +0300 Subject: feat(lua): add `context.env` (environment variables) to `vim._with()` --- runtime/lua/vim/shared.lua | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'runtime/lua/vim') diff --git a/runtime/lua/vim/shared.lua b/runtime/lua/vim/shared.lua index 0c20248d8e..6de1ce9d0c 100644 --- a/runtime/lua/vim/shared.lua +++ b/runtime/lua/vim/shared.lua @@ -1144,6 +1144,7 @@ end --- @field bo? table --- @field buf? integer --- @field emsg_silent? boolean +--- @field env? table --- @field go? table --- @field hide? boolean --- @field keepalt? boolean @@ -1162,18 +1163,19 @@ end --- @nodoc --- @class vim.context.state --- @field bo? table +--- @field env? table --- @field go? table --- @field wo? table local scope_map = { buf = 'bo', global = 'go', win = 'wo' } -local scope_order = { 'o', 'wo', 'bo', 'go' } -local state_restore_order = { 'bo', 'wo', 'go' } +local scope_order = { 'o', 'wo', 'bo', 'go', 'env' } +local state_restore_order = { 'bo', 'wo', 'go', 'env' } --- Gets data about current state, enough to properly restore specified options/env/etc. --- @param context vim.context.mods --- @return vim.context.state local get_context_state = function(context) - local res = { bo = {}, go = {}, wo = {} } + local res = { bo = {}, env = {}, go = {}, wo = {} } -- Use specific order from possibly most to least intrusive for _, scope in ipairs(scope_order) do @@ -1187,7 +1189,9 @@ local get_context_state = function(context) -- Always track global option value to properly restore later. -- This matters for at least `o` and `wo` (which might set either/both -- local and global option values). - res.go[name] = res.go[name] or vim.go[name] + if sc ~= 'env' then + res.go[name] = res.go[name] or vim.go[name] + end end end @@ -1221,6 +1225,7 @@ function vim._with(context, f) vim.validate('context.bo', context.bo, 'table', true) vim.validate('context.buf', context.buf, 'number', true) vim.validate('context.emsg_silent', context.emsg_silent, 'boolean', true) + vim.validate('context.env', context.env, 'table', true) vim.validate('context.go', context.go, 'table', true) vim.validate('context.hide', context.hide, 'boolean', true) vim.validate('context.keepalt', context.keepalt, 'boolean', true) -- cgit