Skip to content

Commit

Permalink
feat(api): modify nvim_exec2() to always return table
Browse files Browse the repository at this point in the history
Details:
- Return table always contains string `output` field, even if
  `{output = false}` as input. Besides better consistency, another
  reason for this is that empty `Dictionary` from C implementation of
  `nvim_exec2()` results into `{ [true] = 6 }` when used from Lua. Not a
  *huge* problem, but doesn't provide the best user experience.
  • Loading branch information
echasnovski committed Mar 16, 2023
1 parent 58af61f commit 175b5ba
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 61 deletions.
9 changes: 5 additions & 4 deletions runtime/doc/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1586,12 +1586,13 @@ nvim_exec2({src}, {*opts}) *nvim_exec2()*
Parameters: ~
{src} Vimscript code
{opts} Optional parameters.
• output: Capture and return all (non-error, non-shell |:!|)
output. Default: `false`.
• output: (boolean, default false) Whether to capture and
return all (non-error, non-shell |:!|) output.

Return: ~
Output (non-error, non-shell |:!|) if `opts.output` is true, else
empty string.
Dictionary containing information about execution, with these keys:
• output: (string) Output if `opts.output` is true, empty string
otherwise.

See also: ~
|execute()|
Expand Down
2 changes: 1 addition & 1 deletion runtime/lua/vim/_editor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ vim.cmd = setmetatable({}, {
if type(command) == 'table' then
return vim.api.nvim_cmd(command, {})
else
return vim.api.nvim_exec2(command, { output = false })
return vim.api.nvim_exec2(command, { output = false }).output
end
end,
__index = function(t, command)
Expand Down
9 changes: 5 additions & 4 deletions src/nvim/api/deprecated.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
/// @see nvim_exec2
String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
FUNC_API_SINCE(7)
FUNC_API_DEPRECATED_SINCE(7)
{
Dict(exec) opts = { 0 };
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(output);
return nvim_exec2(channel_id, src, &opts, err);
return exec_impl(channel_id, src, &opts, err);
}

/// @deprecated
Expand All @@ -40,9 +41,9 @@ String nvim_command_output(uint64_t channel_id, String command, Error *err)
FUNC_API_SINCE(1)
FUNC_API_DEPRECATED_SINCE(7)
{
Dict(exec) opts = { 0 };
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(true);
return nvim_exec2(channel_id, command, &opts, err);
return exec_impl(channel_id, command, &opts, err);
}

/// @deprecated Use nvim_exec_lua() instead.
Expand Down
2 changes: 1 addition & 1 deletion src/nvim/api/keysets.lua
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ return {
{ 'echo_opts', {
"verbose";
}};
{ 'exec', {
{ 'exec_opts', {
"output";
}};
}
18 changes: 13 additions & 5 deletions src/nvim/api/vimscript.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,21 @@
///
/// @param src Vimscript code
/// @param opts Optional parameters.
/// - output: Capture and return all (non-error, non-shell |:!|) output.
/// Default: `false`.
/// - output: (boolean, default false) Whether to capture and return
/// all (non-error, non-shell |:!|) output.
/// @param[out] err Error details (Vim error), if any
/// @return Output (non-error, non-shell |:!|) if `opts.output` is true,
/// else empty string.
String nvim_exec2(uint64_t channel_id, String src, Dict(exec) *opts, Error *err)
/// @return Dictionary containing information about execution, with these keys:
/// - output: (string) Output if `opts.output` is true, empty string
/// otherwise.
Dictionary nvim_exec2(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *err)
FUNC_API_SINCE(11)
{
Dictionary result = ARRAY_DICT_INIT;
PUT(result, "output", STRING_OBJ(exec_impl(channel_id, src, opts, err)));
return result;
}

String exec_impl(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *err)
{
Boolean output = false;
if (HAS_KEY(opts->output)) {
Expand Down
7 changes: 4 additions & 3 deletions src/nvim/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string.h>

#include "nvim/api/private/converter.h"
#include "nvim/api/private/defs.h"
#include "nvim/api/private/helpers.h"
#include "nvim/api/vimscript.h"
#include "nvim/context.h"
Expand Down Expand Up @@ -271,10 +272,10 @@ static inline void ctx_save_funcs(Context *ctx, bool scriptonly)
size_t cmd_len = sizeof("func! ") + strlen(name);
char *cmd = xmalloc(cmd_len);
snprintf(cmd, cmd_len, "func! %s", name);
Dict(exec) opts = { 0 };
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(true);
String func_body = nvim_exec2(VIML_INTERNAL_CALL, cstr_as_string(cmd),
&opts, &err);
String func_body = exec_impl(VIML_INTERNAL_CALL, cstr_as_string(cmd),
&opts, &err);
xfree(cmd);
if (!ERROR_SET(&err)) {
ADD(ctx->funcs, STRING_OBJ(func_body));
Expand Down
10 changes: 7 additions & 3 deletions test/functional/api/keymap_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ describe('nvim_set_keymap, nvim_del_keymap', function()
let g:flip = !g:flip
return g:flip
endfunction
]], { output = true })
]], { output = false })
eq(1, meths.call_function('FlipFlop', {}))
eq(0, meths.call_function('FlipFlop', {}))
eq(1, meths.call_function('FlipFlop', {}))
Expand Down Expand Up @@ -827,8 +827,12 @@ describe('nvim_set_keymap, nvim_del_keymap', function()
exec_lua [[
vim.api.nvim_set_keymap ('n', 'asdf', '', {callback = function() print('jkl;') end })
]]
assert.truthy(string.match(exec_lua[[return vim.api.nvim_exec2(':nmap asdf', { output = true })]],
"^\nn asdf <Lua %d+>"))
assert.truthy(
string.match(
exec_lua[[return vim.api.nvim_exec2(':nmap asdf', { output = true }).output]],
"^\nn asdf <Lua %d+>"
)
)
end)

it ('mapcheck() returns lua mapping correctly', function()
Expand Down
36 changes: 21 additions & 15 deletions test/functional/api/vim_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ describe('API', function()
end)

describe('nvim_exec2', function()
it('always returns table', function()
eq({ output = '' }, nvim('exec2', 'echo "Hello"', {}))
eq({ output = '' }, nvim('exec2', 'echo "Hello"', { output = false }))
eq({ output = 'Hello' }, nvim('exec2', 'echo "Hello"', { output = true }))
end)

it('default options', function()
-- Should be equivalent to { output = false }
nvim('exec2', "let x0 = 'a'", {})
Expand All @@ -103,15 +109,15 @@ describe('API', function()

it(':verbose set {option}?', function()
nvim('exec2', 'set nowrap', { output = false })
eq('nowrap\n\tLast set from anonymous :source',
eq({ output = 'nowrap\n\tLast set from anonymous :source' },
nvim('exec2', 'verbose set wrap?', { output = true }))

-- Using script var to force creation of a script item
nvim('exec2', [[
let s:a = 1
set nowrap
]], { output = false })
eq('nowrap\n\tLast set from anonymous :source (script id 1)',
eq({ output = 'nowrap\n\tLast set from anonymous :source (script id 1)' },
nvim('exec2', 'verbose set wrap?', { output = true }))
end)

Expand All @@ -122,7 +128,7 @@ describe('API', function()
nvim('exec2','lua <<EOF\n\n\n\ny=3\n\n\nEOF', { output = false })
eq(3, nvim('eval', "luaeval('y')"))

eq('', nvim('exec2', 'lua <<EOF\ny=3\nEOF', { output = false }))
eq({ output = '' }, nvim('exec2', 'lua <<EOF\ny=3\nEOF', { output = false }))
eq(3, nvim('eval', "luaeval('y')"))

-- Multiple statements
Expand Down Expand Up @@ -156,15 +162,15 @@ describe('API', function()
eq({a = 98}, request('nvim_eval', 'g:ab'))

-- Script scope (s:)
eq('ahoy! script-scoped varrrrr', nvim('exec2', [[
eq({ output = 'ahoy! script-scoped varrrrr' }, nvim('exec2', [[
let s:pirate = 'script-scoped varrrrr'
function! s:avast_ye_hades(s) abort
return a:s .. ' ' .. s:pirate
endfunction
echo <sid>avast_ye_hades('ahoy!')
]], { output = true }))

eq('ahoy! script-scoped varrrrr', nvim('exec2', [[
eq({ output = "{'output': 'ahoy! script-scoped varrrrr'}" }, nvim('exec2', [[
let s:pirate = 'script-scoped varrrrr'
function! Avast_ye_hades(s) abort
return a:s .. ' ' .. s:pirate
Expand All @@ -179,13 +185,13 @@ describe('API', function()
]], { output = false }))

-- Script items are created only on script var access
eq('1\n0', nvim('exec2', [[
eq({ output = '1\n0' }, nvim('exec2', [[
echo expand("<SID>")->empty()
let s:a = 123
echo expand("<SID>")->empty()
]], { output = true }))

eq('1\n0', nvim('exec2', [[
eq({ output = '1\n0' }, nvim('exec2', [[
echo expand("<SID>")->empty()
function s:a() abort
endfunction
Expand Down Expand Up @@ -257,17 +263,17 @@ describe('API', function()
'continuing in nvim_exec2() called at '..sourcing_fname..':1\n'..
'finished sourcing '..sourcing_fname..'\n'..
'continuing in nvim_exec2() called at nvim_exec2():0'
eq(traceback_output,
eq({ output = traceback_output },
meths.exec2('call nvim_exec2("source '..sourcing_fname..'", {"output": v:false})', { output = true }))
os.remove(fname)
os.remove(sourcing_fname)
end)

it('returns output', function()
eq('this is spinal tap',
eq({ output = 'this is spinal tap' },
nvim('exec2', 'lua <<EOF\n\n\nprint("this is spinal tap")\n\n\nEOF', { output = true }))
eq('', nvim('exec2', 'echo', { output = true }))
eq('foo 42', nvim('exec2', 'echo "foo" 42', { output = true }))
eq({ output = '' }, nvim('exec2', 'echo', { output = true }))
eq({ output = 'foo 42' }, nvim('exec2', 'echo "foo" 42', { output = true }))
end)

it('displays messages when opts.output=false', function()
Expand Down Expand Up @@ -2655,7 +2661,7 @@ describe('API', function()
eq(' 1 %a "[No Name]" line 1\n'..
' 3 h "[Scratch]" line 0\n'..
' 4 h "[Scratch]" line 0',
meths.exec2('ls', { output = true }))
meths.exec2('ls', { output = true }).output)
-- current buffer didn't change
eq({id=1}, meths.get_current_buf())

Expand Down Expand Up @@ -2922,13 +2928,13 @@ describe('API', function()
it('can save message history', function()
nvim('command', 'set cmdheight=2') -- suppress Press ENTER
nvim("echo", {{"msg\nmsg"}, {"msg"}}, true, {})
eq("msg\nmsgmsg", meths.exec2('messages', { output = true }))
eq("msg\nmsgmsg", meths.exec2('messages', { output = true }).output)
end)

it('can disable saving message history', function()
nvim('command', 'set cmdheight=2') -- suppress Press ENTER
nvim_async("echo", {{"msg\nmsg"}, {"msg"}}, false, {})
eq("", meths.exec2("messages", { output = true }))
eq("", meths.exec2("messages", { output = true }).output)
end)
end)

Expand Down Expand Up @@ -3917,7 +3923,7 @@ describe('API', function()

it('sets correct script context', function()
meths.cmd({ cmd = "set", args = { "cursorline" } }, {})
local str = meths.exec2([[verbose set cursorline?]], { output = true })
local str = meths.exec2([[verbose set cursorline?]], { output = true }).output
neq(nil, str:find("cursorline\n\tLast set from API client %(channel id %d+%)"))
end)

Expand Down
4 changes: 2 additions & 2 deletions test/functional/core/remote_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Remote', function()
expect(contents)
eq(1, #funcs.getbufinfo())
-- Since we didn't pass silent, we should get a complaint
neq(nil, string.find(meths.exec2('messages', { output = true }), 'E247'))
neq(nil, string.find(meths.exec2('messages', { output = true }).output, 'E247'))
end)

it('creates server if not found with tabs', function()
Expand All @@ -110,7 +110,7 @@ describe('Remote', function()
eq(2, #funcs.gettabinfo())
eq(2, #funcs.getbufinfo())
-- We passed silent, so no message should be issued about the server not being found
eq(nil, string.find(meths.exec2('messages', { output = true }), 'E247'))
eq(nil, string.find(meths.exec2('messages', { output = true }).output, 'E247'))
end)

pending('exits with error on', function()
Expand Down
4 changes: 2 additions & 2 deletions test/functional/core/startup_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ describe('user config init', function()
clear{ args_rm={'-u'}, env=xenv }
feed('<cr><c-c>') -- Dismiss "Conflicting config …" message.
eq(1, eval('g:lua_rc'))
matches('^E5422: Conflicting configs', meths.exec2('messages', { output = true }))
matches('^E5422: Conflicting configs', meths.exec2('messages', { output = true }).output)
end)
end)
end)
Expand Down Expand Up @@ -872,7 +872,7 @@ describe('runtime:', function()

eq(2, eval('g:lua_plugin'))
-- Check if plugin_file_path is listed in :scriptname
local scripts = meths.exec2(':scriptnames', { output = true })
local scripts = meths.exec2(':scriptnames', { output = true }).output
assert(scripts:find(plugin_file_path))

-- Check if plugin_file_path is listed in startup profile
Expand Down
4 changes: 2 additions & 2 deletions test/functional/ex_cmds/script_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('script_get-based command', function()
%s %s
endif
]])):format(cmd, garbage)))
eq('', meths.exec2('messages', { output = true }))
eq('', meths.exec2('messages', { output = true }).output)
if check_neq then
neq(0, exc_exec(dedent([[
%s %s
Expand All @@ -49,7 +49,7 @@ describe('script_get-based command', function()
EOF
endif
]])):format(cmd, garbage)))
eq('', meths.exec2('messages', { output = true }))
eq('', meths.exec2('messages', { output = true }).output)
if check_neq then
eq(true, pcall(source, (dedent([[
let g:exc = 0
Expand Down
20 changes: 10 additions & 10 deletions test/functional/ex_cmds/source_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ describe(':source', function()
let d = s:s]])

command('source')
eq('2', meths.exec2('echo a', { output = true }))
eq("{'k': 'v'}", meths.exec2('echo b', { output = true }))
eq('2', meths.exec2('echo a', { output = true }).output)
eq("{'k': 'v'}", meths.exec2('echo b', { output = true }).output)

-- Script items are created only on script var access
eq("1", meths.exec2('echo c', { output = true }))
eq("0zBEEFCAFE", meths.exec2('echo d', { output = true }))
eq("1", meths.exec2('echo c', { output = true }).output)
eq("0zBEEFCAFE", meths.exec2('echo d', { output = true }).output)

exec('set cpoptions+=C')
eq('Vim(let):E723: Missing end of Dictionary \'}\': ', exc_exec('source'))
Expand All @@ -124,14 +124,14 @@ describe(':source', function()
-- Source the 2nd line only
feed('ggjV')
feed_command(':source')
eq('3', meths.exec2('echo a', { output = true }))
eq('3', meths.exec2('echo a', { output = true }).output)

-- Source from 2nd line to end of file
feed('ggjVG')
feed_command(':source')
eq('4', meths.exec2('echo a', { output = true }))
eq("{'K': 'V'}", meths.exec2('echo b', { output = true }))
eq("<SNR>1_C()", meths.exec2('echo D()', { output = true }))
eq('4', meths.exec2('echo a', { output = true }).output)
eq("{'K': 'V'}", meths.exec2('echo b', { output = true }).output)
eq("<SNR>1_C()", meths.exec2('echo D()', { output = true }).output)

-- Source last line only
feed_command(':$source')
Expand All @@ -147,7 +147,7 @@ describe(':source', function()
let a = 123
]]
command('source')
eq('123', meths.exec2('echo a', { output = true }))
eq('123', meths.exec2('echo a', { output = true }).output)
end)

it('multiline heredoc command', function()
Expand All @@ -157,7 +157,7 @@ describe(':source', function()
EOF]])

command('source')
eq('4', meths.exec2('echo luaeval("y")', { output = true }))
eq('4', meths.exec2('echo luaeval("y")', { output = true }).output)
end)

it('can source lua files', function()
Expand Down
4 changes: 2 additions & 2 deletions test/functional/helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -824,11 +824,11 @@ function module.skip_fragile(pending_fn, cond)
end

function module.exec(code)
return module.meths.exec2(code, { output = false })
return module.meths.exec2(code, { output = false }).output
end

function module.exec_capture(code)
return module.meths.exec2(code, { output = true })
return module.meths.exec2(code, { output = true }).output
end

function module.exec_lua(code, ...)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/options/keymap_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe("'keymap' / :lmap", function()
command('lmapclear <buffer>')
command('set keymap=dvorak')
command('set nomore')
local bindings = funcs.nvim_exec2('lmap', { output = true })
local bindings = funcs.nvim_exec2('lmap', { output = true }).output
eq(dedent([[
l " @_
Expand Down
Loading

0 comments on commit 175b5ba

Please sign in to comment.