EdenEast / nightfox.nvim

🦊A highly customizable theme for vim and neovim with support for lsp, treesitter and a variety of plugins.
MIT License
2.91k stars 136 forks source link

Fixes for Vim 9.0.1894 + liblua 5.1 #370

Closed achadwick closed 9 months ago

achadwick commented 9 months ago

Hi there! The Vim I use under current Debian testing was showing a few errors since 741366250cff3aa7b85d6588ffc9db60ebbfdcc9. I'm not great with Lua, but I like this colour scheme enough to attempt a fix! Let me know if anything looks wrong with this patch.

Looks like liblua 5.1 deprecates [[ inside a [[ ... ]] string. 01962d86527a8bb70b064caf7421d563582765a2 fixes an error message there. Fixes the error message

Error detected while processing /home/andrewc/.vimrc[1321]../home/andrewc/.vimrc.local[13]../home/andrewc/Configs/Dotfiles/vim/.vim/bundle/nightfox.nvim/colors/dawnfox.vim
line    9:
error loading module 'nightfox.lib.vim.compiler' from file '/home/andrewc/Configs/Dotfiles/vim/.vim/bundle/nightfox.nvim/lua/nightfox/lib/vim/compiler.lua':
^I...Dotfiles/vim/.vim/bundle/nightfox.nvim/lua/nightfox/lib/vim/compiler.lua:22: nesting of [[...]] is deprecated near '['

I think the backwards compatibility code for load vs loadstring in main is in the wrong order. Both functions exist in both 5.1 and 5.2, but if 5.1 tries to use load, and it will, you get

Error detected while processing /home/andrewc/.vimrc[1321]../home/andrewc/.vimrc.local[13]../home/andrewc/Configs/Dotfiles/vim/.vim/bundle/nightfox.nvim/colors/dawnfox.vim
line    9:
...ndle/nightfox.nvim/lua/nightfox/lib/vim/compiler.lua:75: bad argument #1 to 'ld' (function expected, got string)

Perhaps it makes sense to test for loadstring first, even if it is deprecated in 5.2. That means it should still work, I hope. If this fails under 5.2, then duck typing won't fix this. It'll need to test _VERSION or something.

(The failure conditions breaks the theme and makes all the colours go screwy, so I'm motivated to get this right)

EdenEast commented 9 months ago

Thanks for the PR. It is interesting about these failures. I am not able to reproduce them when I try to install vim with lua support and different lua versions. I only attempted using the nix flake in the repo.

For the double bracket deprecation, from searching you can apparently change the outer [[ ... ]] to be [=[ ... ]=]. That would resolve the nested bracket failure.

As for the the string loading, I was not able to reproduce the load failure. When looking at the different versions of lua only lua 5.1 contains the function loadstring. This should only be used on lua 5.1 and fallback to load for all other versions.

Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print(loadstring)
function: 0x146ee80
> print(load)
function: 0x146ee20

Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
> print(loadstring)
nil
> print(load)
function: 0x41d080

Lua 5.3.6  Copyright (C) 1994-2020 Lua.org, PUC-Rio
> print(loadstring)
nil
> print(load)
function: 0x41e810

Lua 5.4.6  Copyright (C) 1994-2023 Lua.org, PUC-Rio
> print(loadstring)
nil
> print(load)
function: 0x425590

Could you let me know what version of lua you are using? Also do you get the same results as me if you are using 5.1.

Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> f = loadstring or load
> print(f, loadstring, load)
function: 0x5afe80      function: 0x5afe80      function: 0x5afe20

From this you can see that f is pointing to the loadstring function location.

Also just for my sanity I did this same test in lua 5.2 with the expected results:

Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
> f = loadstring or load
> print(f, loadstirng, load)
function: 0x41d080      nil     function: 0x41d080
jgb commented 9 months ago

Hello,

I tested the newest commit, this fixes the original error message of [[...]] being deprecated, but now it still doesn't work because of a new error:

nightfox/lib/vim/compiler.lua:75: bad argument #1 to 'ld' (function expected, got string)

So I think it's necessary to merge all of the code changes in this PR to fully make things work on debian with lua 5.1. When I make those changes locally by hand, everything works fine.

Thanks

EdenEast commented 9 months ago

Sorry looked back at it I had it backwards and let it is true that it should be switched. That is what the test I did before shows me.

achadwick commented 9 months ago

Thanks for investigating and fixing the issue, chaps! :1st_place_medal: