Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [RFC] box/lua/console: Add console.fmt module
@ 2019-06-20 21:54 Cyrill Gorcunov
  2019-06-21  7:32 ` [tarantool-patches] " Konstantin Osipov
  0 siblings, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-20 21:54 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko, Cyrill Gorcunov

The rationale of the module is to provide a way to configure
console output format, in particular one might need to copy
results of eval and paste it into some other command. To
switch console into that named lua mode one need to type

> require('console.fmt').set_format('lua')

To set it back to yaml mode just type

> require('console.fmt').set_format('yaml')

At the moment these two modes are supprted only.

Some details on internal structure: general console code
sits in src/box/lua/[console.c|console.lua] files; the C file
is resposible for readline library api and yaml serializer.
In turn the "formatter" now moved into src/box/lua/console_fmt.lua
and coupled with general console code. General console code simply
calls for formatter and provides output depending on active engine
(by default we continue use yaml backend in a sake of backward
compatibility).

There is at least one explicit problem remains: console text protocol
(console.lua:wrap_text_socket()) it waits the other peer to reply in
yaml format and tries to decode it. According to repo history the text
protocol is deprecated by now so I think we might safely remove the
code completely.

Please review carefully, any comments/complains are highly appreciated.
Of course I will need to provide some more test but first would like
to hear early feedback on the code design.

https://github.com/tarantool/tarantool/issues/3834
---
 src/box/CMakeLists.txt      |   1 +
 src/box/lua/console.lua     |  41 ++---
 src/box/lua/console_fmt.lua | 295 ++++++++++++++++++++++++++++++++++++
 src/box/lua/init.c          |   2 +
 4 files changed, 309 insertions(+), 30 deletions(-)
 create mode 100644 src/box/lua/console_fmt.lua

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 481842a39..2f7e90e5e 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -11,6 +11,7 @@ lua_source(lua_sources lua/feedback_daemon.lua)
 lua_source(lua_sources lua/net_box.lua)
 lua_source(lua_sources lua/upgrade.lua)
 lua_source(lua_sources lua/console.lua)
+lua_source(lua_sources lua/console_fmt.lua)
 lua_source(lua_sources lua/xlog.lua)
 lua_source(lua_sources lua/key_def.lua)
 lua_source(lua_sources lua/merger.lua)
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index f922f0320..9298ad44b 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -1,6 +1,7 @@
 -- console.lua -- internal file
 
 local internal = require('console')
+local cfmt = require('console.fmt')
 local session_internal = require('box.internal.session')
 local fiber = require('fiber')
 local socket = require('socket')
@@ -13,26 +14,6 @@ local net_box = require('net.box')
 local YAML_TERM = '\n...\n'
 local PUSH_TAG_HANDLE = '!push!'
 
-local function format(status, ...)
-    local err
-    if status then
-        -- serializer can raise an exception
-        status, err = pcall(internal.format, ...)
-        if status then
-            return err
-        else
-            err = 'console: an exception occurred when formatting the output: '..
-                tostring(err)
-        end
-    else
-        err = ...
-        if err == nil then
-            err = box.NULL
-        end
-    end
-    return internal.format({ error = err })
-end
-
 --
 -- Set delimiter
 --
@@ -81,16 +62,16 @@ local function set_param(storage, func, param, value)
         d = set_delimiter
     }
     if param == nil then
-        return format(false, 'Invalid set syntax, type \\help for help')
+        return cfmt.format(false, 'Invalid set syntax, type \\help for help')
     end
     if params[param] == nil then
-        return format(false, 'Unknown parameter: ' .. tostring(param))
+        return cfmt.format(false, 'Unknown parameter: ' .. tostring(param))
     end
-    return format(pcall(params[param], storage, value))
+    return cfmt.format(pcall(params[param], storage, value))
 end
 
 local function help_wrapper(storage)
-    return format(true, help()) -- defined in help.lua
+    return cfmt.format(true, help()) -- defined in help.lua
 end
 
 local function quit(storage)
@@ -119,7 +100,7 @@ local function preprocess(storage, line)
     end
     if operators[items[1]] == nil then
         local msg = "Invalid command \\%s. Type \\help for help."
-        return format(false, msg:format(items[1]))
+        return cfmt.format(false, msg:format(items[1]))
     end
     return operators[items[1]](storage, unpack(items))
 end
@@ -135,7 +116,7 @@ local function local_eval(storage, line)
         return preprocess(storage, line:sub(2))
     end
     if storage.language == 'sql' then
-        return format(pcall(box.execute, line))
+        return cfmt.format(pcall(box.execute, line))
     end
     --
     -- Attempt to append 'return ' before the chunk: if the chunk is
@@ -148,9 +129,9 @@ local function local_eval(storage, line)
         fun, errmsg = loadstring(line)
     end
     if not fun then
-        return format(false, errmsg)
+        return cfmt.format(false, errmsg)
     end
-    return format(pcall(fun))
+    return cfmt.format(pcall(fun))
 end
 
 local function eval(line)
@@ -258,7 +239,7 @@ local function remote_eval(self, line)
     if line and self.remote.state == 'active' then
         local ok, res = pcall(self.remote.eval, self.remote, line)
         if self.remote.state == 'active' then
-            return ok and res or format(false, res)
+            return ok and res or cfmt.format(false, res)
         end
     end
     local err = self.remote.error
@@ -268,7 +249,7 @@ local function remote_eval(self, line)
     self.prompt = nil
     self.completion = nil
     pcall(self.on_client_disconnect, self)
-    return (err and format(false, err)) or ''
+    return (err and cfmt.format(false, err)) or ''
 end
 
 local function local_check_lua(buf)
diff --git a/src/box/lua/console_fmt.lua b/src/box/lua/console_fmt.lua
new file mode 100644
index 000000000..f860d76fb
--- /dev/null
+++ b/src/box/lua/console_fmt.lua
@@ -0,0 +1,295 @@
+-- # vim: ts=4 sw=4 et
+
+local internal = require('console')
+
+local console_formats = {
+    ["lua"] = nil,
+    ["yaml"] = nil
+}
+
+---
+--- Serializer in Lua syntax, which represent data
+--- in a format suitable for Lua REPL.
+---
+
+local lua_keyword = {
+    ["and"] = true,     ["break"] = true,       ["do"] = true,
+    ["else"] = true,    ["elseif"] = true,      ["end"] = true,
+    ["false"] = true,   ["for"] = true,         ["function"] = true,
+    ["if"] = true,      ["in"] = true,          ["local"] = true,
+    ["nil"] = true,     ["not"] = true,         ["or"] = true,
+    ["repeat"] = true,  ["return"] = true,      ["then"] = true,
+    ["true"] = true,    ["until"] = true,       ["while"] = true
+}
+
+local function has_lquote(s)
+    local lstring_pat = '([%[%]])(=*)%1'
+    local equals, new_equals, _
+    local finish = 1
+    repeat
+        _, finish, _, new_equals = s:find(lstring_pat, finish)
+        if new_equals then
+            equals = math.max(equals or 0, #new_equals)
+        end
+    until not new_equals
+
+    return equals
+end
+
+local function is_identifier(s)
+    return type(s) == 'string' and s:find('^[%a_][%w_]*$') and not lua_keyword[s]
+end
+
+local function quote_string(s)
+    if type(s) ~= 'string' then
+        error("quote_string: s should be a string")
+    end
+
+    -- Find out if there are any embedded long-quote sequences that may cause issues.
+    -- This is important when strings are embedded within strings, like when serializing.
+    -- Append a closing bracket to catch unfinished long-quote sequences at the end of the string.
+    local equal_signs = has_lquote(s .. "]")
+
+    -- Note that strings containing "\r" can't be quoted using long brackets
+    -- as Lua lexer converts all newlines to "\n" within long strings.
+    if (s:find("\n") or equal_signs) and not s:find("\r") then
+        -- If there is an embedded sequence that matches a long quote, then
+        -- find the one with the maximum number of = signs and add one to that number.
+        equal_signs = ("="):rep((equal_signs or -1) + 1)
+        -- Long strings strip out leading newline. We want to retain that, when quoting.
+        if s:find("^\n") then s = "\n" .. s end
+        local lbracket, rbracket =
+            "[" .. equal_signs .. "[",
+            "]" .. equal_signs .. "]"
+        s = lbracket .. s .. rbracket
+    else
+        -- Escape funny stuff. Lua 5.1 does not handle "\r" correctly.
+        s = ("%q"):format(s):gsub("\r", "\\r")
+    end
+    return s
+end
+
+local function quote_if_necessary(v)
+    if not v then
+        return ''
+    else
+        if v:find ' ' then
+            v = quote_string(v)
+        end
+    end
+    return v
+end
+
+local function quote(s)
+    if type(s) == 'table' then
+	    return console_formats["lua"](s, '')
+    else
+        return quote_string(s)
+    end
+end
+
+local function index(numkey, key)
+    if not numkey then
+        key = quote(key)
+        key = key:find("^%[") and (" " .. key .. " ") or key
+    end
+    return '[' .. key .. ']'
+end
+
+--- Create a string representation of a Lua table.
+--
+-- This function never fails, but may complain by returning an
+-- extra value. Normally puts out one item per line, using
+-- the provided indent; set the second parameter to an empty string
+-- if you want output on one line.
+--
+-- @tab tbl Table to serialize to a string.
+-- @string[opt] space The indent to use.
+-- Defaults to two spaces; pass an empty string for no indentation.
+-- @bool[opt] not_clever Pass `true` for plain output, e.g `{['key']=1}`.
+-- Defaults to `false`.
+--
+-- @return a string
+console_formats["lua"] = function(tbl, space, not_clever)
+    if type(tbl) ~= 'table' then
+        if type(tbl) == 'string' then
+            return quote(tbl)
+        end
+        return tostring(tbl)
+    end
+
+    local set = ' = '
+    if space == '' then set = '=' end
+    space = space or '  '
+    local lines = {}
+    local line = ''
+    local tables = {}
+
+    local function put(s)
+        if #s > 0 then
+            line = line .. s
+        end
+    end
+
+    local function putln(s)
+        if #line > 0 then
+            line = line .. s
+            table.insert(lines, line)
+            line = ''
+        else
+            table.insert(lines, s)
+        end
+    end
+
+    local function eat_last_comma()
+        local n = #lines
+        local lastch = lines[n]:sub(-1,-1)
+        if lastch == ',' then
+            lines[n] = lines[n]:sub(1,-2)
+        end
+    end
+
+    local function preprocess(t)
+        local specials = {
+            [box.NULL] = 'box.NULL'
+        }
+        if specials[t] then
+            return true, specials[t]
+        end
+        return false, nil
+    end
+
+    local function writeit(t, oldindent, indent)
+        local pp, pps = preprocess(t)
+        local tp = type(t)
+        if pp then
+            putln(pps .. ',')
+        elseif tp ~= 'string' and  tp ~= 'table' then
+            putln(quote_if_necessary(tostring(t)) .. ',')
+        elseif tp == 'string' then
+            putln(quote_string(t) .. ',')
+        elseif tp == 'table' then
+            if tables[t] then
+                putln('<cycle>,')
+                return
+            end
+
+            tables[t] = true
+            local newindent = indent .. space
+            putln('{')
+            local used = {}
+            if not not_clever then
+                for i,val in ipairs(t) do
+                    put(indent)
+                    writeit(val, indent, newindent)
+                    used[i] = true
+                end
+            end
+            for key,val in pairs(t) do
+                local tkey = type(key)
+                local numkey = tkey == 'number'
+                if not_clever then
+                    key = tostring(key)
+                    put(indent .. index(numkey, key) .. set)
+                    writeit(val, indent, newindent)
+                else
+                    if not numkey or not used[key] then -- non-array indices
+                        if tkey ~= 'string' then
+                            key = tostring(key)
+                        end
+                        if numkey or not is_identifier(key) then
+                            key = index(numkey, key)
+                        end
+                        put(indent .. key .. set)
+                        writeit(val, indent, newindent)
+                    end
+                end
+            end
+            tables[t] = nil
+            eat_last_comma()
+            putln(oldindent .. '},')
+        else
+            putln(tostring(t) .. ',')
+        end
+    end
+    writeit(tbl, '', space)
+    eat_last_comma()
+    return table.concat(lines, #space > 0 and '\n' or '')
+end
+
+---
+--- Serializer in YAML format, basically we call for
+--- external serializer which lives in C code.
+---
+
+console_formats["yaml"] = function(status, ...)
+    local err
+    if status then
+        -- serializer can raise an exception
+        status, err = pcall(internal.format, ...)
+        if status then
+            return err
+        else
+            err = 'console.fmt: an exception in output ' .. tostring(err)
+        end
+    else
+        err = ...
+        if err == nil then
+            err = box.NULL
+        end
+        return internal.format({ error = err })
+    end
+end
+
+--
+-- Default console format
+--
+console_format = console_formats["yaml"]
+
+---
+--- Package API
+---
+
+--
+-- Setup requested format
+local function set_format(name)
+    if console_formats[name] ~= nil then
+        console_format = console_formats[name]
+    else
+        error(string.format("console.fmt: Unsupported console format %s", name))
+    end
+end
+
+--
+-- Report currently used format
+local function get_format()
+    for k,v in pairs(console_formats) do
+        if console_format == v then
+            return k
+        end
+    end
+end
+
+--
+-- Main message formatter, choose a proper
+-- format engine and pass arguments into.
+local function format(status, ...)
+    if console_format == console_formats["lua"] then
+        local data = ...
+        if data == nil then
+            -- In case if there is no data just
+            -- return empty string to not spam
+            -- with "nil" message.
+            return ""
+        end
+        return console_format(..., '')
+    end
+    return console_format(status, ...)
+end
+
+package.loaded['console.fmt'] = {
+    formats_map = console_formats,
+    set_format = set_format,
+    get_format = get_format,
+    format = format
+}
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 76b987b4b..aff84c87a 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -71,6 +71,7 @@ extern char session_lua[],
 	feedback_daemon_lua[],
 	net_box_lua[],
 	upgrade_lua[],
+	console_fmt_lua[],
 	console_lua[],
 	merger_lua[];
 
@@ -81,6 +82,7 @@ static const char *lua_sources[] = {
 	"box/feedback_daemon", feedback_daemon_lua,
 	"box/upgrade", upgrade_lua,
 	"box/net_box", net_box_lua,
+	"box/console_fmt", console_fmt_lua,
 	"box/console", console_lua,
 	"box/load_cfg", load_cfg_lua,
 	"box/xlog", xlog_lua,
-- 
2.20.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-20 21:54 [tarantool-patches] [RFC] box/lua/console: Add console.fmt module Cyrill Gorcunov
@ 2019-06-21  7:32 ` Konstantin Osipov
  2019-06-21  7:36   ` Konstantin Osipov
  2019-06-21  8:16   ` Cyrill Gorcunov
  0 siblings, 2 replies; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21  7:32 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko, Cyrill Gorcunov

* Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 00:55]:
> The rationale of the module is to provide a way to configure
> console output format, in particular one might need to copy
> results of eval and paste it into some other command. To
> switch console into that named lua mode one need to type
> 
> > require('console.fmt').set_format('lua')
> 
> To set it back to yaml mode just type
> 
> > require('console.fmt').set_format('yaml')
> 
> At the moment these two modes are supprted only.

We should have a meta command for this as well:

\set output format lua
\set output format yaml

Are you sure we need a separate namespace for fmt, wouldn't
console.set_format() be enough?

> index 000000000..f860d76fb
> --- /dev/null
> +++ b/src/box/lua/console_fmt.lua
> @@ -0,0 +1,295 @@
> +-- # vim: ts=4 sw=4 et
> +
> +local internal = require('console')
> +
> +local console_formats = {
> +    ["lua"] = nil,
> +    ["yaml"] = nil
> +}

Why didn't  you use a git submodule with an established Lua
formatter?

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21  7:32 ` [tarantool-patches] " Konstantin Osipov
@ 2019-06-21  7:36   ` Konstantin Osipov
  2019-06-21  8:09     ` Cyrill Gorcunov
  2019-06-21  8:16   ` Cyrill Gorcunov
  1 sibling, 1 reply; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21  7:36 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Alexander Turenko, Cyrill Gorcunov

* Konstantin Osipov <kostja@tarantool.org> [19/06/21 10:32]:

Last thing: Lua output should become the default.
It should perhaps also be possible to set the output format of every
console individually.


-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21  7:36   ` Konstantin Osipov
@ 2019-06-21  8:09     ` Cyrill Gorcunov
  2019-06-21 12:11       ` Konstantin Osipov
  0 siblings, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21  8:09 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 10:36:43AM +0300, Konstantin Osipov wrote:
> * Konstantin Osipov <kostja@tarantool.org> [19/06/21 10:32]:
> 
> Last thing: Lua output should become the default.

But it will break backward compatibility, is it ok?

> It should perhaps also be possible to set the output
> format of every console individually.

OK, i think it should be a next step. At the moment I'm
gathering feedback to make code flexible enough for
extensions.

Previously you pointed that we might need to support
json/sql formats and such. Could you please elaborate
with some examples so I would treat them as a vectore
where to move in general design.

Thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21  7:32 ` [tarantool-patches] " Konstantin Osipov
  2019-06-21  7:36   ` Konstantin Osipov
@ 2019-06-21  8:16   ` Cyrill Gorcunov
  2019-06-21 12:14     ` Konstantin Osipov
  1 sibling, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21  8:16 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 10:32:41AM +0300, Konstantin Osipov wrote:
> > At the moment these two modes are supprted only.
> 
> We should have a meta command for this as well:
> 
> \set output format lua
> \set output format yaml

OK, will do.

> Are you sure we need a separate namespace for fmt, wouldn't
> console.set_format() be enough?

Because I want to separate output engine from general
console code. To me console is like a transport peer,
which includes setting up a connection to remote machine,
run evaluator and etc, but output formatter is like a
separate engine so it should live in other module. Or
you meant something else?

> > index 000000000..f860d76fb
> > --- /dev/null
> > +++ b/src/box/lua/console_fmt.lua
> > @@ -0,0 +1,295 @@
> > +-- # vim: ts=4 sw=4 et
> > +
> > +local internal = require('console')
> > +
> > +local console_formats = {
> > +    ["lua"] = nil,
> > +    ["yaml"] = nil
> > +}
> 
> Why didn't  you use a git submodule with an established Lua
> formatter?

I considered using vanilla "pretty print" or "serpent" but

1) they are too huge
2) we need own extensions for decoding own subset of keywords
   (for example box.NULL symbol)

so we simply can't use unmodified third party engines thus
better to keep own much simpler (hopefully) engine.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21  8:09     ` Cyrill Gorcunov
@ 2019-06-21 12:11       ` Konstantin Osipov
  2019-06-21 12:33         ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21 12:11 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 11:10]:
> On Fri, Jun 21, 2019 at 10:36:43AM +0300, Konstantin Osipov wrote:
> > * Konstantin Osipov <kostja@tarantool.org> [19/06/21 10:32]:
> > 
> > Last thing: Lua output should become the default.
> 
> But it will break backward compatibility, is it ok?

Yes, it's OK for 2.x series.

> > It should perhaps also be possible to set the output
> > format of every console individually.
> 
> OK, i think it should be a next step. At the moment I'm
> gathering feedback to make code flexible enough for
> extensions.
> 
> Previously you pointed that we might need to support
> json/sql formats and such. Could you please elaborate
> with some examples so I would treat them as a vectore
> where to move in general design.

For JSON, please take a look at how MongoDB presents its output.

For SQL, PostgreSQL or MySQL or CockroachDB would be nice -they
even colorize table headings. In any case, SQL demands tabular
form. Tabular output is difficult. Especially with non-ascii
characters and alphabets, which have digraphs,
and ligatures.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21  8:16   ` Cyrill Gorcunov
@ 2019-06-21 12:14     ` Konstantin Osipov
  2019-06-21 12:41       ` Cyrill Gorcunov
  2019-06-21 20:07       ` Cyrill Gorcunov
  0 siblings, 2 replies; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21 12:14 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 11:20]:
> Because I want to separate output engine from general
> console code. To me console is like a transport peer,
> which includes setting up a connection to remote machine,
> run evaluator and etc, but output formatter is like a
> separate engine so it should live in other module. Or
> you meant something else?

I don't care much about the internals here, but about the
consistency of the console api:
https://www.tarantool.io/en/doc/1.10/reference/reference_lua/console/
> 
> > > index 000000000..f860d76fb
> > > --- /dev/null
> > > +++ b/src/box/lua/console_fmt.lua
> > > @@ -0,0 +1,295 @@
> > > +-- # vim: ts=4 sw=4 et
> > > +
> > > +local internal = require('console')
> > > +
> > > +local console_formats = {
> > > +    ["lua"] = nil,
> > > +    ["yaml"] = nil
> > > +}
> > 
> > Why didn't  you use a git submodule with an established Lua
> > formatter?
> 
> I considered using vanilla "pretty print" or "serpent" but
> 
> 1) they are too huge
> 2) we need own extensions for decoding own subset of keywords
>    (for example box.NULL symbol)
> 
> so we simply can't use unmodified third party engines thus
> better to keep own much simpler (hopefully) engine.

I don't see why we can't fork serpent. But OK, provided you don't
want serpent, what can serpent do that we're going to miss in an
own implementation? Could you write down a summary so that we can
analyze this ahead of time?

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 12:11       ` Konstantin Osipov
@ 2019-06-21 12:33         ` Cyrill Gorcunov
  0 siblings, 0 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21 12:33 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 03:11:44PM +0300, Konstantin Osipov wrote:
> > 
> > But it will break backward compatibility, is it ok?
> 
> Yes, it's OK for 2.x series.

OK

> > 
> > Previously you pointed that we might need to support
> > json/sql formats and such. Could you please elaborate
> > with some examples so I would treat them as a vectore
> > where to move in general design.
> 
> For JSON, please take a look at how MongoDB presents its output.
> 
> For SQL, PostgreSQL or MySQL or CockroachDB would be nice -they
> even colorize table headings. In any case, SQL demands tabular
> form. Tabular output is difficult. Especially with non-ascii
> characters and alphabets, which have digraphs, and ligatures.

I see, thanks! Knowing that I should review the engine to make it
more flexible. Also thought about line-oriented mode, say one defines
(in lua mode)

# > a = {1,2,3}
# > a
# {1,2,3}

but then recode on the fly:

# console.fmt(json,a)
# {"a":[1,2,3]}

or something like that.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 12:14     ` Konstantin Osipov
@ 2019-06-21 12:41       ` Cyrill Gorcunov
  2019-06-21 12:47         ` Konstantin Osipov
  2019-06-21 20:07       ` Cyrill Gorcunov
  1 sibling, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21 12:41 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 03:14:12PM +0300, Konstantin Osipov wrote:
> 
> I don't care much about the internals here, but about the
> consistency of the console api:
> https://www.tarantool.io/en/doc/1.10/reference/reference_lua/console/

Well, I don't mind extending console api instead just need to figure
out how to put the engine into separate file (serisouly, console.lua
is already big enough).

> 
> I don't see why we can't fork serpent. But OK, provided you don't
> want serpent, what can serpent do that we're going to miss in an
> own implementation? Could you write down a summary so that we can
> analyze this ahead of time?

I will write the summary, but most important part is that they are
fetching dependency and code bloats like hell even if we don't need
most of the parts of serpent/pretty-print (initially I started with
serpent, integrated it into our code but didn't like the results much,
then I merged pretty-print engine, and again found that we need own
extensions for symbols like box.NULL which lead me to the conclusion
that if we can't use vanilla code then there is no much point to carry
these projects in our repo, better to stick small and simple functions
and extend them with time if we have to).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 12:41       ` Cyrill Gorcunov
@ 2019-06-21 12:47         ` Konstantin Osipov
  2019-06-21 13:17           ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21 12:47 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 15:45]:
> I will write the summary, but most important part is that they are
> fetching dependency and code bloats like hell even if we don't need
> most of the parts of serpent/pretty-print (initially I started with
> serpent, integrated it into our code but didn't like the results much,
> then I merged pretty-print engine, and again found that we need own
> extensions for symbols like box.NULL which lead me to the conclusion
> that if we can't use vanilla code then there is no much point to carry
> these projects in our repo, better to stick small and simple functions
> and extend them with time if we have to).

the only problem with this is that you may step on the same rakes
and break backward compatibility by following the same path as
them.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 12:47         ` Konstantin Osipov
@ 2019-06-21 13:17           ` Cyrill Gorcunov
  0 siblings, 0 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21 13:17 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 03:47:37PM +0300, Konstantin Osipov wrote:
> 
> the only problem with this is that you may step on the same rakes
> and break backward compatibility by following the same path as
> them.

True. But code size is incomarable. And while (and if) our _small_
code chunk cover all our needs, it is a way better to support it
than those big subprojects.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 12:14     ` Konstantin Osipov
  2019-06-21 12:41       ` Cyrill Gorcunov
@ 2019-06-21 20:07       ` Cyrill Gorcunov
  2019-06-21 20:33         ` Konstantin Osipov
  1 sibling, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21 20:07 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 03:14:12PM +0300, Konstantin Osipov wrote:
> > so we simply can't use unmodified third party engines thus
> > better to keep own much simpler (hopefully) engine.
> 
> I don't see why we can't fork serpent. But OK, provided you don't
> want serpent, what can serpent do that we're going to miss in an
> own implementation? Could you write down a summary so that we can
> analyze this ahead of time?

To be precise my complains against serpent is only one: we will
need to patch it on top to handle special symbols from "box"
(as mentioned in the original github issue box.NULL). Note that
the seializer I provided are mostly taken from Konstantin's
draft patch (see https://github.com/tarantool/tarantool/issues/3834)
which in turn is derived from
https://github.com/stevedonovan/Penlight/blob/master/lua/pl/pretty.lua
i guess.

I think currently we don't support "block" mode (or as serpent names
it -- multiline output). For simple examples of output serpent provides
same data as our console.fmt code but I think the devil in details which
I didn't discover yet.

Thus if you're _quite_ sure that we better should use serpent then I'll
try to prepare new version and we will see how it looks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 20:07       ` Cyrill Gorcunov
@ 2019-06-21 20:33         ` Konstantin Osipov
  2019-06-21 22:34           ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Konstantin Osipov @ 2019-06-21 20:33 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches, Alexander Turenko

* Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 23:11]:
> Thus if you're _quite_ sure that we better should use serpent then I'll
> try to prepare new version and we will see how it looks.

No I am not but given no one made a detailed comparison I would
err on the side of an established module.

Pretty printing multi-line output is a must btw.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [RFC] box/lua/console: Add console.fmt module
  2019-06-21 20:33         ` Konstantin Osipov
@ 2019-06-21 22:34           ` Cyrill Gorcunov
  0 siblings, 0 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-06-21 22:34 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Alexander Turenko

On Fri, Jun 21, 2019 at 11:33:48PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/06/21 23:11]:
> > Thus if you're _quite_ sure that we better should use serpent then I'll
> > try to prepare new version and we will see how it looks.
> 
> No I am not but given no one made a detailed comparison I would
> err on the side of an established module.

To make a detailed compare I need more time, since you know I
spent a lot of efforts simply to figure out how to merge new
code into our builtin modules.

> 
> Pretty printing multi-line output is a must btw.

Ah, didnt know that. Will keep in mind, thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-06-21 22:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-20 21:54 [tarantool-patches] [RFC] box/lua/console: Add console.fmt module Cyrill Gorcunov
2019-06-21  7:32 ` [tarantool-patches] " Konstantin Osipov
2019-06-21  7:36   ` Konstantin Osipov
2019-06-21  8:09     ` Cyrill Gorcunov
2019-06-21 12:11       ` Konstantin Osipov
2019-06-21 12:33         ` Cyrill Gorcunov
2019-06-21  8:16   ` Cyrill Gorcunov
2019-06-21 12:14     ` Konstantin Osipov
2019-06-21 12:41       ` Cyrill Gorcunov
2019-06-21 12:47         ` Konstantin Osipov
2019-06-21 13:17           ` Cyrill Gorcunov
2019-06-21 20:07       ` Cyrill Gorcunov
2019-06-21 20:33         ` Konstantin Osipov
2019-06-21 22:34           ` Cyrill Gorcunov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox