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

[Chris Sosnin <k.sosnin@tarantool.org>]

Thank you very much for the review and for your fixes!

I squashed your changes and pushed the updated version.

> On 3 Apr 2020, at 02:14, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> 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.
> 
>