Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): nvim_exec2(), deprecate nvim_exec() #19032

Merged
merged 1 commit into from
Mar 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions runtime/doc/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1549,11 +1549,11 @@ nvim_command({command}) *nvim_command()*

On execution error: fails with VimL error, updates v:errmsg.

Prefer using |nvim_cmd()| or |nvim_exec()| over this. To evaluate multiple
lines of Vim script or an Ex command directly, use |nvim_exec()|. To
construct an Ex command using a structured format and then execute it, use
|nvim_cmd()|. To modify an Ex command before evaluating it, use
|nvim_parse_cmd()| in conjunction with |nvim_cmd()|.
Prefer using |nvim_cmd()| or |nvim_exec2()| over this. To evaluate
multiple lines of Vim script or an Ex command directly, use
|nvim_exec2()|. To construct an Ex command using a structured format and
then execute it, use |nvim_cmd()|. To modify an Ex command before
evaluating it, use |nvim_parse_cmd()| in conjunction with |nvim_cmd()|.

Parameters: ~
{command} Ex command string
Expand All @@ -1570,7 +1570,7 @@ nvim_eval({expr}) *nvim_eval()*
Return: ~
Evaluation result or expanded object

nvim_exec({src}, {output}) *nvim_exec()*
nvim_exec2({src}, {*opts}) *nvim_exec2()*
Executes Vimscript (multiline block of Ex commands), like anonymous
|:source|.

Expand All @@ -1580,12 +1580,14 @@ nvim_exec({src}, {output}) *nvim_exec()*
On execution error: fails with VimL error, updates v:errmsg.

Parameters: ~
{src} Vimscript code
{output} Capture and return all (non-error, non-shell |:!|) output
{src} Vimscript code
{opts} Optional parameters.
• output: (boolean, default false) Whether to capture and
return all (non-error, non-shell |:!|) output.
Copy link
Member

@justinmk justinmk Mar 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should output param be a string, is it likely that a caller would want to also capture "error" output in the future? So the current accepted values would be output: "yes"|"none" and in the future it might be output: "yes"|"none"|"all" ? Or could be an Integer.

If this is not expected then it can stay boolean.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure here. To me having a one-to-one relation between opts and returned dictionary seems like a better design right now. So if opts.output = true, then output in result will represent output. If opts.error = true, then errors will be safely captured and result error will have those possible error captures. And so on.


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

See also: ~
|execute()|
Expand Down Expand Up @@ -1738,7 +1740,7 @@ nvim_cmd({*cmd}, {*opts}) *nvim_cmd()*
empty string.

See also: ~
|nvim_exec()|
|nvim_exec2()|
|nvim_command()|

*nvim_create_user_command()*
Expand Down
3 changes: 2 additions & 1 deletion runtime/doc/deprecated.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ Deprecated features
API
- *nvim_buf_clear_highlight()* Use |nvim_buf_clear_namespace()| instead.
- *nvim_buf_set_virtual_text()* Use |nvim_buf_set_extmark()| instead.
- *nvim_command_output()* Use |nvim_exec()| instead.
- *nvim_command_output()* Use |nvim_exec2()| instead.
- *nvim_execute_lua()* Use |nvim_exec_lua()| instead.
- *nvim_get_hl_by_name()* Use |nvim_get_hl()| instead.
- *nvim_get_hl_by_id()* Use |nvim_get_hl()| instead.
- *nvim_exec()* Use |nvim_exec2()| instead.

COMMANDS
- *:rv* *:rviminfo* Deprecated alias to |:rshada| command.
Expand Down
6 changes: 3 additions & 3 deletions runtime/doc/lua.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1355,9 +1355,9 @@ cmd({command}) *vim.cmd()*
Parameters: ~
{command} string|table Command(s) to execute. If a string, executes
multiple lines of Vim script at once. In this case, it is
an alias to |nvim_exec()|, where `output` is set to false.
Thus it works identical to |:source|. If a table, executes
a single command. In this case, it is an alias to
an alias to |nvim_exec2()|, where `opts.output` is set to
false. Thus it works identical to |:source|. If a table,
executes a single command. In this case, it is an alias to
|nvim_cmd()| where `opts` is empty.

See also: ~
Expand Down
2 changes: 2 additions & 0 deletions runtime/doc/news.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ The following changes may require adaptations in user config or plugins.

• Renamed vim.pretty_print to vim.print. |deprecated|

|nvim_exec()| is now deprecated in favor of |nvim_exec2()|.

==============================================================================
NEW FEATURES *news-features*

Expand Down
2 changes: 1 addition & 1 deletion runtime/lua/_vim9script.lua
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ local vim9 = (function()
end
end

vim.api.nvim_exec(table.concat(file, '\n'), false)
vim.api.nvim_exec2(table.concat(file, '\n'), { output = false })
end,
})
end
Expand Down
6 changes: 3 additions & 3 deletions runtime/lua/vim/_editor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ end
---
---@param command string|table Command(s) to execute.
--- If a string, executes multiple lines of Vim script at once. In this
--- case, it is an alias to |nvim_exec()|, where `output` is set to
--- false. Thus it works identical to |:source|.
--- case, it is an alias to |nvim_exec2()|, where `opts.output` is set
--- to false. Thus it works identical to |:source|.
--- If a table, executes a single command. In this case, it is an alias
--- to |nvim_cmd()| where `opts` is empty.
---@see |ex-cmd-index|
Expand All @@ -338,7 +338,7 @@ vim.cmd = setmetatable({}, {
if type(command) == 'table' then
return vim.api.nvim_cmd(command, {})
else
return vim.api.nvim_exec(command, false)
return vim.api.nvim_exec2(command, { output = false }).output
justinmk marked this conversation as resolved.
Show resolved Hide resolved
end
end,
__index = function(t, command)
Expand Down
2 changes: 1 addition & 1 deletion src/nvim/api/command.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ Dictionary nvim_parse_cmd(String str, Dictionary opts, Error *err)
///
/// On execution error: fails with VimL error, updates v:errmsg.
///
/// @see |nvim_exec()|
/// @see |nvim_exec2()|
/// @see |nvim_command()|
///
/// @param cmd Command to execute. Must be a Dictionary that can contain the same values as
Expand Down
17 changes: 15 additions & 2 deletions src/nvim/api/deprecated.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,26 @@
# include "api/deprecated.c.generated.h"
#endif

/// @deprecated Use nvim_exec2() instead.
/// @see nvim_exec2
String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
FUNC_API_SINCE(7)
FUNC_API_DEPRECATED_SINCE(11)
{
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(output);
return exec_impl(channel_id, src, &opts, err);
}

/// @deprecated
/// @see nvim_exec
/// @see nvim_exec2
String nvim_command_output(uint64_t channel_id, String command, Error *err)
FUNC_API_SINCE(1)
FUNC_API_DEPRECATED_SINCE(7)
{
return nvim_exec(channel_id, command, true, err);
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(true);
return exec_impl(channel_id, command, &opts, err);
}

/// @deprecated Use nvim_exec_lua() instead.
Expand Down
3 changes: 3 additions & 0 deletions src/nvim/api/keysets.lua
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,7 @@ return {
{ 'echo_opts', {
"verbose";
}};
{ 'exec_opts', {
"output";
}};
}
36 changes: 28 additions & 8 deletions src/nvim/api/vimscript.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,33 @@
/// @see |nvim_cmd()|
///
/// @param src Vimscript code
/// @param output Capture and return all (non-error, non-shell |:!|) output
/// @param opts Optional parameters.
/// - 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 `output` is true,
/// else empty string.
String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
FUNC_API_SINCE(7)
/// @return Dictionary containing information about execution, with these keys:
/// - output: (string|nil) Output if `opts.output` is true.
Dictionary nvim_exec2(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *err)
FUNC_API_SINCE(11)
{
Dictionary result = ARRAY_DICT_INIT;

String output = exec_impl(channel_id, src, opts, err);
if (ERROR_SET(err)) {
return result;
}

if (HAS_KEY(opts->output) && api_object_to_bool(opts->output, "opts.output", false, err)) {
PUT(result, "output", STRING_OBJ(output));
}

return result;
}

String exec_impl(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *err)
{
Boolean output = api_object_to_bool(opts->output, "opts.output", false, err);

const int save_msg_silent = msg_silent;
Copy link
Member

@justinmk justinmk Jun 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using TRY_WRAP may improve exception behavior. Don't need to make this optional.

see other functions in this file for TRY_WRAP example usage.

TRY_WRAP({

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinmk, didn't manage to find a way to do this. More thoughts in a recent comment.

garray_T *const save_capture_ga = capture_ga;
const int save_msg_col = msg_col;
Expand All @@ -69,7 +89,7 @@ String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)

const sctx_T save_current_sctx = api_set_sctx(channel_id);

do_source_str(src.data, "nvim_exec()");
do_source_str(src.data, "nvim_exec2()");
if (output) {
capture_ga = save_capture_ga;
msg_silent = save_msg_silent;
Expand Down Expand Up @@ -108,8 +128,8 @@ String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
///
/// On execution error: fails with VimL error, updates v:errmsg.
///
/// Prefer using |nvim_cmd()| or |nvim_exec()| over this. To evaluate multiple lines of Vim script
/// or an Ex command directly, use |nvim_exec()|. To construct an Ex command using a structured
/// Prefer using |nvim_cmd()| or |nvim_exec2()| over this. To evaluate multiple lines of Vim script
/// or an Ex command directly, use |nvim_exec2()|. To construct an Ex command using a structured
/// format and then execute it, use |nvim_cmd()|. To modify an Ex command before evaluating it, use
/// |nvim_parse_cmd()| in conjunction with |nvim_cmd()|.
///
Expand Down
7 changes: 5 additions & 2 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,8 +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);
String func_body = nvim_exec(VIML_INTERNAL_CALL, cstr_as_string(cmd),
true, &err);
Dict(exec_opts) opts = { 0 };
opts.output = BOOLEAN_OBJ(true);
String func_body = exec_impl(VIML_INTERNAL_CALL, cstr_as_string(cmd),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not call nvim_exec2, why do we need exec_impl ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short answer: I tried but couldn't do it. This worked, so decided to use it.

Long answer: I didn't manage to write proper C code to get output field from nvim_exec2() dictionary output.

Code for nvim_exec() was like this:

String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
  FUNC_API_SINCE(7)
  FUNC_API_DEPRECATED_SINCE(11)
{
  Dict(exec_opts) opts = { 0 };
  opts.output = BOOLEAN_OBJ(output);
  Dictionary exec2_output = nvim_exec2(channel_id, src, &opts, err);

  for (size_t i = 0; i < exec2_output.size; i++) {
    if (strequal(exec2_output.items[i].key.data, "output")) {
      return exec2_output.items[i].value.data.string;
    }
  }
  return (String)STRING_INIT;
}

I thought this should essentially be return nvim_exec2(src, { output = output }).output but it always returned empty string (while same nvim_exec2() call does return non-empty string). There seems to also be some potential memory leak, so Bjorn suggested to write a helper while leaving nvim_exec2() to construct output dictionary.

&opts, &err);
xfree(cmd);
if (!ERROR_SET(&err)) {
ADD(ctx->funcs, STRING_OBJ(func_body));
Expand Down
2 changes: 1 addition & 1 deletion test/deprecated.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
local M = {}

function M.redir_exec()
error('redir_exec is deprecated, use nvim_exec() or pcall_err()')
error('redir_exec is deprecated, use nvim_exec2() or pcall_err()')
end

return M
6 changes: 3 additions & 3 deletions test/functional/api/extmark_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1413,12 +1413,12 @@ describe('API/extmarks', function()
end)

it('does not crash with append/delete/undo sequence', function()
meths.exec([[
meths.exec2([[
let ns = nvim_create_namespace('myplugin')
call nvim_buf_set_extmark(0, ns, 0, 0, {})
call append(0, '')
%delete
undo]],false)
undo]], { output = false })
assert_alive()
end)

Expand Down Expand Up @@ -1450,7 +1450,7 @@ describe('API/extmarks', function()

feed('u')
-- handles pasting
meths.exec([[let @a='asdfasdf']], false)
meths.exec2([[let @a='asdfasdf']], { output = false })
feed([["ap]])
eq({ {1, 0, 0}, {2, 0, 8} },
meths.buf_get_extmarks(0, ns, 0, -1, {}))
Expand Down
12 changes: 8 additions & 4 deletions test/functional/api/keymap_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -681,13 +681,13 @@ describe('nvim_set_keymap, nvim_del_keymap', function()
end)

it('can set <expr> mappings whose RHS change dynamically', function()
meths.exec([[
meths.exec2([[
function! FlipFlop() abort
if !exists('g:flip') | let g:flip = 0 | endif
let g:flip = !g:flip
return g:flip
endfunction
]], 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_exec(':nmap asdf', 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
Loading