Hard to read custom options

[AbcAeffchen]

Hi,

I current experiment a little with CLI11 and tried to read a std::pair<int, float> as a custom option.
All works fine. I used set_custom_option("INT FLOAT", 2), to enforce that there are exactly two arguments and to reflect the types I expect. But the help text that is generated looks weird.

-p INT FLOAT x 2      Some text

It gets even worst if I also print default values (depends a little on how you stringify a pair).

-p INT FLOAT=(1, 0.00000) x 2      Some text

I looked a while into the documentation and what methods are provided, but I couldn't find a solution to this.

So I took a quick look into the code and I added a quick fix for this. See the commit 1561a1a in my fork. If you are interested I can send a Pull Request.

But please let me know if I just didn't saw the right options.

Best,
Alex

[henryiii]

I like the idea, but do you think the following solution might be better than adding a flag that hides the number print:

• Options would gain a new property "size" that tells CLI11 how "large" one option is. Complex would be 2, most options would be 1, flags would be 0. Size would be passed through set_custom_option in instead of expected.
• The number of options expected would then become 1 in your case (1 * 2 = two arguments parsed), so the x 2 would go away. If you asked for 2, then the x 2 would be printed and 4 options would be expected.

What do you think? Would that be general enough?

[AbcAeffchen]

I think this is general enough for my use case. At the moment I think it covers all possibilities for a custom input type. If it gets much more complex one should split it up in multiple options.
So for me it is fine.

Will you implement this or should I look at this? I'm not sure how much code needs to be changed for that. You mentioned the complex type.

[henryiii]

I don't think it would be that hard - very similar to what you've done already. A new size_t would be added, maybe "option_size_", instead of the bool you originally added. The loop to collect options would be option_size_*expected_. I'll be a while before I can try to implement it, so feel free to have a go at it! I'll help if you have questions or problems.

[AbcAeffchen]

expected_ is also involved in some checking I think.
If I change set_custom_option such that it stores

expected_ = expected < 0 ? -1 : expected * size

everything should work as expected. Then only the help function needs to be changed. This also works for the description of excpected_. Just the meaning of the expected argument of set_custom_option needs to be adjusted to something like "The number of arguments" while the size is "The number of inputs per argument"? I'm not sure I use the right words to distinguish between options, input and parts of input that belong to an resulting object.

[henryiii]

I would store both values. I would not allow set_custom_option to set expected_, and I would make size_ take over most of the old duties of expected. You can git grep expexted_ to see where it is used. The point of having expected set by a special custom options method in the first place was to allow 0 to be set on flags. That quirk will be fixed by having a new size property.

I think separating the two concepts (number of options vs number of items in an option) will make things simpler.

[henryiii]

Since it does show up in more places than I thought, I could start the patch, then you could fix the printing output? Would that work?

[AbcAeffchen]

Sorry for answering so late. Currently I'm very busy so I have no time to make big changes.

What I wanted to say in the last post was:
The old expected_ looks like it is the new expected_ times the new size_. So we store size in size_ and expected * size in expected_ and most things should work. I think there is only one thing that needs more changes: If you read a arbitrary long input of custom objects, then you need to make sure that the input is a multiple of the size.

I have not enough time at the moment to do the big changes, but I can fix the output after you finished the patch. Or I try to implement it but this can take a while.

[henryiii]

It's more involved than I thought, but ultimately a good improvement, allowing some things I'm not fond of, like "unchangable", to be dropped. I've started a branch and will work on it occasionally. You are welcome to contribute if you see something that needs fixing (I'm giving you write access to my branch). I'll be amending the original comment for now, though, to keep the history clean.

[AbcAeffchen]

OK. Thanks for the write access. I will try to do something this week, but I cannot promise.

[henryiii]

Okay, I'm not sure how fast I'll be able to get anything done. My goal is to at least get the refactor working and tests passing, then maybe you can touch up the help printing?

[AbcAeffchen]

Sounds good.

[henryiii]

Added a simple test for what I assume you are doing, hope that's good enough...

[AbcAeffchen]

I'm really sorry that was absent the last two weeks.

This looks perfectly fine. Thanks a lot.