From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3C2E342EF5C for ; Thu, 11 Jun 2020 19:57:17 +0300 (MSK) Date: Thu, 11 Jun 2020 19:48:08 +0300 From: Igor Munkin Message-ID: <20200611164808.GR5745@tarantool.org> References: <20200610094354.31831-1-arkholga@tarantool.org> <20200610094354.31831-3-arkholga@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200610094354.31831-3-arkholga@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 2/2] console: add line carrying backslash List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Olga Arkhangelskaia Cc: tarantool-patches@dev.tarantool.org, alexander.turenko@tarantool.org Olya, Thanks for the patch! On 10.06.20, Olga Arkhangelskaia wrote: > When using interactive console(stdin) instead of \set delimiter > with "\", "\" in the end if line can be used. I propose to reword commit message a bit to make it clearer. Consider the following: | Before this patch one has to explicitly set delimiter different from | newline ('\n') symbol when using interactive console. | | The changes make possible to split console input into multiple lines | via backslash ('\\') at the end of the line instead of setting a | delimiter. Feel free to change it on your own. > > Closes #4317 > --- > src/box/lua/console.lua | 18 ++++++++----- > test/app-tap/gh-4317.test.lua | 51 +++++++++++++++++++++++++++++++++++ > 2 files changed, 63 insertions(+), 6 deletions(-) > create mode 100755 test/app-tap/gh-4317.test.lua > > diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua > index 17e2c91b2..4c2c7a132 100644 > --- a/src/box/lua/console.lua > +++ b/src/box/lua/console.lua > @@ -573,14 +573,20 @@ local function local_read(self) > break > end > if delim == "" then > - local lang = box.session.language > - if not lang or lang == 'lua' then > - -- stop once a complete Lua statement is entered > - if local_check_lua(buf) then > + -- if no delim is set and line ends with the backslash > + -- continue reading > + if buf:sub(-1, -1) == '\\' then > + buf = buf:sub(0, #buf - 1) > + else > + local lang = box.session.language > + if not lang or lang == 'lua' then > + -- stop once a complete Lua statement is entered > + if local_check_lua(buf) then > + break > + end > + else > break > end > - else > - break > end > elseif #buf >= #delim and buf:sub(#buf - #delim + 1) == delim then > buf = buf:sub(0, #buf - #delim) > diff --git a/test/app-tap/gh-4317.test.lua b/test/app-tap/gh-4317.test.lua > new file mode 100755 > index 000000000..d7a5063e7 > --- /dev/null > +++ b/test/app-tap/gh-4317.test.lua Minor: it would be nice to describe the bug in test name using 2-3 words[1]: | gh-####-description.test.lua > @@ -0,0 +1,51 @@ > +#!/usr/bin/env tarantool > + > +local tap = require('tap') > +local console = require('console') The line above is excess. > +local fio = require('fio') > + > +-- The following cases use LD_PRELOAD, so will not work under > +-- Mac OS X. > +if jit.os == 'OSX' then > + os.exit(0) > +end > + > +test = tap.test("console") > +test:plan(1) > + > +-- Allow to run the test from the repository root without > +-- test-run: > +-- > +-- $ ./src/tarantool test/app-tap/gh-4317.test.lua > +-- > +-- It works at least for in-source build. > +local is_under_test_run = pcall(require, 'test_run') > +local isatty_lib_dir = is_under_test_run and '../..' or 'test' > +local isatty_lib_path = fio.pathjoin(isatty_lib_dir, 'libisatty.so') > +local saved_path = os.getenv('PATH') > +if not is_under_test_run then > + os.setenv('PATH', './src:', saved_path) IMHO, using relative paths for such case is too fragile: | $ ./tarantool ../test/app-tap/gh-4317.test.lua | TAP version 13 | 1..1 | not ok - Backslash | --- | filename: ../test/app-tap/gh-4317.test.lua | trace: | - line: 0 | source: '@../test/app-tap/gh-4317.test.lua' | filename: ../test/app-tap/gh-4317.test.lua | what: main | namewhat: | src: ../test/app-tap/gh-4317.test.lua | line: 0 | expected: "tarantool> local a = 0 \\\n > for i = 1, 10 do\n > a = | a + i\n > end \\\n > print(a)\n55\n---\n...\n\ntarantool> " | got: | ... | # failed subtest: 1 Yes, it's not the way you described in the comment above, *but* one can try to run the chunk this way. You can use arg[-1] that provides the Lua interpreter running this script. Consider the following: | $ cat test.lua | print(inspect(arg)) | $ LUA_PATH=~/.luarocks/share/lua/5.1/?.lua ./src/tarantool -linspect test.lua | { | [-1] = "/home/imun/tarantool/src/tarantool", | [0] = "test.lua" | } I attached the diff at the end of reply: it makes possible to run the given test any desired way for both in-source and out-of-source builds (NB: test Lua files for out-of-source build are located within the source directory). > +end > + > +local tarantool_command = "local a = 0 \\\nfor i = 1, 10 do\na = a + i\nend \\\nprint(a)" > +local result_str = [[tarantool> local a = 0 \ > + > for i = 1, 10 do > + > a = a + i > + > end \ > + > print(a) > +55 > +--- > +... > + > +tarantool> ]] > +local cmd = ([[printf '%s\n' | LD_PRELOAD='%s' tarantool ]] .. > +[[2>/dev/null]]):format(tarantool_command, isatty_lib_path) > +local fh = io.popen(cmd, 'r') > +-- Readline on CentOS 7 produces \e[?1034h escape > +-- sequence before tarantool> prompt, remove it. > +local result = fh:read('*a'):gsub('\x1b%[%?1034h', '') > +fh:close() > +test:is(result, result_str, "Backslash") > + > +test:check() > +os.exit(0) Please use the following idiom[1] in your tests: | Use os.exit(test:check() and 0 or 1) at the end. > -- > 2.20.1 (Apple Git-117) > Here is the diff: ================================================================================ diff --git a/test/app-tap/gh-4317.test.lua b/test/app-tap/gh-4317.test.lua index d7a5063e7..82c48d302 100755 --- a/test/app-tap/gh-4317.test.lua +++ b/test/app-tap/gh-4317.test.lua @@ -2,7 +2,6 @@ local tap = require('tap') local console = require('console') -local fio = require('fio') -- The following cases use LD_PRELOAD, so will not work under -- Mac OS X. @@ -13,19 +12,10 @@ end test = tap.test("console") test:plan(1) --- Allow to run the test from the repository root without --- test-run: --- --- $ ./src/tarantool test/app-tap/gh-4317.test.lua --- --- It works at least for in-source build. -local is_under_test_run = pcall(require, 'test_run') -local isatty_lib_dir = is_under_test_run and '../..' or 'test' -local isatty_lib_path = fio.pathjoin(isatty_lib_dir, 'libisatty.so') -local saved_path = os.getenv('PATH') -if not is_under_test_run then - os.setenv('PATH', './src:', saved_path) -end +-- Locate libisattyLso related to the running tarantool binary. +local binary = arg[-1] +local isatty_lib_path = binary:gsub('tarantool$', '../test/libisatty.so') +assert(isatty_lib_path:match('libisatty.so$'), 'Failed to locate libisatty.so') local tarantool_command = "local a = 0 \\\nfor i = 1, 10 do\na = a + i\nend \\\nprint(a)" local result_str = [[tarantool> local a = 0 \ @@ -38,8 +28,8 @@ local result_str = [[tarantool> local a = 0 \ ... tarantool> ]] -local cmd = ([[printf '%s\n' | LD_PRELOAD='%s' tarantool ]] .. -[[2>/dev/null]]):format(tarantool_command, isatty_lib_path) +local cmd = ([[printf '%s\n' | LD_PRELOAD='%s' %s 2>/dev/null]]) + :format(tarantool_command, isatty_lib_path, binary) local fh = io.popen(cmd, 'r') -- Readline on CentOS 7 produces \e[?1034h escape -- sequence before tarantool> prompt, remove it. ================================================================================ [1]: https://github.com/tarantool/doc/issues/1004 -- Best regards, IM