keaising / im-select.nvim

Switch Input Method automatically depends on Neovim's edit mode
MIT License
170 stars 25 forks source link

refactor: use vim.loop.spawn when calling im-select #22

Closed hongyuanjia closed 7 months ago

hongyuanjia commented 7 months ago

Pull request overview

我遇到了和 #16 一样的问题。我看到了 plenary.job 里使用了 vim.loop.spawn,所以就试着把 vim.fn.jobstart 替换成了 vim.loop.spwan,发现 #16 中的问题消失,在Windows上运行非常顺畅,至于具体原因是什么,我不太清楚。或者在Windows上vim.fn.jobstart直接调用CMD有关?由于我对Lua不是很熟悉,如果代码有不规范或者错误的地方还请指出。

keaising commented 7 months ago

Looks good!

I want to wait madjxatw comment for several days.

If this PR looks good for they, I will merge it.

hongyuanjia commented 7 months ago

Thanks. Let's wait for @madjxatw's feedback

keaising commented 7 months ago

I have test it in macOS and WSL2, works fine for me.

madjxatw commented 7 months ago

Thanks, @hongyuanjia , I've confirmed it works well with vim.loop.spawn(). A little more info I want to share is that the real cause of this issue is not jobstart() itself but the type of the first argument passed in to it. Let's see what the Neovim doc says:

jobstart({cmd} [, {opts}])              *jobstart()*
        Spawns {cmd} as a job.
        If {cmd} is a List it runs directly (no 'shell').
        If {cmd} is a String it runs in the 'shell', like this:  
          :call jobstart(split(&shell) + split(&shellcmdflag) + ['{cmd}'])

It tells that jobstart() can take either a list or a string object as its first argument. When a string is given, the {cmd} is executed by the shell program which defaults to cmd.exe on Windows, and this does open a cmd.exe window each time nvim switches back to Normal mode from Insert mode. But sorry that I have no time to dig why it only happens on some Windows 11 machines (like yours and mine) but not on Windows 10. Anyway, there is no need to concatenate a list into a string for joystart to work, the following changes could also fix the issue.

diff --git a/lua/im_select.lua b/lua/im_select.lua
index 998ff0a..64f8d92 100644
--- a/lua/im_select.lua
+++ b/lua/im_select.lua
@@ -145,9 +145,13 @@ local function change_im_select(cmd, method)
     end

     if C.async_switch_im then
-        vim.fn.jobstart(table.concat(command, " "), { detach = true })
+        -- Fix: pass a list instead of a string to joystart to prevent the IME
+        -- switching command from being executed via shell.
+        vim.fn.jobstart(command, { detach = true })
     else
-        local jobid = vim.fn.jobstart(table.concat(command, " "), { detach = false })
+        local jobid = vim.fn.jobstart(command, { detach = false })
         vim.fn.jobwait({ jobid }, 200)
     end
 end

Hi, @keaising , could you please check my fix as well? Thanks for your plugin again!

keaising commented 7 months ago

@madjxatw Thanks for your feedback.

Your fix works well on my PC, and I would like to merge this pull request after I read a lot docs and articles about async jobs in NeoVim, I think libuv is a better choice for im-select.nvim.