search

numToStr / Navigator.nvim

:sparkles: Smoothly navigate between neovim and terminal multiplexer(s) :sparkles:
MIT License
391 stars 21 forks source link

use correct syntax for file read to avoid E5108 in nvim #5

Closed invine closed 3 years ago

invine commented 3 years ago

Following error pops up when require("Navigator") command is used with any direction: E5108: Error executing lua .../pack/packer/start/Navigator.nvim/lua/Navigator/tmux.lua:28: bad argument #1 to 'read' (invalid option) According to documentation http://www.lua.org/manual/5.1/manual.html#5.7 "*l" option should be passed to file:read() in order to read a new line instead of "l". Proposed change gets rid of E5108 on tab switch between nvim and tmux

numToStr commented 3 years ago

Thanks for the patch. Can you give me some reference doc to read more about this? As I never faced this issue with my setup.

invine commented 3 years ago

Added ref to docs in pr description. Forgot it for some reason. As far as i know, neovim uses lua jit which is syntactically equal to lua 5.1. I also checked lua 5.3 just in case and syntax is the same.

numToStr commented 3 years ago

According to the docs file:read('*l') is same as file:read(), *l being the default. So, IMO we can totally remove that part and for that, there is already a PR #4.

If you don't mind we can go with that :)

bresilla commented 3 years ago

Following error pops up when require("Navigator") command is used with any direction: E5108: Error executing lua .../pack/packer/start/Navigator.nvim/lua/Navigator/tmux.lua:28: bad argument #1 to 'read' (invalid option) According to documentation http://www.lua.org/manual/5.1/manual.html#5.7 "*l" option should be passed to file:read() in order to read a new line instead of "l". Proposed change gets rid of E5108 on tab switch between nvim and tmux

Thank you ;) The file:read('*l') fixed the error message for me :)

invine commented 3 years ago

According to the docs file:read('*l') is same as file:read(), *l being the default. So, IMO we can totally remove that part and for that, there is already a PR #4.

If you don't mind we can go with that :)

Sure, that would work too

numToStr commented 3 years ago

So, now #4 is merged. I think the issue will be fixed.

Thanks for the contribution.