From 40ce8577977fcdce8ad76863c70eb522e4cefd4d Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 3 May 2024 03:20:03 -0700 Subject: [PATCH] fix(vim.ui)!: change open() to return `result|nil, errmsg|nil` #28612 reverts e0d92b9cc20b58179599f53dfa74ca821935a539 #28502 Problem: `vim.ui.open()` has a `pcall()` like signature, under the assumption that this is the Lua idiom for returning result-or-error. However, the `result|nil, errmsg|nil` pattern: - has precedent in: - `io.open` - `vim.uv` (`:help luv-error-handling`) - has these advantages: - Can be used with `assert()`: ``` local result, err = assert(foobar()) ``` - Allows LuaLS to infer the type of `result`: ``` local result, err = foobar() if err then ... elseif result then ... end ``` Solution: - Revert to the `result|nil, errmsg|nil` pattern. - Document the pattern in our guidelines. --- runtime/doc/develop.txt | 5 +++++ runtime/doc/lua.txt | 8 ++++---- runtime/doc/news.txt | 2 +- runtime/lua/vim/_defaults.lua | 15 +++++++-------- runtime/lua/vim/lsp/handlers.lua | 6 +++--- runtime/lua/vim/ui.lua | 14 +++++++------- test/functional/lua/ui_spec.lua | 2 +- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/runtime/doc/develop.txt b/runtime/doc/develop.txt index b843988b94..f2eef7b131 100644 --- a/runtime/doc/develop.txt +++ b/runtime/doc/develop.txt @@ -309,6 +309,11 @@ See also |dev-naming|. - return iterable instead of table - mimic the pairs() or ipairs() interface if the function is intended to be used in a "for" loop. + - when a result-or-error interface is needed, return `result|nil, errmsg|nil`: > + ---@return Foo|nil # Result object, or nil if not found. + ---@return nil|string # Error message on failure, or nil on success. +< + - Examples: |vim.ui.open()| |io.open()| |luv-error-handling| Interface conventions ~ diff --git a/runtime/doc/lua.txt b/runtime/doc/lua.txt index 2b1d20feda..4f538f52ab 100644 --- a/runtime/doc/lua.txt +++ b/runtime/doc/lua.txt @@ -2551,8 +2551,8 @@ vim.ui.open({path}) *vim.ui.open()* vim.ui.open("https://neovim.io/") vim.ui.open("~/path/to/file") -- Synchronous (wait until the process exits). - local ok, cmd = vim.ui.open("$VIMRUNTIME") - if ok then + local cmd, err = vim.ui.open("$VIMRUNTIME") + if cmd then cmd:wait() end < @@ -2561,8 +2561,8 @@ vim.ui.open({path}) *vim.ui.open()* • {path} (`string`) Path or URL to open Return (multiple): ~ - (`boolean`) false if command not found, else true. - (`vim.SystemObj|string`) Command object, or error message on failure + (`vim.SystemObj?`) Command object, or nil if not found. + (`string?`) Error message on failure, or nil on success. See also: ~ • |vim.system()| diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index 78d5560878..3886dbdfb0 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -163,7 +163,7 @@ cycle (Nvim HEAD, the "master" branch). • Renamed vim.tbl_isarray() to vim.isarray(). -• Changed |vim.ui.open()| return-signature to match pcall() convention. +• Changed |vim.ui.open()| return-signature to match `result|nil, errormsg|nil` convention. • Renamed Iter:nextback() to Iter:pop() diff --git a/runtime/lua/vim/_defaults.lua b/runtime/lua/vim/_defaults.lua index 29f8e71264..6d31a3ea93 100644 --- a/runtime/lua/vim/_defaults.lua +++ b/runtime/lua/vim/_defaults.lua @@ -98,19 +98,18 @@ do --- Map |gx| to call |vim.ui.open| on the at cursor. do local function do_open(uri) - local ok, cmd_or_err = vim.ui.open(uri) - local rv = ok and (cmd_or_err --[[@as vim.SystemObj]]):wait(1000) or nil - if rv and rv.code ~= 0 then - ok = false - cmd_or_err = ('vim.ui.open: command %s (%d): %s'):format( + local cmd, err = vim.ui.open(uri) + local rv = cmd and cmd:wait(1000) or nil + if cmd and rv and rv.code ~= 0 then + err = ('vim.ui.open: command %s (%d): %s'):format( (rv.code == 124 and 'timeout' or 'failed'), rv.code, - vim.inspect(cmd_or_err.cmd) + vim.inspect(cmd.cmd) ) end - if not ok then - vim.notify(cmd_or_err --[[@as string]], vim.log.levels.ERROR) + if err then + vim.notify(err, vim.log.levels.ERROR) end end diff --git a/runtime/lua/vim/lsp/handlers.lua b/runtime/lua/vim/lsp/handlers.lua index ab4fa52c40..4672d94105 100644 --- a/runtime/lua/vim/lsp/handlers.lua +++ b/runtime/lua/vim/lsp/handlers.lua @@ -654,15 +654,15 @@ M[ms.window_showDocument] = function(_, result, ctx, _) if result.external then -- TODO(lvimuser): ask the user for confirmation - local ok, cmd_or_err = vim.ui.open(uri) - local ret = ok and (cmd_or_err --[[@as vim.SystemObj]]):wait(2000) or nil + local cmd, err = vim.ui.open(uri) + local ret = cmd and cmd:wait(2000) or nil if ret == nil or ret.code ~= 0 then return { success = false, error = { code = protocol.ErrorCodes.UnknownErrorCode, - message = ret and ret.stderr or cmd_or_err, + message = ret and ret.stderr or err, }, } end diff --git a/runtime/lua/vim/ui.lua b/runtime/lua/vim/ui.lua index b8323efa66..3c947c51b0 100644 --- a/runtime/lua/vim/ui.lua +++ b/runtime/lua/vim/ui.lua @@ -118,16 +118,16 @@ end --- vim.ui.open("https://neovim.io/") --- vim.ui.open("~/path/to/file") --- -- Synchronous (wait until the process exits). ---- local ok, cmd = vim.ui.open("$VIMRUNTIME") ---- if ok then +--- local cmd, err = vim.ui.open("$VIMRUNTIME") +--- if cmd then --- cmd:wait() --- end --- ``` --- ---@param path string Path or URL to open --- ----@return boolean # false if command not found, else true. ----@return vim.SystemObj|string # Command object, or error message on failure +---@return vim.SystemObj|nil # Command object, or nil if not found. +---@return nil|string # Error message on failure, or nil on success. --- ---@see |vim.system()| function M.open(path) @@ -147,7 +147,7 @@ function M.open(path) if vim.fn.executable('rundll32') == 1 then cmd = { 'rundll32', 'url.dll,FileProtocolHandler', path } else - return false, 'vim.ui.open: rundll32 not found' + return nil, 'vim.ui.open: rundll32 not found' end elseif vim.fn.executable('wslview') == 1 then cmd = { 'wslview', path } @@ -156,10 +156,10 @@ function M.open(path) elseif vim.fn.executable('xdg-open') == 1 then cmd = { 'xdg-open', path } else - return false, 'vim.ui.open: no handler found (tried: wslview, explorer.exe, xdg-open)' + return nil, 'vim.ui.open: no handler found (tried: wslview, explorer.exe, xdg-open)' end - return true, vim.system(cmd, { text = true, detach = true }) + return vim.system(cmd, { text = true, detach = true }), nil end return M diff --git a/test/functional/lua/ui_spec.lua b/test/functional/lua/ui_spec.lua index 3f5c051347..d69e893c96 100644 --- a/test/functional/lua/ui_spec.lua +++ b/test/functional/lua/ui_spec.lua @@ -144,7 +144,7 @@ describe('vim.ui', function() end if not is_os('bsd') then local rv = - exec_lua [[local _, cmd = vim.ui.open('non-existent-file'); return cmd:wait(100).code]] + exec_lua [[local cmd = vim.ui.open('non-existent-file'); return cmd:wait(100).code]] ok(type(rv) == 'number' and rv ~= 0, 'nonzero exit code', rv) end