Re: [Tarantool-patches] [PATCH 2/2] console: add line carrying backslash

[Igor Munkin <imun@tarantool.org>]

Olya,

Thanks for the patch!

On 10.06.20, Olga Arkhangelskaia wrote:
> When using interactive console(stdin) instead of \set delimiter <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