[Tarantool-patches] [PATCH v2 2/2] box: fix formatting in session.push

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Apr 3 02:14:16 MSK 2020


Hi! Thanks for the fixes!

>>> +		lua_pushvalue(L, -3);
>>> +		lua_pcall(L, 1, 1, 0);
>>
>> 2. In case lua_pcall() fails, the function should fail too, and should
>> return NULL + set an error in diag. Also it would be good to have a
>> test on it.
> 
> I couldn’t really find a way to make serpent raise an error.

Yeah, me neither. But anyway an error should be handled. With
panic or with return -1, but we won't be able to test it.

>>
>>> 	}
>>> -	assert(rc == 1);
>>> 	assert(lua_isstring(L, -1));
>>> 	size_t len;
>>> 	const char *result = lua_tolstring(L, -1, &len);
>>> diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
>>> index 7208d3d30..fb7d9ca9b 100644
>>> --- a/src/box/lua/console.lua
>>> +++ b/src/box/lua/console.lua
>>> @@ -207,6 +208,18 @@ local function current_output()
>>>     end
>>> end
>>>
>>> +-- Used by console_session_push.
>>> +box_internal.format_lua_value = function(value)
>>> +    local opts = current_output()["opts"]
>>> +    value = format_lua_value(value, opts)
>>> +    return '-- Push begin\n' .. value .. '\n-- Push end\n'
>>
>> 3. Did you check that it is really necessary to append
>> a special 'push end' tail? I couldn't find any Lua value,
>> which couldn't be identified by 'push begin' and eos (';')
>> markers.
> 
> I agree, removed -- Push end 

Since there is no 'Push end' anymore, it should not be
called 'Push begin'. I renamed it to just 'Push'.

Also you didn't add a test on the push arriving out of
bound. I did it on the branch.

See my review fixes on top of this commit on the branch,
and below. If you agree, squash them. If don't, then lets
discuss.




More information about the Tarantool-patches mailing list