Move plugins to Sol. #5622
Move plugins to Sol. #5622
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
CMakeLists.txt
Outdated
| @@ -212,6 +212,7 @@ endif() | |||
| if (CHATTERINO_PLUGINS) | |||
| set(LUA_INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/lib/lua/src") | |||
| add_subdirectory(lib/lua) | |||
| add_definitions(-DSOL_ALL_SAFETIES_ON=1) | |||
There was a problem hiding this comment.
This is done in FindSol2.cmake already. Afaik, these INTERFACE definitions propagate to all targets that (transitively) use them (but I haven't checked).
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Mm2PL
force-pushed
the
chore/move-everything-to-sol
branch
from
d2a86e4 to
df915d4
Compare
Mm2PL
force-pushed
the
chore/move-everything-to-sol
branch
from
df915d4 to
19cb8db
Compare
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Nerixyz <nerixdev@outlook.de>
For #include reasons
including tab completion
The public postman-echo service returns different data
…/move-everything-to-sol
| )lua")); | ||
| } | ||
|
|
||
| TEST_F(PluginTest, requireNoData) |
There was a problem hiding this comment.
There's no test for a successful require (could be in a followup).
| // If you're modifying this PLEASE verify it works, Sol is very annoying about serialization | ||
| // - Mm2PL | ||
| sol::state_view lua(s); | ||
| auto load = lua.registry()["real_load"]; | ||
| sol::protected_function_result ret = load(data, "=(load)", "t"); | ||
| return ret; |
There was a problem hiding this comment.
If we have this comment, we should test it. Maybe a test that's only in debug mode?
There was a problem hiding this comment.
Added a comment on the function for this
| vtypes = [] | ||
| for variant in variants: | ||
| vtype = f'{name}.{variant}' | ||
| vtypes.append(vtype) | ||
| out.write(f'---@alias {vtype} "{vtype}"\n') | ||
|
|
||
| out.write(f"---@alias {name} {'|'.join(vtypes)}\n") |
There was a problem hiding this comment.
Why is this done? ---@alias c2.LogLevel.Debug "c2.LogLevel.Debug" seems wrong. It defines a type c2.LogLevel.Debug to be the literal string c2.LogLevel.Debug.
There was a problem hiding this comment.
Yes, this is so the language server doesn't go number == number and instead will show the right overload.
There was a problem hiding this comment.
This doesn't work:
Seems like what we really want is ---@enum <name>.
Diff
diff --git a/scripts/make_luals_meta.py b/scripts/make_luals_meta.py
index b1420e780..cb7b3cdf4 100644
--- a/scripts/make_luals_meta.py
+++ b/scripts/make_luals_meta.py
@@ -242,25 +242,16 @@ def read_file(path: Path, out: TextIOWrapper):
)
name = header[0].split(" ", 1)[1]
printmsg(path, reader.line_no(), f"enum {name}")
- variants = reader.read_enum_variants()
-
- vtypes = []
- for variant in variants:
- vtype = f'{name}.{variant}'
- vtypes.append(vtype)
- out.write(f'---@alias {vtype} "{vtype}"\n')
-
- out.write(f"---@alias {name} {'|'.join(vtypes)}\n")
if header_comment:
out.write(f"--- {header_comment}\n")
- out.write("---@type { ")
+ out.write(f"---@enum {name}\n")
+ out.write(f"{name} = {{\n")
out.write(
- ", ".join(
- [f"{variant}: {typ}" for variant, typ in zip(variants,vtypes)]
+ "\n".join(
+ [f" {variant} = 0," for variant in reader.read_enum_variants()]
)
)
- out.write(" }\n")
- out.write(f"{name} = {{}}\n\n")
+ out.write("\n}\n\n")
continue
# classRegardless, this isn't related to the SOL migration.
There was a problem hiding this comment.
The patch you've sent makes luals fail to typecheck a simple example:
c2.register_callback(c2.EventType.CompletionRequested, function(ev)end)
--[[
Cannot assign `integer` to parameter `c2.EventType.CompletionRequested`.
- `integer` cannot match `c2.EventType.CompletionRequested`
- Type `integer` cannot match `c2.EventType.CompletionRequested`
- Type `number` cannot match `c2.EventType.CompletionRequested`",
]]For me it also suggests using raw enum values which are all zero which feels even worse than getting strings.
There was a problem hiding this comment.
Oops, I thought we used numbers for enums. Generating strings here shouldn't be too hard, though.
There was a problem hiding this comment.
We actually currently can pick whichever representation suits us, the documentation states that the values may be changed any time and accessing through the enum table is the recommended option.
| /** | ||
| * @brief Dumps the Lua stack into qCDebug(chatterinoLua) | ||
| * | ||
| * @param tag is a string to let you know which dump is which when browsing logs | ||
| */ | ||
| void stackDump(lua_State *L, const QString &tag); | ||
|
|
||
| // This is for calling stackDump out of gdb as it's not easy to create a QString there | ||
| const QString GDB_DUMMY = "GDB_DUMMY"; |
There was a problem hiding this comment.
Shouldn't this be in the source file?
There was a problem hiding this comment.
It needs to be in a header so it's accessible. It could have a default value assigned in a source file although that might have linking issues.
There was a problem hiding this comment.
Could this be a null-string? That would be constexpr + all null. I guess we can't have it be inline because we don't use it.
One should be able to debug the lua stack without stackDump though. For GDB there's https://github.com/mkottman/lua-gdb-helper (might be a bit outdated). And for VS there's https://github.com/huoyang11/lua_debug_natvis (that one is a bit outdated and incompatible with qt, but not too hard to fix). There's probably something for LLDB as well.
|
Before I forget this, plugins are not enabled in the macOS and Windows tests. |
maybe unneccesary
|
Quick patch to be able to compile with clang-cl: diff --git a/lib/lua/CMakeLists.txt b/lib/lua/CMakeLists.txt
index 9d7aaef90..70a4033b5 100644
--- a/lib/lua/CMakeLists.txt
+++ b/lib/lua/CMakeLists.txt
@@ -52,5 +52,8 @@ set_target_properties(${liblua} PROPERTIES
CXX_STANDARD 98
CXX_EXTENSIONS TRUE
)
-target_compile_options(lua PRIVATE -w) # this makes clang shut up about c-as-c++
+target_compile_options(lua PRIVATE
+ -w # this makes clang shut up about c-as-c++
+ $<$<AND:$<BOOL:${MSVC}>,$<CXX_COMPILER_ID:Clang>>:/EHsc> # enable exceptions in clang-cl
+)
set_source_files_properties(${LUA_SRC} PROPERTIES LANGUAGE CXX)
|
Co-authored-by: Nerixyz <nerixdev@outlook.de>
Diffdiff --git a/tests/src/Plugins.cpp b/tests/src/Plugins.cpp
index e7951d4d0..156b442d5 100644
--- a/tests/src/Plugins.cpp
+++ b/tests/src/Plugins.cpp
@@ -375,8 +375,10 @@ TEST_F(PluginTest, testHttp)
{"/status/200", true, false, 200, "200"},
{"/delay/2", false, true, 0, "TimeoutError"},
# ifdef CHATTERINO_TEST_USE_PUBLIC_HTTPBIN
-# warning \
- "Cannot test data returned from httpbin. The public one does not return data verbatim."
+# if __cplusplus >= 202302L || defined(Q_CC_GNU) || defined(Q_CC_CLANG)
+# warning \
+ "Cannot test data returned from httpbin. The public one does not return data verbatim."
+# endif
# else
{"/post", true, false, 200, "200", NetworkRequestType::Post,
"Example data"},
@@ -543,13 +545,15 @@ TEST_F(PluginTest, testTimerRec)
sol::protected_function fn = lua->script(R"lua(
local i = 0
+ local called_done = false
f = function()
i = i + 1
c2.log(c2.LogLevel.Debug, "cb", i)
if i < 1024 then
c2.later(f, 1)
- else
+ elseif not called_done then
done()
+ called_done = true
end
end
c2.later(f, 1) |
Co-authored-by: Nerixyz <nerixdev@outlook.de>
This PR moves the existing plugin implementation to sol2 from using the raw Lua API.
TODO: