From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D2F52469710 for ; Mon, 18 May 2020 15:21:03 +0300 (MSK) References: <20200512135052.221379-1-gorcunov@gmail.com> <20200512135052.221379-7-gorcunov@gmail.com> From: Oleg Babin Message-ID: Date: Mon, 18 May 2020 15:21:00 +0300 MIME-Version: 1.0 In-Reply-To: <20200512135052.221379-7-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 6/7] box/console: switch to new lua serializer List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Hi! Thanks for your patch. LGTM. Please file an issue to remove serpent in the next version. On 12/05/2020 16:50, Cyrill Gorcunov wrote: > I do not drop the serpent module for a while > since I need to make sure that everything work > as expected as we've not break backward compatibility > significantly (though we didn't claim the lua mode > output is stable enough). > > Fixes #4682 > > Signed-off-by: Cyrill Gorcunov > --- > src/box/lua/console.c | 77 +++++++++++++++++++++++++++++++ > src/box/lua/console.lua | 76 +++++++++++++----------------- > test/app-tap/console_lua.test.lua | 18 ++++---- > 3 files changed, 119 insertions(+), 52 deletions(-) > > diff --git a/src/box/lua/console.c b/src/box/lua/console.c > index c8e0dea58..d1bf7949c 100644 > --- a/src/box/lua/console.c > +++ b/src/box/lua/console.c > @@ -39,6 +39,7 @@ > #include "coio.h" > #include "lua/msgpack.h" > #include "lua-yaml/lyaml.h" > +#include "serialize_lua.h" > #include > #include > #include > @@ -50,6 +51,7 @@ > extern char serpent_lua[]; > > static struct luaL_serializer *serializer_yaml; > +static struct luaL_serializer *serializer_lua; > > /* > * Completion engine (Mike Paul's). > @@ -68,6 +70,54 @@ lua_rl_complete(lua_State *L, const char *text, int start, int end); > */ > static struct lua_State *readline_L; > > +/** > + * Encode Lua object into Lua form. > + */ > +static int > +lbox_console_format_lua(struct lua_State *L) > +{ > + lua_dumper_opts_t opts; > + int arg_count; > + > + /* Parse options and remove them */ > + lua_parse_opts(L, &opts); > + lua_remove(L, 1); > + > + arg_count = lua_gettop(L); > + > + /* If nothing to process just exit early */ > + if (arg_count == 0) > + return 0; > + > + /* > + * When processing arguments we might > + * need to modify reference (for example > + * when __index references to object itself) > + * thus make a copy of incoming data. > + * > + * Note that in yaml there is no map for > + * Lua's "nil" value so for yaml encoder > + * we do > + * > + * if (lua_isnil(L, i + 1)) > + * luaL_pushnull(L); > + * else > + * lua_pushvalue(L, i + 1); > + * > + * > + * For lua mode we have to preserve "nil". > + */ > + lua_createtable(L, arg_count, 0); > + for (int i = 0; i < arg_count; ++i) { > + lua_pushvalue(L, i + 1); > + lua_rawseti(L, -2, i + 1); > + } > + > + lua_replace(L, 1); > + lua_settop(L, 1); > + return lua_encode(L, serializer_lua, &opts); > +} > + > /* > * console_completion_handler() > * Called by readline to collect plausible completions; > @@ -557,6 +607,7 @@ tarantool_lua_console_init(struct lua_State *L) > {"add_history", lbox_console_add_history}, > {"completion_handler", lbox_console_completion_handler}, > {"format_yaml", lbox_console_format_yaml}, > + {"format_lua", lbox_console_format_lua}, > {NULL, NULL} > }; > luaL_register_module(L, "console", consolelib); > @@ -581,6 +632,32 @@ tarantool_lua_console_init(struct lua_State *L) > * load_history work the same way. > */ > lua_setfield(L, -2, "formatter"); > + > + /* > + * We don't export it as a module > + * for a while, so the library > + * is kept empty. > + */ > + static const luaL_Reg lualib[] = { > + { }, > + }; > + > + serializer_lua = luaL_newserializer(L, NULL, lualib); > + serializer_lua->has_compact = 1; > + serializer_lua->encode_invalid_numbers = 1; > + serializer_lua->encode_load_metatables = 1; > + serializer_lua->encode_use_tostring = 1; > + serializer_lua->encode_invalid_as_nil = 1; > + > + /* > + * Keep a reference to this module so it > + * won't be unloaded. > + */ > + lua_setfield(L, -2, "formatter_lua"); > + > + /* Output formatter in Lua mode */ > + lua_serializer_init(L); > + > struct session_vtab console_session_vtab = { > .push = console_session_push, > .fd = console_session_fd, > diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua > index 5642ca956..6ea27a393 100644 > --- a/src/box/lua/console.lua > +++ b/src/box/lua/console.lua > @@ -69,60 +69,50 @@ output_handlers["yaml"] = function(status, opts, ...) > return internal.format_yaml({ error = err }) > end > > --- A map for internal symbols in case if they > --- are not inside tables and serpent won't be > --- able to handle them properly. > -local lua_map_direct_symbols = { > - [box.NULL] = 'box.NULL', > -} > - > --- A map for internal symbols in case if they > --- are coming from tables and we need to depict > --- them into user known values. > -local lua_map_table_symbols = { > - ['"cdata: NULL"'] = 'box.NULL' > -} > - > -- > --- Map internal symbols which serpent doesn't > --- know about to a known representation. > -local serpent_map_symbols = function(tag, head, body, tail, level) > - for k,v in pairs(lua_map_table_symbols) do > - body = body:gsub(k, v) > +-- Format a Lua value. > +local function format_lua_value(status, internal_opts, value) > + local err > + if status then > + status, err = pcall(internal.format_lua, internal_opts, value) > + if status then > + return err > + else > + local m = 'console: exception while formatting output: "%s"' > + err = m:format(tostring(err)) > + end > + else > + err = value > + if err == nil then > + err = box.NULL > + end > end > - return tag..head..body..tail > + return internal.format_lua(internal_opts, { error = err }) > end > > -- > --- Format a Lua value. > -local function format_lua_value(value, opts) > - for k,v in pairs(lua_map_direct_symbols) do > - if k == value then > - return v > - end > - end > - local serpent_opts = { > - custom = serpent_map_symbols, > - comment = false, > - nocode = true, > - } > +-- Convert options from user form to the internal format. > +local function gen_lua_opts(opts) > if opts == "block" then > - return serpent.block(value, serpent_opts) > + return {block=true, indent=2} > + else > + return {block=false, indent=2} > end > - return serpent.line(value, serpent_opts) > end > > output_handlers["lua"] = function(status, opts, ...) > local collect = {} > -- > - -- If no data present at least EOS should be put, > - -- otherwise wire readers won't be able to find > - -- where the end of string is. > - if not ... then > - return output_eos["lua"] > - end > + -- Convert options to internal dictionary. > + local internal_opts = gen_lua_opts(opts) > for i = 1, select('#', ...) do > - collect[i] = format_lua_value(select(i, ...), opts) > + collect[i] = format_lua_value(status, internal_opts, select(i, ...)) > + if collect[i] == nil then > + -- > + -- table.concat doesn't work for nil > + -- so convert it explicitly. > + collect[i] = "nil" > + end > end > return table.concat(collect, ', ') .. output_eos["lua"] > end > @@ -210,8 +200,8 @@ end > > -- Used by console_session_push. > box_internal.format_lua_push = function(value) > - local opts = current_output()["opts"] > - value = format_lua_value(value, opts) > + local internal_opts = gen_lua_opts(current_output()["opts"]) > + value = format_lua_value(true, internal_opts, value) > return '-- Push\n' .. value .. ';' > end > > diff --git a/test/app-tap/console_lua.test.lua b/test/app-tap/console_lua.test.lua > index 3ed6aad97..8bb6eb0b3 100755 > --- a/test/app-tap/console_lua.test.lua > +++ b/test/app-tap/console_lua.test.lua > @@ -102,29 +102,29 @@ local cases = { > prepare = 'a = {1, 2, 3}', > opts = {block = false}, > input = '1, 2, nil, a', > - expected = '1, 2, box.NULL, {1, 2, 3}', > + expected = '1, 2, nil, {1, 2, 3}', > cleanup = 'a = nil', > }, { > name = 'multireturn block mode', > prepare = 'a = {1, 2, 3}', > opts = {block = true}, > input = '1, 2, nil, a', > - expected = '1, 2, box.NULL, {\n 1,\n 2,\n 3\n}', > + expected = '1, 2, nil, {\n 1,\n 2,\n 3\n}', > cleanup = 'a = nil', > }, { > - name = 'trailing nils, line mode', > + name = 'trailing nils, box.NULL, line mode', > opts = {block = false}, > - input = '1, nil, nil, nil', > - expected = '1, box.NULL, box.NULL, box.NULL', > + input = '1, nil, box.NULL, nil', > + expected = '1, nil, box.NULL, nil', > }, { > - name = 'trailing nils, block mode', > + name = 'trailing nils, box.NULL, block mode', > opts = {block = true}, > - input = '1, nil, nil, nil', > - expected = '1, box.NULL, box.NULL, box.NULL', > + input = '1, nil, box.NULL, nil', > + expected = '1, nil, box.NULL, nil', > }, { > name = 'empty output', > input = '\\set output', > - expected = '"Specify output format: lua or yaml."', > + expected = '{error = \"Specify output format: lua or yaml.\"}', > } > } > >