Merge remote-tracking branch 'myk002/myk_negative_number_params' into develop

docs/changelog.txt

@@ -45,7 +45,10 @@ changelog.txt uses a syntax similar to RST, with a few special sequences:
 
 ## Lua
 - ``gui.Painter``: fixed error when calling ``viewport()`` method
-- `xlsxreader`: added Lua class wrappers for the xlsxreader plugin API
+- ``utils.processArgsGetopt()``: now returns negative numbers (e.g. ``-10``) in the list of positional parameters instead of treating it as an option string equivalent to ``-1 -0``
+- ``utils.processArgsGetopt()``: now properly handles ``--`` like GNU ``getopt`` as a marker to treat all further parameters as non-options
+- ``utils.processArgsGetopt()``: now detects when required arguments to long-form options are missing
+- `xlsxreader`: Added Lua class wrappers for the xlsxreader plugin API
 
 ## Documentation
 - Added more client library implementations to the `remote interface docs <remote-client-libs>`

library/lua/3rdparty/alt_getopt.lua

@@ -21,10 +21,10 @@
 
 -- based on https://github.com/LuaDist/alt-getopt/blob/master/alt_getopt.lua
 -- MIT licence
--- modified to support aggregation of non-options and to call qerror instead of
+-- modified to support negative numbers as non-options, to aggregate non-options
--- os.exit() on error. can be used directly or via the utils.processArgs2()
+-- for return, and to call error/qerror instead of os.exit on error. can be used
--- wrapper.
+-- directly or via the utils.processArgsGetopt() wrapper.
-
+--
 -- sh_opts should be in standard getopt format: a string of letters that
 -- represent options, each followed by a colon if that option takes an argument.
 -- e.g.: 'ak:hv' has three flags (options with no arguments): 'a', 'h', and 'v'
@@ -35,6 +35,13 @@
 --   -abcd is equivalent to -a -b -c -d if none of them accept arguments.
 --   -abckVALUE and -abck VALUE are also acceptable (where k is the only option
 --      in the string that takes a value).
+--
+-- Note that arguments that have a number as the second character are
+-- interpreted as positional parameters and not options. For example, the
+-- following strings are never interpreted as options:
+--   -10
+--   -0
+--   -1a
 
 local _ENV = mkmodule('3rdparty.alt_getopt')
 
@@ -69,7 +76,7 @@ local function canonicalize(options, opt)
     end
 
     if type(options[opt]) ~= 'number' then
-        qerror(string.format(
+        error(string.format(
                 'Option "%s" resolves to non-number for has_arg flag', opt))
     end
 
@@ -93,6 +100,7 @@ function get_ordered_opts(args, sh_opts, long_opts)
         local a = args[optind]
         if a == '--' then
             optind = optind + 1
+            break
         elseif a:sub(1, 2) == '--' then
             local pos = a:find('=', 1, true)
             if pos then
@@ -106,7 +114,7 @@ function get_ordered_opts(args, sh_opts, long_opts)
                 local opt = a:sub(3)
                 opts[count] = opt
                 if has_arg(options, opt) then
-                    if i == #args then
+                    if optind == #args then
                         qerror(string.format(
                                 'Missing value for option "%s"', a))
                     end
@@ -115,7 +123,7 @@ function get_ordered_opts(args, sh_opts, long_opts)
                 end
             end
             count = count + 1
-        elseif a:sub(1, 1) == '-' then
+        elseif a:sub(1, 1) == '-' and not tonumber(a:sub(2, 2)) then
             local j
             for j=2,#a do
                 local opt = canonicalize(options, a:sub(j, j))

library/lua/utils.lua

@@ -620,6 +620,9 @@ end
 -- can be combined into a single option string (e.g. '-abc' is the same as
 -- '-a -b -c' if options 'a' and 'b' do not take arguments.
 --
+-- Numbers cannot be options and negative numbers (e.g. -10) will be interpreted
+-- as positional parameters and returned in the nonoptions list.
+--
 -- optionActions is a vector with elements in the following format:
 -- {shortOptionName, longOptionAlias, hasArg=boolean, handler=fn}
 -- shortOptionName and handler are required. If the option takes an argument,

test/library/utils.lua

@@ -40,3 +40,107 @@ function test.invert_overwrite()
     expect.eq(i.b, 2)
     expect.eq(i.a, 3)
 end
+
+function test.processArgsGetopt_happy_path()
+    local quiet, verbose, name
+
+    local function process(args, expected_q, expected_v, expected_n)
+        quiet, verbose, name = false, false, nil
+        local nonoptions = utils.processArgsGetopt(args, {
+            {'q', handler=function() quiet = true end},
+            {'v', 'verbose', handler=function() verbose = true end},
+            {'n', 'name', hasArg=true,
+                handler=function(optarg) name = optarg end},
+        })
+        expect.eq(expected_q, quiet)
+        expect.eq(expected_v, verbose)
+        expect.eq(expected_n, name)
+        return nonoptions
+    end
+
+    local args = {}
+    expect.table_eq({}, process(args, false, false, nil))
+
+    args = {'-q'}
+    expect.table_eq({}, process(args, true, false, nil))
+
+    args = {'-v'}
+    expect.table_eq({}, process(args, false, true, nil))
+
+    args = {'--verbose'}
+    expect.table_eq({}, process(args, false, true, nil))
+
+    args = {'-n', 'foo'}
+    expect.table_eq({}, process(args, false, false, 'foo'))
+
+    args = {'-n', 'foo'}
+    expect.table_eq({}, process(args, false, false, 'foo'))
+
+    args = {'-nfoo'}
+    expect.table_eq({}, process(args, false, false, 'foo'))
+
+    args = {'--name', 'foo'}
+    expect.table_eq({}, process(args, false, false, 'foo'))
+
+    args = {'--name=foo'}
+    expect.table_eq({}, process(args, false, false, 'foo'))
+
+    args = {'-vqnfoo'}
+    expect.table_eq({}, process(args, true, true, 'foo'))
+
+    args = {'nonopt1', '-nfoo', 'nonopt2', '-1', '-10', '-0v'}
+    expect.table_eq({'nonopt1', 'nonopt2', '-1', '-10', '-0v'},
+                    process(args, false, false, 'foo'))
+
+    args = {'nonopt1', '--', '-nfoo', '--nonopt2', 'nonopt3'}
+    expect.table_eq({'nonopt1', '-nfoo', '--nonopt2', 'nonopt3'},
+                    process(args, false, false, nil))
+end
+
+function test.processArgsGetopt_action_errors()
+    expect.error_match('missing option letter',
+        function() utils.processArgsGetopt({}, {{handler=function() end}}) end)
+
+    expect.error_match('missing option letter',
+        function() utils.processArgsGetopt({}, {{'notoneletter'}}) end)
+
+    expect.error_match('missing option letter',
+        function() utils.processArgsGetopt({}, {{function() end}}) end)
+
+    expect.error_match('handler missing',
+        function() utils.processArgsGetopt({}, {{'r'}}) end)
+end
+
+function test.processArgsGetopt_parsing_errors()
+    expect.error_match('Unknown option',
+                       function() utils.processArgsGetopt({'-abc'},
+                                    {{'a', handler=function() end}})
+                       end,
+                       'use undefined short option')
+
+    expect.error_match('Unknown option',
+                       function() utils.processArgsGetopt({'--abc'},
+                                    {{'a', handler=function() end}})
+                       end,
+                       'use undefined long option')
+
+    expect.error_match('Bad usage',
+                       function() utils.processArgsGetopt({'--ab=c'},
+                                    {{'a', 'ab', handler=function() end}})
+                       end,
+                       'pass value to param that does not take one')
+
+    expect.error_match('Missing value',
+                       function() utils.processArgsGetopt({'-a'},
+                                    {{'a', 'ab', hasArg=true,
+                                      handler=function() end}})
+                       end,
+                       'fail to pass value to short param that requires one')
+
+    expect.error_match('Missing value',
+                       function() utils.processArgsGetopt({'--ab'},
+                                    {{'a', 'ab', hasArg=true,
+                                      handler=function() end}})
+                       end,
+                       'fail to pass value to long param that requires one')
+end