diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua index 8dae388b1e82ec7fac9d8cbfabcaa380cf987413..faa0ae130699fa693525df4ebdf09f75de4e0a26 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) @@ -21,13 +31,18 @@ local function result_set_add(t_out, key, val) end local function err_bad_parameter_value(name, got, expected) - if type(got) ~= 'string' then - got = 'nothing' + assert(type(got) == 'boolean' or type(got) == 'string') + assert(type(got) ~= expected) + + local reason + if type(got) == 'boolean' then + reason = ('Expected %s, got nothing'):format(expected) + elseif not parameter_has_value(expected) then + reason = ('No value expected, got "%s"'):format(got) else - got = string.format('"%s"', got) + reason = ('Expected %s, got "%s"'):format(expected, got) end - error(string.format('Bad value for parameter "%s". Expected %s, got %s', - name, expected, got)) + error(string.format('Bad value for parameter "%s". %s', name, reason)) end local function convert_parameter_simple(name, convert_from, convert_to) @@ -38,17 +53,9 @@ local function convert_parameter_simple(name, convert_from, convert_to) end return converted elseif convert_to == 'boolean' then - if type(convert_from) == 'boolean' then - return convert_from - end - convert_from = string.lower(convert_from) - if convert_from == '0' or convert_from == 'false' then - return false - end - if convert_from == '1' or convert_from == 'true' then - return true + if type(convert_from) ~= 'boolean' then + return err_bad_parameter_value(name, convert_from, convert_to) end - return err_bad_parameter_value(name, convert_from, convert_to) elseif convert_to == 'string' then if type(convert_from) ~= 'string' then return err_bad_parameter_value(name, convert_from, convert_to) @@ -85,6 +92,20 @@ end local function parameters_parse(t_in, options) local t_out, t_in = {}, t_in or {} + + -- Prepare a lookup table for options. An option name -> a + -- type name to convert a parameter into or true (which means + -- returning a value as is). + local lookup = {} + if options then + for _, v in ipairs(options) do + if type(v) ~= 'table' then + v = {v} + end + lookup[v[1]] = (v[2] or true) + end + end + local skip_param = false for i, v in ipairs(t_in) do -- we've used this parameter as value @@ -108,13 +129,20 @@ local function parameters_parse(t_in, options) if key == nil or val == nil then error(("bad argument #%d: ID not valid"):format(i)) end + -- Disallow an explicit value after '=' for a + -- 'boolean' or 'boolean+' argument. + 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) else if command:match("^([%a_][%w_-]+)$") == nil then error(("bad argument #%d: ID not valid"):format(i)) end local val = true - do + -- Don't consume a value after a 'boolean' or + -- 'boolean+' argument. + 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) @@ -133,13 +161,7 @@ local function parameters_parse(t_in, options) ::nextparam:: end if options then - local lookup, unknown = {}, {} - for _, v in ipairs(options) do - if type(v) ~= 'table' then - v = {v} - end - lookup[v[1]] = (v[2] or true) - end + local unknown = {} for k, v in pairs(t_out) do if lookup[k] == nil and type(k) == "string" then table.insert(unknown, k) diff --git a/test/app/argparse.result b/test/app/argparse.result index 04f0439996a34025491345ec1a807ff7e39bab01..f3a634e1d7eb07e688b3134c515b6fe5c0fa0d0b 100644 --- a/test/app/argparse.result +++ b/test/app/argparse.result @@ -137,13 +137,13 @@ argparse({'--verh=42'}, {'verh'}) ... argparse({'--verh=42'}, {{'verh', 'boolean'}}) --- -- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". Expected - boolean, got "42"' +- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". No value + expected, got "42"' ... argparse({'--verh=42'}, {{'verh', 'boolean+'}}) --- -- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". Expected - boolean, got "42"' +- error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "verh". No value + expected, got "42"' ... argparse({'--verh=42'}, {'niz'}) --- diff --git a/test/app/gh-4076-argparse-wrong-bool-handling.result b/test/app/gh-4076-argparse-wrong-bool-handling.result index fd933a37dc7f7f63c9d7678dda78d68e023142bf..0abbc7bb76d48dd322c16312c4c26ee0583252b7 100644 --- a/test/app/gh-4076-argparse-wrong-bool-handling.result +++ b/test/app/gh-4076-argparse-wrong-bool-handling.result @@ -25,31 +25,98 @@ params[2] = {'flag2', 'boolean'} params[3] = {'flag3', 'boolean'} | --- | ... -params[4] = {'flag4', 'boolean'} +args = {'--flag1', 'positional value', '--flag2'} | --- | ... -params[5] = {'flag5', 'boolean'} +argparse(args, params) + | --- + | - 1: positional value + | flag1: true + | flag2: true + | ... + +-- +-- When several 'boolean' arguments are passed, the result will be +-- `true` (just as for one such argument). +-- +params = {} + | --- + | ... +params[1] = {'foo', 'boolean'} | --- | ... -args = {'--flag1', 'true', '--flag2', '1', '--flag3', 'false', '--flag4', '0', '--flag5', 'TrUe'} +args = {'--foo', '--foo'} | --- | ... argparse(args, params) | --- - | - flag4: false - | flag1: true - | flag5: true - | flag2: true - | flag3: false + | - foo: true | ... -args = {'--flag1', 'abc'} +-- +-- When several 'boolean+' arguments are passed, the result will +-- be a list of `true` values. +-- +params = {} + | --- + | ... +params[1] = {'foo', 'boolean+'} + | --- + | ... +args = {'--foo', '--foo'} | --- | ... argparse(args, params) | --- - | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "flag1". Expected - | boolean, got "abc"' + | - foo: + | - true + | - true + | ... + +params = {} + | --- + | ... +params[1] = {'foo', 'boolean+'} + | --- + | ... +args = {'--foo', 'positional value', '--foo'} + | --- + | ... +argparse(args, params) + | --- + | - foo: + | - true + | - true + | 1: positional value + | ... + +-- +-- When a value is provided for a 'boolean' / 'boolean+' option +-- using --foo=bar syntax, the error should state that a value is +-- not expected for this option. +-- +params = {} + | --- + | ... +params[1] = {'foo', 'boolean'} + | --- + | ... +argparse({'--foo=bar'}, params) + | --- + | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "foo". No value + | expected, got "bar"' + | ... + +params = {} + | --- + | ... +params[1] = {'foo', 'boolean+'} + | --- + | ... +argparse({'--foo=bar'}, params) + | --- + | - error: 'builtin/internal.argparse.lua:<line>"]: Bad value for parameter "foo". No value + | expected, got "bar"' | ... -- @@ -69,7 +136,10 @@ argparse({'--value'}, params) | number, got nothing' | ... -params[1][2] = 'string' +params = {} + | --- + | ... +params[1] = {'value', 'string'} | --- | ... argparse({'--value'}, params) @@ -78,6 +148,55 @@ argparse({'--value'}, params) | string, got nothing' | ... +-- +-- Verify that short 'boolean' and 'boolean+' options behaviour +-- is the same as for long options. +-- +params = {} + | --- + | ... +params[1] = {'f', 'boolean'} + | --- + | ... +args = {'-f'} + | --- + | ... +argparse(args, params) + | --- + | - f: true + | ... +args = {'-f', '-f'} + | --- + | ... +argparse(args, params) + | --- + | - f: true + | ... + +params = {} + | --- + | ... +params[1] = {'f', 'boolean+'} + | --- + | ... +args = {'-f'} + | --- + | ... +argparse(args, params) + | --- + | - f: + | - true + | ... +args = {'-f', '-f'} + | --- + | ... +argparse(args, params) + | --- + | - f: + | - true + | - true + | ... + test_run:cmd("clear filter") | --- | - true diff --git a/test/app/gh-4076-argparse-wrong-bool-handling.test.lua b/test/app/gh-4076-argparse-wrong-bool-handling.test.lua index a8ee5e67037473309a0d334e1a37fb21bd1910f4..b4ce4c7e689695db2f119750dd566a673e7ff8d7 100644 --- a/test/app/gh-4076-argparse-wrong-bool-handling.test.lua +++ b/test/app/gh-4076-argparse-wrong-bool-handling.test.lua @@ -9,14 +9,45 @@ params = {} params[1] = {'flag1', 'boolean'} params[2] = {'flag2', 'boolean'} params[3] = {'flag3', 'boolean'} -params[4] = {'flag4', 'boolean'} -params[5] = {'flag5', 'boolean'} -args = {'--flag1', 'true', '--flag2', '1', '--flag3', 'false', '--flag4', '0', '--flag5', 'TrUe'} +args = {'--flag1', 'positional value', '--flag2'} argparse(args, params) -args = {'--flag1', 'abc'} +-- +-- When several 'boolean' arguments are passed, the result will be +-- `true` (just as for one such argument). +-- +params = {} +params[1] = {'foo', 'boolean'} +args = {'--foo', '--foo'} +argparse(args, params) + +-- +-- When several 'boolean+' arguments are passed, the result will +-- be a list of `true` values. +-- +params = {} +params[1] = {'foo', 'boolean+'} +args = {'--foo', '--foo'} +argparse(args, params) + +params = {} +params[1] = {'foo', 'boolean+'} +args = {'--foo', 'positional value', '--foo'} argparse(args, params) +-- +-- When a value is provided for a 'boolean' / 'boolean+' option +-- using --foo=bar syntax, the error should state that a value is +-- not expected for this option. +-- +params = {} +params[1] = {'foo', 'boolean'} +argparse({'--foo=bar'}, params) + +params = {} +params[1] = {'foo', 'boolean+'} +argparse({'--foo=bar'}, params) + -- -- When parameter value was omitted, it was replaced internally -- with boolean true, and sometimes was showed in error messages. @@ -26,7 +57,26 @@ params = {} params[1] = {'value', 'number'} argparse({'--value'}, params) -params[1][2] = 'string' +params = {} +params[1] = {'value', 'string'} argparse({'--value'}, params) +-- +-- Verify that short 'boolean' and 'boolean+' options behaviour +-- is the same as for long options. +-- +params = {} +params[1] = {'f', 'boolean'} +args = {'-f'} +argparse(args, params) +args = {'-f', '-f'} +argparse(args, params) + +params = {} +params[1] = {'f', 'boolean+'} +args = {'-f'} +argparse(args, params) +args = {'-f', '-f'} +argparse(args, params) + test_run:cmd("clear filter")