Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Roman Khabibov <roman.habibov@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] lua: modify the error message in 'fio.pathjoin'
Date: Mon, 3 Dec 2018 17:25:48 +0300	[thread overview]
Message-ID: <372c9ce1-c96f-fd26-b779-3eeba9665cb0@tarantool.org> (raw)
In-Reply-To: <20181202002729.8968-1-roman.habibov@tarantool.org>

Thanks for the patch! See 2 comments below,
review fix at the end of the email and on the
branch.

On 02/12/2018 03:27, Roman Khabibov wrote:
> Display line number with error and fix the bug with bad argument number.
> 
> Closes #3580
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
> Issue: https://github.com/tarantool/tarantool/issues/3580
> ---
>   src/lua/fio.lua       |  4 ++--
>   test/app/fio.result   | 13 +++++++++++++
>   test/app/fio.test.lua |  5 +++++
>   3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lua/fio.lua b/src/lua/fio.lua
> index 55faebdcb..13a4d36dc 100644
> --- a/src/lua/fio.lua
> +++ b/src/lua/fio.lua
> @@ -213,7 +213,7 @@ fio.pathjoin = function(...)
>       while i <= len do
>           local sp = select(i, ...)
>           if sp == nil then
> -            error("Undefined path part "..i)
> +            error("in function 'pathjoin' undefined path part "..i, 2)

1. Usually we write error messages without saying 'in function'. Also,
you did not specify module name 'fio'. I've fixed it on the branch in a
separate commit. Please, look and squash if it is ok for you.

>           end
>   
>           sp = tostring(sp)

2. The module 'fio' contains some other error messages without
function name. I've fixed them too in a follow-up commit. Look
at it, say if do not agree, but do not squash. It should be
separate.

If all my changes are ok, then send v2 patch to Vova, please.

===================================================================

commit 4ec3b8238faa0515cb13dea43a3fcc38e7ecce18
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Mon Dec 3 17:04:52 2018 +0300

     Review fix

diff --git a/src/lua/fio.lua b/src/lua/fio.lua
index 13a4d36dc..1d0bb4bd1 100644
--- a/src/lua/fio.lua
+++ b/src/lua/fio.lua
@@ -213,7 +213,7 @@ fio.pathjoin = function(...)
      while i <= len do
          local sp = select(i, ...)
          if sp == nil then
-            error("in function 'pathjoin' undefined path part "..i, 2)
+            error("fio.pathjoin() undefined path part "..i, 2)
          end
  
          sp = tostring(sp)
@@ -233,7 +233,7 @@ fio.pathjoin = function(...)
      while i <= len do
          local sp = select(i, ...)
          if sp == nil then
-            error("in function 'pathjoin' undefined path part "..i, 2)
+            error("fio.pathjoin() undefined path part "..i, 2)
          end
  
          sp = tostring(sp)
diff --git a/test/app/fio.result b/test/app/fio.result
index fd3e289c4..9777a0aec 100644
--- a/test/app/fio.result
+++ b/test/app/fio.result
@@ -58,15 +58,15 @@ fio.pathjoin('abc', 'awdeq///qweqwqwe///', "//asda//")
  --gh-3580 Modify the error message in 'fio.pathjoin'.
  fio.pathjoin(nil)
  ---
-- error: in function 'pathjoin' undefined path part 1
+- error: fio.pathjoin() undefined path part 1
  ...
  fio.pathjoin('abc', nil)
  ---
-- error: in function 'pathjoin' undefined path part 2
+- error: fio.pathjoin() undefined path part 2
  ...
  fio.pathjoin('abc', 'cde', nil)
  ---
-- error: in function 'pathjoin' undefined path part 3
+- error: fio.pathjoin() undefined path part 3
  ...
  -- basename
  st, err = pcall(fio.basename, nil)

      reply	other threads:[~2018-12-03 14:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-02  0:27 [tarantool-patches] " Roman Khabibov
2018-12-03 14:25 ` Vladislav Shpilevoy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=372c9ce1-c96f-fd26-b779-3eeba9665cb0@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=roman.habibov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] lua: modify the error message in '\''fio.pathjoin'\''' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox