Re: [Tarantool-patches] [PATCH] Introduce fselect - formatted select

[Alexander Turenko <alexander.turenko@tarantool.org>]

I have not much to say here. It's nice. It is for humans and so unlikely
we should provide some guarantees except options meaning.

My first thoughts were how to generalize and allow to reuse it.
box.execute() also provides columns metainfo, but the format is a bit
different. I guess it'll be the first request to us: support such output
in SQL. Maybe just for box.execute(), via Lua. However we can be
consider it as a separate task.

I also had thoughts whether we should tweak formatting things on some
other level: say, provide an option for :select(), which sets a
metatable with particular __serialize function. So data will be
available as usual in a script, but will be formatted this way in the
interactive console. I'm not sure, because there is the donwside: it
surely not so short as 'fselect'.

I see that you continue to improve the patch on the branch: now 'sql',
'github' and 'jira' delimiters are supported. For me it looks like it
goes to be a separate module for formatting tables (like 'csv' we
already have). Not just 'formatted select' thing. However here I'm not
sure too.

That's all. It is nice and it is okay to push from my side.

Several nits are below.

WBR, Alexander Turenko.

> +-- index.fselect - formatted select.
> +base_index_mt.fselect = function(index, key, opts)
> +    -- options.
> +    local max_width = 140

We can obtain terminal width and use it as default:

 | tarantool> ffi = require('ffi')
 | tarantool> ffi.cdef('void rl_get_screen_size(int *rows, int *cols);')
 | tarantool> colsp = ffi.new('int[1]')
 | tarantool> ffi.C.rl_get_screen_size(nil, colsp)
 | tarantool> colsp[0]
 | ---
 | - 190
 | ...

It returns zero, when I run it from a file (not from the console), so a
default when there is no terminal size information is necessary anyway.

Not sure about portability of calling rl_get_screen_size(), though.

> +    local num_rows = #tab
> +    local space = box.space[index.space_id]
> +    local fmt = space:format() 

Nit: trailing whitespace.

> +    -- format string - cut or fill with spaces to make is exactly n symbols.
> +    -- also replace spaces with non-break spaces.
> +    local fmt_str = function(x, n)
> +        if not x then x = '' end
> +        local str
> +        if x:len() <= n then
> +            local add = n - x:len()
> +            local addl = math.floor(add/2)
> +            local addr = math.ceil(add/2)
> +            str = string.rep(' ', addl) .. x .. string.rep(' ', addr)

There is quite similar string.center() function (src/lua/string.lua).