From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olegrok@tarantool.org>
Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251])
 (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 A01FB452566
 for <tarantool-patches@dev.tarantool.org>;
 Mon, 11 Nov 2019 17:43:40 +0300 (MSK)
References: <20191022131429.39400-1-olegrok@tarantool.org>
 <20191024062150.uzuct2lkmrix3cne@tarantool.org>
From: Oleg Babin <olegrok@tarantool.org>
Message-ID: <fa9b79a7-4fb6-7260-1db0-916fad82a0ee@tarantool.org>
Date: Mon, 11 Nov 2019 17:43:26 +0300
MIME-Version: 1.0
In-Reply-To: <20191024062150.uzuct2lkmrix3cne@tarantool.org>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH] error: Add
 __concat method to error object
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org>, tarantool-patches@dev.tarantool.org
Cc: Oleg Babin <babinoleg@mail.ru>

Hello, thank you for review!


I will add test in second version of patch.

> Hello,
>
> On 22 окт 16:14, Kirill Yukhin wrote:
> Why do you prohibit rhs to be nil. If needed, should we also disable
> box.NULL?
>

We expect that error will have behavior as usual string then we should
prohibit

using of nils and box.NULLs:


```


tarantool> 'simple string' .. nil
---
- error: '[string "return ''simple string'' .. nil"]:1: attempt to
concatenate a nil
    value'
...

tarantool> 'simple string' .. box.NULL
---
- error: '[string "return ''simple string'' .. box.NULL"]:1: attempt to
concatenate
    ''string'' and ''void *'''
...


```

In this case "attempt to concatenate struct error and nil" will be returned


On 24/10/2019 09:21, Kirill Yukhin wrote:
> Hello,
>
> On 22 окт 16:14, Kirill Yukhin wrote:
>> From: Oleg Babin <babinoleg@mail.ru>
>>
>> Usually functions return pair {nil, err} and expected that err is string.
>> Let's make the behaviour of error object closer to string
>> and define __concat metamethod.
>>
>> Closes tarantool/tarantool#4489
>> ---
>>  src/lua/error.lua | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/src/lua/error.lua b/src/lua/error.lua
>> index 28fc0377d..b44fab7b6 100644
>> --- a/src/lua/error.lua
>> +++ b/src/lua/error.lua
>> @@ -150,9 +150,17 @@ local function error_index(err, key)
>>      return error_methods[key]
>>  end
>>  
>> +local function error_concat(lhs, rhs)
>> +    if lhs == nil or rhs == nil then
>> +        error("attempt to concatenate struct error and nil")
>> +    end
> Why do you prohibit rhs to be nil. If needed, should we also disable
> box.NULL?
>
> Why you didn't add a test?
>
> --
> Regards, Kirill Yukhin
>
--

Oleg Babin