On 07/26/2018 12:56 AM, Vladislav Shpilevoy wrote: > > > On 25/07/2018 10:51, imeevma@tarantool.org wrote: >> This patch allow to use option "--language" with >> "tarantoolctl enter" command. Also default value >> for language were added in default configuration >> file. Language is either SQL or Lua. >> >> Closes #2385. >> --- >> Branch: >> https://github.com/tarantool/tarantool/tree/imeevma/gh-2385-select-input-language-tarantoolctl >> >> Issue: https://github.com/tarantool/tarantool/issues/2385 >> >> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in >> index 2dd5d74..55711b8 100755 >> --- a/extra/dist/tarantoolctl.in >> +++ b/extra/dist/tarantoolctl.in >> @@ -155,6 +155,11 @@ local function load_default_file(default_file) >>       d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name) >>       d.log       = fio.pathjoin(d.log,       instance_name .. '.log') >>   +    d.language  = (d.language or "lua"):lower() >> +    if d.language ~= "lua" and d.language ~= "sql" then >> +        d.language = "lua" > > 1. Please, do not ignore unknown language. Log this error like others > in the same function already do. Done. > > 2. Why do you need language in default_cfg? It is box.cfg as I > understand and box.cfg has no language option. It is not? If you > need to parse a language from the file, then you can store it in > another table or even in a separate variable like others on lines > 30-49. Done. > >> +    end >> + >>       default_cfg = d >>         if not usermode the> @@ -629,6 +635,17 @@ local function >> logrotate() >>   end >>     local function enter() >> +    local options = keyword_arguments >> +    local language = options.language >> +    if language ~= nil then >> +        language = language:lower() >> +        if language ~= "lua" and language ~= "sql" then >> +            log.warn("Language \"%s\" is not recognized. Default >> language" .. >> +                     " \"%s\" is set.", language, default_cfg.language) >> +            language = nil > > 3. For enter() function it is ok to fail on unknown language. > You should not ignore the error because a caller could use > something like this: > >     cat my_script.sql | tarantoolctl enter --language sqlll > > And if you ignore invalid language, this command will feed the > entire script on SQL into Lua console. It will produce huge > number of syntax errors if no worse. Done. > >> +        end >> +    end >> +    language = language or default_cfg.language >>       local console_sock_path = uri.parse(console_sock).service >>       if fio.stat(console_sock_path) == nil then >>           log.error("Can't connect to %s (%s)", console_sock_path, >> errno.strerror()) >> @@ -644,7 +661,9 @@ local function enter() >>           console_sock, TIMEOUT_INFINITY >>       ) >>   -    console.on_start(function(self) self:eval(cmd) end) >> +    local cmd_language = string.format("\\set language %s", language) > > 4. Why not to merge cmd_language into cmd? It is created a line above. Discussed, partly changed. > >> + >> +    console.on_start(function(self) self:eval(cmd) >> self:eval(cmd_language) end) >>       console.on_client_disconnect(function(self) self.running = >> false end) >>       console.start() >>       return 0 >> @@ -1284,6 +1303,7 @@ do >>           { 'chdir',       'string'  }, >>           { 'only-server', 'string'  }, >>           { 'server',      'string'  }, >> +        { 'language',    'string'  }, >>       }) >>         local cmd_name >> > > 5. Lets add the same option to 'eval', 'connect' to make the commands > symmetric. Discussed, decided to reject the idea. > > 6. Please, update the comments. For example, on line 1028 I > still see the comment about 'enter': "Enter an instance's interactive > Lua console", > but actually it is not complete now - you can choose SQL. > Done. commit 7088606bba3810ffc6db313cf31d7aa101d7175d Author: Mergen Imeev Date:   Fri Jul 13 19:25:32 2018 +0300     Tarantoolctl "enter" with set language     This patch allow to use option "--language" with     "tarantoolctl enter" command. Also default value     for language were added in default configuration     file. Language is either SQL or Lua.     Closes #2385. diff --git a/extra/dist/CMakeLists.txt b/extra/dist/CMakeLists.txt index 862689e..3045b5a 100644 --- a/extra/dist/CMakeLists.txt +++ b/extra/dist/CMakeLists.txt @@ -34,6 +34,7 @@ message (STATUS "tarantoolctl logdir: ${TARANTOOL_LOGDIR}")  set(TARANTOOL_RUNDIR "${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/run/tarantool")  message (STATUS "tarantoolctl rundir: ${TARANTOOL_RUNDIR}")  set(TARANTOOL_USER "tarantool") +set(TARANTOOL_LANGUAGE "lua")  set(SYSCONFIG_AVAILABLEDIR "tarantool/instances.available")  set(SYSCONFIG_ENABLEDDIR "tarantool/instances.enabled")  set(TARANTOOL_AVAILABLEDIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/${SYSCONFIG_AVAILABLEDIR}") diff --git a/extra/dist/default/tarantool.in b/extra/dist/default/tarantool.in index 9a8dad3..01e7144 100644 --- a/extra/dist/default/tarantool.in +++ b/extra/dist/default/tarantool.in @@ -20,6 +20,7 @@ default_cfg = {      vinyl_dir  = "@TARANTOOL_DATADIR@", -- @TARANTOOL_DATADIR@/${INSTANCE}      log        = "@TARANTOOL_LOGDIR@", -- @TARANTOOL_LOGDIR@/${INSTANCE}.log      username   = "@TARANTOOL_USER@", +    language   = "@TARANTOOL_LANGUAGE@",  }  -- instances.available - all available instances diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in index 2dd5d74..ef9da2f 100755 --- a/extra/dist/tarantoolctl.in +++ b/extra/dist/tarantoolctl.in @@ -47,6 +47,7 @@ local default_cfg  local positional_arguments  local keyword_arguments  local lua_arguments = arg +local language  -- function for printing usage reference  local usage @@ -155,6 +156,14 @@ local function load_default_file(default_file)      d.vinyl_dir = fio.pathjoin(d.vinyl_dir, instance_name)      d.log       = fio.pathjoin(d.log,       instance_name .. '.log') +    language = (d.language or "lua"):lower() +    d.language  = nil + +    if language ~= "lua" and language ~= "sql" then +        log.error('Unknown language: %s', language) +        os.exit(1) +    end +      default_cfg = d      if not usermode then @@ -629,6 +638,12 @@ local function logrotate()  end  local function enter() +    local options = keyword_arguments +    language = (options.language or language):lower() +    if language ~= "lua" and language ~= "sql" then +        log.error('Unknown language: %s', options.language) +        return 1 +    end      local console_sock_path = uri.parse(console_sock).service      if fio.stat(console_sock_path) == nil then          log.error("Can't connect to %s (%s)", console_sock_path, errno.strerror()) @@ -644,7 +659,9 @@ local function enter()          console_sock, TIMEOUT_INFINITY      ) -    console.on_start(function(self) self:eval(cmd) end) +    local cmd_connect = string.format("\\set language %s", language) + +    console.on_start(function(self) self:eval(cmd) self:eval(cmd_connect) end)      console.on_client_disconnect(function(self) self.running = false end)      console.start()      return 0 @@ -1008,11 +1025,15 @@ local commands = setmetatable({          }      }, enter = {          func = exit_wrapper(enter), process = process_local, help = { -            header = "%s enter INSTANCE", +            header = "%s enter INSTANCE [--language=language]",              linkmode = "%s enter",              description =  [=[ -        Enter an instance's interactive Lua console. +        Enter an instance's interactive Lua or SQL console. + +        Supported options: +        * --language=language to set interactive console language. +          May be either Lua or SQL.  ]=],              weight = 65,              deprecated = false, @@ -1284,6 +1305,7 @@ do          { 'chdir',       'string'  },          { 'only-server', 'string'  },          { 'server',      'string'  }, +        { 'language',    'string'  },      })      local cmd_name