Removing entries from a seq results in a non-zero exit

[0branch]

Attempting to remove entries from a seq with the plural update operation -= leads to a non-zero exit.

For example, given cli.nim:

import cligen # 1.5.23

proc foo(bar = @["a", "b", "c"]) = echo bar

dispatch(foo)

addition works fine

$ ./cli --bar+=d
@["a", "b", "c", "d"]
$ echo $?
0

but removal doesn't

$ ./cli --bar-=a
$ echo $?
1

Expected output: @["b", "c"].

[c-blake]

Thanks for the bug report! I reproduced it no problem. This works if you change your test from a seq[string] to a set[enum] (and probably a HashSet[string], too). It looks like a logic bug at cligen/argcvt:383. My quick guess is some exception gets raised, but I will probably have to look at it in more detail tomorrow.

[c-blake]

I am pretty sure this is fixed in master, but let me know if I am wrong. I can punch a new release soon-ish for you, too. Let me know if you are impatient about that.

[0branch]

Thanks! The sample is working now.

Re: c353e76, I'd just printed the exception here when I saw you'd closed the issue:

(...)/nim-1.6.4/lib/system/iterators.nim(173, 11)
    `len(a) == L` the length of the seq changed while iterating over it

Thanks for the prompt resolution. No need to rush out a release just for me; please take your time.

[c-blake]

I never did see that assert message print. My nimsuggest was misbehaving, as is common, and I didn't even read the delete definition when I wrote that commit message. I just read my own code more carefully, and intuited the problem (correctly as it turned out...).

Unsure removing every "a" for the CLuser in your -=a example is ideal. At least to me, that does make the seq variant of -= seem most like set[] and HashSet[] which is..something. If you really want, we could have -= remove only the first match and --= remove all or something. BUT...All this aggregate updating stuff is already much more than CLusers expect/probably use, any CLauthor can basically do all that already anyway with some work, there's not really a "standard" in the world for this kind of thing, most systems are not as flexible as cligen, and defaults can be tricky. So, probably nothing more to do here.

[0branch]

Unsure removing every "a" for the CLuser in your -=a example is ideal. At least to me, that does make the seq variant of -= seem most like set[] and HashSet[] which is..something. If you really want, we could have -= remove only the first match and --= remove all or something. BUT...All this aggregate updating stuff is already much more than CLusers expect/probably use, any CLauthor can basically do all that already anyway with some work, there's not really a "standard" in the world for this kind of thing, most systems are not as flexible as cligen, and defaults can be tricky. So, probably nothing more to do here.

Given that cligen's behaviour is type driven, differentiation between set and seq for these operators might make sense.

Related: more surprising than -= removing all instances is bare = performing an append rather than assignment.

Expectation: --bar=d would result in @["d"], not @["a", "b", "c", "d"] (which can already be obtained using +=, or even ^= to prepend the "d").

[c-blake]

I agree = equiv to += is surprising within a cligen setting, but I was trying to mimic what little standard behavior there is for long options in unrelated tools..e.g., cc -Ipath1 -Ipath2. I haven't really done a statistical survey, but I suspect we would be in the minority surprise-wise in "the greater toolscape". I consider this notational defect as similar to square brackets ([]) for both keyed/associative access as well as positional access where I also prefer distinct notation, but tend to capitulate to the masses.

If you really hate it, I am pretty sure it is possible for a CLauthor to provide their own entire cligen/argcvt.nim in some subdir of their package or maybe earlier in their Nim path (and you would want to replace syntaxHelp.nim as well).

[0branch]

I agree = equiv to += is surprising within a cligen setting, but I was trying to mimic what little standard behavior there is for long options in unrelated tools..e.g., cc -Ipath1 -Ipath2.

Good point.

Perhaps it's the explicit = that's misleading (though it's common in cases like -Dfoo=bar as you mention)?

I haven't really done a statistical survey, but I suspect we would be in the minority surprise-wise in "the greater toolscape". I consider this notational defect as similar to square brackets ([]) for both keyed/associative access as well as positional access where I also prefer distinct notation, but tend to capitulate to the masses.

I'd guess that few parsers handle collection types well, if at all.

If you really hate it, I am pretty sure it is possible for a CLauthor to provide their own entire cligen/argcvt.nim in some subdir of their package or maybe earlier in their Nim path (and you would want to replace syntaxHelp.nim as well).

To be clear, I don't "hate" it: the behaviour is just a bit surprising. (In my cruder parseopt-based implementation, I was just splitting on "," to obtain a seq[string]: --bar=a,b,c vs. --bar=a etc.)

[c-blake]

Yeah...But then the equivalence of --foo=blah and --foo blah is probably even more standard than -Ipath1 -Ipath2. :-( Anyway, it was the best compromise I could come up with.

OTOH, -= only worked for like half a year before that assert addition broke it for 3 years. So, changing that would be easier since it's a safe bet no one was successfully using it. ;-)

There are other possibilities like trying to "parameterize" such behaviors, maybe with some defines or adding something like @= to clobber-set. I get you're not really asking. I'm just expressing being open-minded, but also liking to not disrupt other users since many seem to use this package.

[0branch]

There are other possibilities like trying to "parameterize" such behaviors, maybe with some defines or adding something like @= to clobber-set. I get you're not really asking. I'm just expressing being open-minded, but also liking to not disrupt other users since many seem to use this package.

Appreciated. Sidenote: --help-syntax is excellent.

Thanks for creating cligen.

[c-blake]

I usually just type --helps at the command-line. :-) and you're welcome.