CLI11 icon indicating copy to clipboard operation
CLI11 copied to clipboard

Include order for custom converters

Open jbriales opened this issue 4 years ago • 8 comments

I was trying the support for custom converters described in https://cliutils.gitlab.io/CLI11Tutorial/chapters/advanced-topics.html#custom-converters

I was curious about the constraint to set up the custom istringstream& operator << overload before including CLI11.

Could you provide any pointers as to what's the concrete C++ limitation that doesn't allow us to be more flexible in this sense?

In my experience constraining the ordering of includes is quite tricky and a source of headaches down the road. I guess you may have a very good reason why this cannot be avoided :)

jbriales avatar Nov 24 '19 04:11 jbriales

That example seems a bit out of date, the whole book probably needs some updating.

As far as I know In previous versions of CLI11 there was components that used overloads in which case the entire overload set would likely needed to have been defined before including CLI11.

In recent versions it is based much more on Templates so the overload is required to be defined before the template is instantiated which is a much looser requirement.

Most of the cases in which I am using a custom converter I am creating a template specialization of lexical_cast. which I know I don't need to define before including CLI11.

phlptp avatar Nov 24 '19 14:11 phlptp

Oh that sounds great! Ideally we could update the documentation a bit in that sense then. Meanwhile, could you maybe just share a quick toy example on specializing lexical_cast in here that serves as a reference? :)

jbriales avatar Nov 24 '19 20:11 jbriales

I haven't updated the book too much but am starting to look at it a little more. The main README is kept up to date pretty well.

here is an example of one of our use cases with a custom converter to a time object. The whole file is just a wrapper to set up some standardized usage around the CLI11 App for our application since we use it in quite a few places.

The NewParseTest has some other examples of overloading lexical cast.

Adding an example that actually used the istream overload might not be a bad idea at some point. I think most of the previous examples for that with optional and enums have been rendered redundant with better typing and support in the included templates.

phlptp avatar Nov 24 '19 23:11 phlptp

Thanks for the pointers Philip! I think it makes more sense to me now. I noticed the README is often a better reference, but in practice when I'm Googling some keywords to see if CLI11 can do it, I get entries to the book, and the README is pretty long and it's hard to locate concrete sections in there unless you know exactly what you're looking for (in this case, lexical_cast in https://github.com/CLIUtils/CLI11#how-it-works).

jbriales avatar Nov 25 '19 01:11 jbriales

Follow-up: I was trying the lexical_cast/to_string/type_name approach and I saw it's tricky too. It seems it needs to include CLI11 headers before the definition of new specializations. Also it quickly runs into template function overload/specialization issues (quite tricky template-stuff) and I'm not even sure the include order constraint I mentioned in the beginning is fixed. Overall I think overloading the istream operator>> and operator<< are going to be simpler in the long run :)

jbriales avatar Nov 25 '19 07:11 jbriales

I think now that the book is part of the main repo it might be a little easier to keep that updated, but it does have some needed work to get it to that point.

phlptp avatar Nov 25 '19 23:11 phlptp

I take it this issue now is "Update the book"? Or can it be closed?

henryiii avatar Dec 31 '19 16:12 henryiii

Book definitely needs some updates. I think it would be good to see if there are some issues with the include order on different compilers as well just to make sure.

phlptp avatar Dec 31 '19 17:12 phlptp