Not parsing the last argument correctly

[MartijnKuipers]

Hi, I am either doing something wrong or not able to parse the last argument correctly. I am not sure I am seeing a single bug or two. There seems to be something weird going on with the last argument, and the second issue I am seeing is that the (non-optional) arguments are not removed from the resulting argv.

Please find attached my minimal test-code and an input file last-arg.zip.

The input file looks like this:

test:
  test1: false
  test2: false

I have the following options:

• -t1 set test1 to true
• -t2 set test2 true
• -t3 set test3 to the non-optional argument "argument"

I then print the resulting YAML tree to stdout.

Test 1: OK

./c4conf_parsing ../input.yaml 
test:
  test1: false
  test2: false

Test 2: OK

./c4conf_parsing ../input.yaml -t1
test:
  test1: true
  test2: false

Test 3: Not OK. I was expecting test1 and test2 to be true.

./c4conf_parsing ../input.yaml -t1 -t2
test:
  test1: true
  test2: false

Test 4: Not OK. I was expecting test1 and test2 to be true.

./c4conf_parsing ../input.yaml -t2 -t1
test:
  test1: false
  test2: true

Test 5: Not OK. I was expecting test1 and test2 to be true.

./c4conf_parsing ../input.yaml -t1 -t2 -t3 test
test:
  test1: true
  test2: false
  test3: test

Test 6: Not OK. I was expecting that the (non optional) argument to -t3 also was removed from the remaining argv...

./c4conf_parsing ../input.yaml -t1 -t3 test -t2
Requires one file for the starting tree
Given:  ../input.yaml  test

Let me know if I can assist in any way. As you can see from my code I am a C-programmer doing C++ and have not really used C++ for 20 years (the C++17 standard has improved things significantly).

Kind regards, Martijn

[MartijnKuipers]

I think I found it. The check for checking optional arguments is never returning false in the case that there is no optional argument in the spec. The below patch seems to solve all the issues I was seeing, but I don't know the code well enough to be sure it is the correct approach.

diff --git a/src/c4/conf/conf.cpp b/src/c4/conf/conf.cpp
index 31f33ec..c5e4d40 100644
--- a/src/c4/conf/conf.cpp
+++ b/src/c4/conf/conf.cpp
@@ -492,6 +492,7 @@ size_t parse_opts(int *argc, char ***argv,
             {
                 C4_ASSERT(*argc >= iarg + 1);
                 _dbg("arg[" << iarg << "]=" << getarg(iarg) << ": no arg expected");
+                return false;
             }
             else
             {

[biojppm]

Thanks for reporting. I'll create some tests for this.