From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (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 457F046971A for ; Wed, 11 Dec 2019 02:20:17 +0300 (MSK) References: <20191015155953.10246-1-roman.habibov@tarantool.org> <822D629F-3969-4FD2-B5A3-B30C1EB7FB52@tarantool.org> From: Vladislav Shpilevoy Message-ID: <33fc5170-2cd5-8b2f-83d6-ebbe8a78aac0@tarantool.org> Date: Wed, 11 Dec 2019 00:20:15 +0100 MIME-Version: 1.0 In-Reply-To: <822D629F-3969-4FD2-B5A3-B30C1EB7FB52@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [tarantool-patches] [PATCH] json: improve error messages List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! On 06/12/2019 22:16, Roman Khabibov wrote: > Hi! Thanks for the review. > >> On Oct 17, 2019, at 00:45, Vladislav Shpilevoy wrote: >> >> Hi! Thanks for the patch! >> >> Sorry, but I am not sure that now the messages are >> better. Just look at it: >> >> tarantool> json.decode('{') >> --- >> - error: Expected object key string but found '\0' on line 1 at character 2 here ^^^... >> ... >> >> What is `^^^`, what is `...`? In the ticket it is said, that a user should have a >> context, but there is no a context. >> >> I was thinking, that the author of 4339 rather meant that ^^^ should be below the >> error in an error string, like this: >> >> tarantool> json.decode('{') >> --- >> - error: Expected object key string but found '\0' on line 1 at character 2 here: >> "{ " >> ^^^ >> ... >> >> But I do realize, that it is too complex. And still ^^^ looks confusing. >> It looks like arrows pointing up, while you are trying to put a >> context after them. >> >> At least, you could make a context like this: >> >> tarantool> json.decode('{') >> --- >> - error: Expected object key string but found '\0' on line 1 at character 2 here >> "{>>\0<<" >> ... >> >> So you would have arrows pointing right at the bad position. >> >> I tried an error not related to eof, and got the same confusing >> result: >> >> tarantool> json.decode('{1: "world"}') >> --- >> - error: 'Expected object key string but found int on line 1 at character 2 here ^^^: >> "world"}...' >> ... >> >> From that message looks like the error is in ':', or in "world", or >> somewhere above because of up arrows. > Let’s just remove “T_*” tokens. Let print of a context will be “wontfix". Double > arrows isn’t very good, IMO, because we don’t know the length of wrong token. > Sorry, but the context is a part of the ticket. The ticket consists of 3 points: - Replace T_*; - Provide context; - Provide line number. Line numbers already work. This patch removes T_*. But the context still is not provided. Context is asked not because it is necessary. But because it is really hard to count character count, when it is big. Please, add the context. Talking of "don't know the length of a wrong token" - ok, then you can provide just arrows pointing at the error from left, if you don't know right end: tarantool> json.decode('{blablabla') --- - error: Expected object key string but found 'b' on line 1 at character 2 here "{ >> blabla..." ... > @@ -154,4 +154,34 @@ tap.test("json", function(test) > _, err_msg = pcall(serializer.decode, '{"hello": "world",\n 100: 200}') > test:ok(string.find(err_msg, 'line 2 at character 2') ~= nil, > 'mistake on second line') > + > + -- > + -- gh-3316: Make sure that tokens 'T_*' are absent in error Double whitespace after 'are'.