Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua 5.2+ compatbility #9280

Merged
merged 3 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ if(LUACHECK_PRG)
-P ${PROJECT_SOURCE_DIR}/cmake/RunLuacheck.cmake)

add_custom_target(
blobcodelint
lintbuiltinlua
COMMAND
${CMAKE_COMMAND}
-DLUACHECK_PRG=${LUACHECK_PRG}
Expand All @@ -697,10 +697,21 @@ if(LUACHECK_PRG)
-DREAD_GLOBALS=vim
-P ${PROJECT_SOURCE_DIR}/cmake/RunLuacheck.cmake
)
add_custom_target(
lintruntimelua
COMMAND
${CMAKE_COMMAND}
-DLUACHECK_PRG=${LUACHECK_PRG}
-DLUAFILES_DIR=${CMAKE_CURRENT_SOURCE_DIR}/runtime/lua
-DCMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAME}
-DREAD_GLOBALS=vim
-P ${PROJECT_SOURCE_DIR}/cmake/RunLuacheck.cmake
)
# TODO(ZyX-I): Run linter for all lua code in src
add_custom_target(
lualint
DEPENDS blobcodelint
DEPENDS lintruntimelua
DEPENDS lintbuiltinlua
)
endif()

Expand Down
5 changes: 4 additions & 1 deletion cmake/RunTests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ set(ENV{SYSTEM_NAME} ${SYSTEM_NAME})
execute_process(
COMMAND ${BUSTED_PRG} ${TEST_TAG} ${TEST_FILTER} -v -o ${BUSTED_OUTPUT_TYPE}
--lua=${LUA_PRG} --lazy --helper=${TEST_DIR}/${TEST_TYPE}/preload.lua
--lpath=${BUILD_DIR}/?.lua --lpath=?.lua ${TEST_PATH}
--lpath=${BUILD_DIR}/?.lua
--lpath=${WORKING_DIR}/runtime/lua/?.lua
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a try at sharing code between tests/runtime/core. Maybe we will do it differently, I will have a followup PR for further discussion, after this one.

Related: #8677 cc @KillTheMule

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand makefiles, so just a thought: Is ? recursive, so it also catches runtime/lua/plugin/init.lua? Also, does the test runner have access to vim.api? If so, great boon to writing tests!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's recursive.

Also, does the test runner have access to vim.api? If so, great boon to writing tests!

vim.api is built-into Nvim so that would not be "shared". The test utilities could spawn nvim to use its features. We probably should do that, to reduce some of the code in */helpers.lua.

--lpath=?.lua
${TEST_PATH}
WORKING_DIRECTORY ${WORKING_DIR}
ERROR_VARIABLE err
RESULT_VARIABLE res
Expand Down
8 changes: 5 additions & 3 deletions runtime/lua/man.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require('vim.compat')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to do this implicitly instead. But let's discuss that later.


local buf_hls = {}

local function highlight_line(line, linenr)
Expand All @@ -10,9 +12,9 @@ local function highlight_line(line, linenr)
local attr = NONE
local byte = 0 -- byte offset

local function end_attr_hl(attr)
local function end_attr_hl(attr_)
for i, hl in ipairs(hls) do
if hl.attr == attr and hl.final == -1 then
if hl.attr == attr_ and hl.final == -1 then
hl.final = byte
hls[i] = hl
end
Expand Down Expand Up @@ -106,7 +108,7 @@ local function highlight_line(line, linenr)
-- the range 0x20 - 0x3f, then 'm'. (See ECMA-48, sections 5.4 & 8.3.117)
local sgr = prev_char:match("^%[([\032-\063]*)m$")
if sgr then
local match = ''
local match
while sgr and #sgr > 0 do
-- Match against SGR parameters, which may be separated by ';'
match, sgr = sgr:match("^(%d*);?(.*)")
Expand Down
12 changes: 12 additions & 0 deletions runtime/lua/vim/compat.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- Lua 5.1 forward-compatibility layer.
-- For background see https://github.com/neovim/neovim/pull/9280
--
-- Reference the lua-compat-5.2 project for hints:
-- https://github.com/keplerproject/lua-compat-5.2/blob/c164c8f339b95451b572d6b4b4d11e944dc7169d/compat52/mstrict.lua
-- https://github.com/keplerproject/lua-compat-5.2/blob/c164c8f339b95451b572d6b4b4d11e944dc7169d/tests/test.lua

local lua_version = _VERSION:sub(-3)

if lua_version >= "5.2" then
mcepl marked this conversation as resolved.
Show resolved Hide resolved
unpack = table.unpack -- luacheck: ignore 121 143
end
1 change: 1 addition & 0 deletions test/functional/helpers.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require('vim.compat')
require('coxpcall')
local luv = require('luv')
local lfs = require('lfs')
Expand Down