R-nvim / R.nvim

Neovim plugin to edit R files
GNU General Public License v3.0
132 stars 15 forks source link

`vim.system()` api did not exist in Neovim current release (v0.9.5) #36

Closed hongyuanjia closed 4 months ago

hongyuanjia commented 4 months ago

Thank you all for your efforts! Currently, I failed to use R.nvim on the current Neovim release. (v0.9.5). vim.system() Lua API was added in PR https://github.com/neovim/neovim/pull/23827, which was before when Neovim v0.9.5 was released. However, somehow, it was not included in the current Neovim release. Since vim.system() was a wrapper around vim.loop.spawn(), maybe we can use the latter directly? Or at least note that R.nvim only works for the development version of Neovim.

Error detected while processing BufReadPost Autocommands for "*":
Error executing lua callback: ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24: Error executing lua: ...coop/apps/neovim/current/share/nvim/runt
ime/filetype.lua:25: BufReadPost Autocommands for "*"..FileType Autocommands for "*"..function <SNR>1_LoadFTPlugin[19]..script C:\Users\hongyuanjia\AppData\Lo
cal\nvim-data\lazy\tmp-R-nvim\ftplugin\r_rnvim.lua: Vim(runtime):E5113: Error while calling lua chunk: ...AppData/Local/nvim-data/lazy/tmp-R-nvim/lua/r/config
.lua:543: attempt to call field 'system' (a nil value)
stack traceback:
        ...AppData/Local/nvim-data/lazy/tmp-R-nvim/lua/r/config.lua:543: in function 'get_rip'
        ...AppData/Local/nvim-data/lazy/tmp-R-nvim/lua/r/config.lua:553: in function 'windows_config'
        ...AppData/Local/nvim-data/lazy/tmp-R-nvim/lua/r/config.lua:698: in function 'global_setup'
        ...AppData/Local/nvim-data/lazy/tmp-R-nvim/lua/r/config.lua:799: in function 'real_setup'
        ...ata/Local/nvim-data/lazy/tmp-R-nvim/ftplugin/r_rnvim.lua:12: in main chunk
        [C]: in function 'nvim_cmd'
        ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:25: in function <...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24>
        [C]: in function 'nvim_buf_call'
        ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24: in function <...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:10>
stack traceback:
        [C]: in function 'nvim_cmd'
        ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:25: in function <...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24>
        [C]: in function 'nvim_buf_call'
        ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24: in function <...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:10>
stack traceback:
        [C]: in function 'nvim_buf_call'
        ...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:24: in function <...coop/apps/neovim/current/share/nvim/runtime/filetype.lua:10>
Press ENTER or type command to continue

Neovim version:

:version
NVIM v0.9.5
Build type: RelWithDebInfo
LuaJIT 2.1.1703942320
Compilation: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe /MD /Zi /O2 /Ob1  -W3 -wd4311 -wd
4146 -DUNIT_TESTING -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_WIN32_WINNT=0x0602 -DMSWIN -DINCLUDE_GENERATED_DECLARATIONS -ID:/a/neovim/neovim/
.deps/usr/include/luajit-2.1 -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/build/src/nvim/auto -ID:/a/neo
vim/neovim/build/include -ID:/a/neovim/neovim/build/cmake.config -ID:/a/neovim/neovim/src -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/.deps/us
r/include -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/.deps/usr/include -ID:/a/neovim/neovim/.deps/usr/
include -ID:/a/neovim/neovim/.deps/usr/include

   system vimrc file: "$VIM\sysinit.vim"
  fall-back for $VIM: "C:/Program Files (x86)/nvim/share/nvim"

Run :checkhealth for more info
jalvesaq commented 4 months ago

I didn't know that. I saw somewhere a recommendation of using vim.system() instead of vim.fn.system() and remembered that Vim's function system() did not work properly on Windows (that's why the use a .bat file in Nvim-R).

I see no problem in using vim.loop.spaw(). We have only to remember of renaming vim.loop as vim.uv before vim.loop is definitively deprecated. But we already use vim.loop at other places. So we would have to do this anyway.

Can you do the replacement?

akthe-at commented 4 months ago

Okay so I got tmp-R-Nvim up and running with Windows and neovim nightly. Will use it during work as my daily driver with R tomorrow and see if any issues pop up.

jalvesaq commented 4 months ago

Great news, @akthe-at !

Looking at Neovim git log, vim.system() was added on June 2023, but was not included in the next two releases ("focusing on bug fixes"). So, maybe we should take a step back and replace vim.system with vim.fn.system (if we need to wait for the output and it works on both Linux and Windows), jobstart (if we don't need the result), or vim.uv.spaw as a last resort.

hongyuanjia commented 4 months ago

@jalvesaq Please see #41. I added a utils.system() to mimic vim.system(), and it works on Windows.

jalvesaq commented 4 months ago

Thanks! I will try it.