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

lua5.1 compatibility #206

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

lua5.1 compatibility #206

wants to merge 2 commits into from

Conversation

smarek
Copy link
Contributor

@smarek smarek commented Aug 10, 2020

This directly provides compiler functions, required by updated Lua runtime, see kaitai-io/kaitai_struct_lua_runtime#5 for reference

This PR will need the tests to be re-compiled, since Lua 5.1 does not support unicode escape sequence, i've switched to UTF8 decimal encoding, this fixes tests StrEncodings and StrEncodingsDefault (along with SJIS support added in Lua runtime)

Example diff over kaitai_struct_tests after merging this looks like this

> git diff spec/lua/test_s*
diff --git a/spec/lua/test_str_encodings.lua b/spec/lua/test_str_encodings.lua
index d0e0531f..26b8c8e6 100644
--- a/spec/lua/test_str_encodings.lua
+++ b/spec/lua/test_str_encodings.lua
@@ -10,7 +10,7 @@ function TestStrEncodings:test_str_encodings()
     local r = StrEncodings:from_file("src/str_encodings.bin")
 
     luaunit.assertEquals(r.str1, "Some ASCII")
-    luaunit.assertEquals(r.str2, "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}")
-    luaunit.assertEquals(r.str3, "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}")
-    luaunit.assertEquals(r.str4, "\u{2591}\u{2592}\u{2593}")
+    luaunit.assertEquals(r.str2, "\227\129\147\227\130\147\227\129\171\227\129\161\227\129\175")
+    luaunit.assertEquals(r.str3, "\227\129\147\227\130\147\227\129\171\227\129\161\227\129\175")
+    luaunit.assertEquals(r.str4, "\226\150\145\226\150\146\226\150\147")
 end
diff --git a/spec/lua/test_str_encodings_default.lua b/spec/lua/test_str_encodings_default.lua
index 04e6851b..18172060 100644
--- a/spec/lua/test_str_encodings_default.lua
+++ b/spec/lua/test_str_encodings_default.lua
@@ -10,7 +10,7 @@ function TestStrEncodingsDefault:test_str_encodings_default()
     local r = StrEncodingsDefault:from_file("src/str_encodings.bin")
 
     luaunit.assertEquals(r.str1, "Some ASCII")
-    luaunit.assertEquals(r.rest.str2, "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}")
-    luaunit.assertEquals(r.rest.str3, "\u{3053}\u{3093}\u{306b}\u{3061}\u{306f}")
-    luaunit.assertEquals(r.rest.str4, "\u{2591}\u{2592}\u{2593}")
+    luaunit.assertEquals(r.rest.str2, "\227\129\147\227\130\147\227\129\171\227\129\161\227\129\175")
+    luaunit.assertEquals(r.rest.str3, "\227\129\147\227\130\147\227\129\171\227\129\161\227\129\175")
+    luaunit.assertEquals(r.rest.str4, "\226\150\145\226\150\146\226\150\147")
 end
diff --git a/spec/lua/test_switch_manual_int_size_eos.lua b/spec/lua/test_switch_manual_int_size_eos.lua
index 36001a93..7163a227 100644
--- a/spec/lua/test_switch_manual_int_size_eos.lua
+++ b/spec/lua/test_switch_manual_int_size_eos.lua
@@ -18,7 +18,7 @@ function TestSwitchManualIntSizeEos:test_switch_manual_int_size_eos()
     luaunit.assertEquals(r.chunks[2].body.body.entries, {"AAAA", "BBBB", "CCCC"})
 
     luaunit.assertEquals(r.chunks[3].code, 0x33)
-    luaunit.assertEquals(r.chunks[3].body.body, "\x10\x20\x30\x40\x50\x60\x70\x80")
+    luaunit.assertEquals(r.chunks[3].body.body, "\16\32\48\64\80\96\112\128")
 
     luaunit.assertEquals(r.chunks[4].code, 0xff)
     luaunit.assertEquals(r.chunks[4].body.body, "")

@smarek
Copy link
Contributor Author

smarek commented Aug 19, 2020

Well I'm happy you appreciated my work, i've updated the commit to reflect on your points.

Also it would be great, if the PRs on this repository built with Travis, so we know the PR is working well (eg. the mentioned conflict about used Java APIs when building ScalaJS build for JavaScript).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants