search

c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
509 stars 24 forks source link

Doesn't split comma separated values in some configs #87

Closed genotrance closed 5 years ago

genotrance commented 5 years ago

If you specify --symOverride=1,2,3 on Linux, it splits it into @["1", "2", "3"] but the same code on Windows ends up with @["1,2,3"].

genotrance commented 5 years ago

Linux also failed on AppVeyor with this test for some reason. Not sure if it is cligen related but worth investigating.

c-blake commented 5 years ago

I don't have a Windows test environment. For me, on Linux, ./test/AllSeqTypes --s=a,b yields s: @["a,b"] which is the expected/documented behavior.

Splitting CSV/multi-argument in one argv slot should not happen anymore. I'm not sure how anything even knows to use comma as a split delimiter on whichever environment, really. I was going to mention that for your last issue as well.

c-blake commented 5 years ago

Besides not being able to reproduce this reported comma splitting on Linux with my own test program, I also cannot reproduce it with toast.nim (current VC head of nim, nimterop, Linux). I hacked an echo symOverride into the first line of your proc main and ./toast -O a,b gave me a seq of len 1. (I know that's not what you want in terms of CLI, but it's what cligen is supposed to deliver to the client.)

To me, it almost seems like a build environment issue with perhaps some old version of cligen lying somewhere in a path or some similar environmental issue. I'm not sure how else a split on comma (as opposed to split on white space or split on anything else) might happen. If you cannot present a reproducible misbehavior here, we should probably close this issue.

Anyway, I think something like your getSplitComma is the answer to get the CLI behavior you want. Debatable if you also want that for include paths or all seq[string] types..Directories rarely have "," in them, and include path directories even more rarely. defines is probably the most likely thing to need CLI-user-defined delimiting. Alternatively to getSplitComma, if you did want your entirely own semantics you could also do your own argParse for seq[string] as test/CustomType.nim does.

With both getSplitComma and custom argParse approaches, --help-syntax can start to become...less helpful. In the argParse way, you could copy more code/behavior from cligen/argcvt.argAggSplit to keep your custom syntax pretty similar to what --help-syntax describes. With the getSplitComma approach just mentioning CSV in help["symOverrides"] is probably enough to guide CLI users (e.g., "specified ,-separated symbols"). With the custom argParse solution you can also switch-off ArgcvtParams.parNm as my test/FancyRepeats.nim does, to say handle defines differently from symOverride.