[Tarantool-patches] [PATCH] app/argparse: expect no value for a boolean option

[Alexander Turenko]

Updated the patch. See the incremental diff below.

WBR, Alexander Turenko.

> > Fixes #4076 (again).
> 
> 1. Strictly speaking, it is not a fix, it is a follow-up. The bug is
> fixed already.

Closed the issue back, marked the commit as the follow up.

> > ---
> > 
> > https://github.com/tarantool/tarantool/issues/4076
> > https://github.com/tarantool/tarantool/tree/Totktonada/gh-4076-argparse-no-value-for-boolean-arg

> > +                -- Don't consume a value after a 'boolean' or
> > +                -- 'boolean+' argument.
> > +                if lookup[command] ~= 'boolean' and
> > +                        lookup[command] ~= 'boolean+' then
> 
> 2. Something is wrong with the indentation.

Let's consider three variants of splitting long if conditions:

 | if long_cond1 or
 |    long_cond2 then
 |     <...>
 | end

 | if long_cond1 or
 |     long_cond2 then
 |     <...>
 | end

 | if long_cond1 or
 |         long_cond2 then
 |     <...>
 | end

As I see the first and the second variants are used across tarantool's
built-in Lua code. You are right, third one, which I personally prefer,
is not used within the project.

Maybe it is better to always wrap such conditions into a function and
assign to a variable.

Anyway, now it is wrapped into a function.

> 3. This check "type ~= boolean and type ~= boolean+" is repeated 3
> times in this file now. I would add a function like 'type_has_value(type)'
> or similar which would encapsulate it.

It is hard to choose a name here. I stopped on parameter_has_value().

> >                      -- in case next argument is value of this key (not --arg)
> >                      local next_arg = t_in[i + 1]
> >                      local is_long, is_short, is_dash = parse_param_prefix(next_arg)

----

diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua
index 45917ec0d..faa0ae130 100644
--- a/src/lua/argparse.lua
+++ b/src/lua/argparse.lua
@@ -8,6 +8,16 @@ local function parse_param_prefix(param)
     return is_long, is_short, is_dash
 end
 
+-- Determine whether a value should be provided for a parameter.
+--
+-- The value can be passed after '=' within the same argument or
+-- in a next argument.
+--
+-- @param convert_to a type of the parameter
+local function parameter_has_value(convert_to)
+    return convert_to ~= 'boolean' and convert_to ~= 'boolean+'
+end
+
 local function result_set_add(t_out, key, val)
     if val == nil then
         table.insert(t_out, key)
@@ -27,7 +37,7 @@ local function err_bad_parameter_value(name, got, expected)
     local reason
     if type(got) == 'boolean' then
         reason = ('Expected %s, got nothing'):format(expected)
-    elseif expected == 'boolean' or expected == 'boolean+' then
+    elseif not parameter_has_value(expected) then
         reason = ('No value expected, got "%s"'):format(got)
     else
         reason = ('Expected %s, got "%s"'):format(expected, got)
@@ -121,7 +131,7 @@ local function parameters_parse(t_in, options)
                 end
                 -- Disallow an explicit value after '=' for a
                 -- 'boolean' or 'boolean+' argument.
-                if lookup[key] == 'boolean' or lookup[key] == 'boolean+' then
+                if not parameter_has_value(lookup[key]) then
                     return err_bad_parameter_value(key, val, lookup[key])
                 end
                 result_set_add(t_out, key, val)
@@ -132,8 +142,7 @@ local function parameters_parse(t_in, options)
                 local val = true
                 -- Don't consume a value after a 'boolean' or
                 -- 'boolean+' argument.
-                if lookup[command] ~= 'boolean' and
-                        lookup[command] ~= 'boolean+' then
+                if parameter_has_value(lookup[command]) then
                     -- in case next argument is value of this key (not --arg)
                     local next_arg = t_in[i + 1]
                     local is_long, is_short, is_dash = parse_param_prefix(next_arg)