Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limitations of cxxopts in F3D #434

Closed
Tracked by #1243
mwestphal opened this issue Sep 25, 2022 · 2 comments
Closed
Tracked by #1243

Limitations of cxxopts in F3D #434

mwestphal opened this issue Sep 25, 2022 · 2 comments
Labels
type:bug Something isn't working
Milestone

Comments

@mwestphal
Copy link
Contributor

mwestphal commented Sep 25, 2022

F3D uses cxxopts for parsing options, however there is some limitations that negativela impacts F3D, it has to do with how cxxopts handles vectors.

F3D is using actual value of the option as default value, parsing it to a string before providing this string to cxxopts as a "default value".

The main issue is that an empty string is not parsed as an empty vector by cxxopts. It forces F3D to provide default value to some options when they should not have any.

A secondary issue is that the value stored in the vector is not replaced by cxxopts but pushed back into it, which means that before parsing, the value needs to be reseted.

The way we use cxxopts and these two cxxopts limitations forces us to define "default values" for values that should not have any (--camera-* and --range). Not a very big deal as this can be handled internally however they do appear when using --help which is not great.

That being said, this is not a critical issue, I'm opening this as a documentation of the current behavior if we ever want to fix it.

A few possible fixes are:

  1. Convince upstream / fork so that vector are handled differently and parsing an empty string for a vector is supported. The current implementation actually make sense and simplify many things in cxxopts code, so fork may be needed.

  2. Rework complettely our option system to stored default value in string from somewhere else. This is possible however it means that libf3d option default and these new string defautl will be duplicated/synchronised

  3. Use another option parsing lib

1. is of course the simplest but 3. may be what is needed in the long run.

@mwestphal mwestphal added the type:bug Something isn't working label Sep 25, 2022
@mwestphal
Copy link
Contributor Author

BTW, 1. is not easy if using std::vectorstd::string

@mwestphal
Copy link
Contributor Author

Fixed by #1580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

1 participant