shell action helper nvim 0.8 compatibility

[ibhagwan]

Not sure what changed with neovim 0.8 but it causes shell actions to print an additional error to stderr.

Run the below code with the nightly:

local fzf = require("fzf").fzf
local raw_action = require("fzf.actions").raw_action

local raw_act_string = raw_action(function(args)
  return "selected: ".. args[1]
end)

coroutine.wrap(function()
  fzf({1, 2, 3, 4}, "--preview-window=nohidden --preview " ..
    vim.fn.shellescape(raw_act_string))
end)()

And you will see an additional error message in the preview as such:

This is actually theaction_server_address (our RPC server), the reason for this error is the way the shell helper works by utilizing slots 1-2 in the arglist for the server address and function ID, with neovim 0.8 it will attempt to open the temp pipe and fail thus printing the error in the screenshot.

I recently solved this in fzf-lua (https://github.com/ibhagwan/fzf-lua/issues/409) so I figured I'd save you a couple of hours of banging your head against neovim :-)

This PR changes the way the shell helper command is structured, instead of using luafile it uses lua loadfile(<file>)().fnc(<args>) {fzf_field_expression}, this way neovim never attempts to open the RPC pipe and fails.

[vijaymarupudi]

Thanks for the pull request! I have reverted to neovim stable so I did not notice this issue.

The problem is that nvim --headless interprets the arguments it has been passed as files in neovim 0.8, while neovim 0.7 did not do this. An strace into nvim reveals that it calls the stat() syscall on the argument. When the file doesn't exist, there is no problem, but when the file exists, it opens it the wrong way. We shouldn't be passing data that way either, so I think the approach taken in this PR is a nice way to do it.

One note about the PR, I worry that it will cause issues with paths with ']]' in their names. I know that this is very unlikely, but you never know how nvim will be used, and how lack of escaping might result in a rm -rf / being executed. The way to solve this issue would be to create a lua string escaping function that converts a string like testing ]] 1 2 to lua code like this [[testing]] .. "]]" .. [[ 1 2]]. This will then allow us to insert the string directly the way you have done it.

I don't have time to get to this soon as it is academic conference season, however I will get to it eventually it you haven't already!

[ibhagwan]

Ty for the insights @vijaymarupudi :)

One note about the PR, I worry that it will cause issues with paths with ']]' in their names. I know that this is very unlikely, but you never know how nvim will be used, and how lack of escaping might result in a rm -rf / being executed. The way to solve this issue would be to create a lua string escaping function that converts a string like testing ]] 1 2 to lua code like this [[testing]] .. "]]" .. [[ 1 2]]. This will then allow us to insert the string directly the way you have done it.

For the moment, that was a risk I’m willing to take, the path used here is in almost all cases the neovim config path under $XDG_HOME which is unlikely to have [[]], might even be solvable with nested vim.fn.shellescape but I didn’t think through it thoroughly.

[vijaymarupudi]

Found time today to implement the escaping, you could import the solution to fzf-lua. Greatly appreciate the pull request!

[ibhagwan]

Ty @vijaymarupudi looks great :)

[ibhagwan]

@vijaymarupudi, I did some testing with your lua_escape function and it fails when the string ends in a ]:

print(lua_escape("test]"))
-- results in the invalid string "[[test]]]"
-- trying to use this string will result in:
:lua print([[test]]])
E5107: Error loading lua [string ":lua"]:1: ')' expected near ']'

I think there's a simpler solution to that by using lua nested quotes, which are achieved by wrapping the string with [=[<string>]=] or [==[<string>]==] (instead of [[]]):

:lua print([=[test]]=])  -- prints "test]"
:lua print([=[test]]]=])  -- prints "test]]"
:lua print([==[test]]==])  -- prints "test]"

Change this: https://github.com/vijaymarupudi/nvim-fzf/blob/5ca0732c3437d7ac3857823962910e5634799ca3/lua/fzf/actions.lua#L61-L67

To this:

local call_arg_table_string = ("{ action_server=[==[%s]==], function_id=%d }"):format( 
    action_server_address, id)

local action_helper_vim_cmd = vim.fn.shellescape(("lua loadfile([==[%s]==])().rpc_nvim_exec_lua(%s)") 
    :format(nvim_fzf_directory .. "/action_helper.lua", call_arg_table_string)) 

[vijaymarupudi]

Good catch! I just pushed a fix for that here https://github.com/vijaymarupudi/nvim-fzf/commit/d0b4d60704f27eddaa96b921f10a4a2a20f1b20e

I think the problem with the nested quotes solution is that you'd have a problem with strings like "test]==". Then that would become [==[test]==]==] which is invalid lua code. I think the escaping solution implemented now, thanks to your testing, doesn't have any such limitations.