-
Notifications
You must be signed in to change notification settings - Fork 307
gz-sim-model is broken with 2.2.0 <= CLI11 Version < 2.5.0 (gz-sim abusing of option and flags) #2918
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
Comments
gz-sim-model also works properly with the cli11 version vendored in gz-utils, version 1.9.1.
|
temporarily restoring cli11 vendoring in gazebosim/gz-utils#178 |
I bisected the failure to CLIUtils/CLI11@f7d26f2, which was first released in CLI v2.2.0 |
I bisected the bug fix to CLIUtils/CLI11@f760095 |
I've isolated the fix to the change to include/CLI/impl/Option_inl.hpp in CLIUtils/CLI11@f760095 |
workaround ready for review in #2921, in conjunction with gazebosim/gz-utils#179 |
workaround merged in #2921 |
Coming from #2910 (comment).
When using CLI11 on Ubuntu Noble (version 2.4.2) the system is not able to handle our CLI options groups properly so the
--list
option always requires--model
option.I think that the root of the problem is that we are abusing of "options" and "flags" in CLI11 where a more natural approach would be to use "subcommand". The problem is that subcommands in CLI11 do not accept dashes so we can not keep our current parameters.
A subcommand possible implementation for gz-sim-model with two subcommands (list and model) would be:
https://gist.github.com/j-rivero/b4e332833440a85b463f66f9ee87524f
There is a breakage in the code when mixing options and flags and make one of the flags to require one of the options. This is the case now with the option "--model" that is being required by "--pose" so when trying to use the CLI executable without parameters or with "--list" the result is:
"--pose requires --model".
A quick workaround would be to stop making the flag to require the option and improve the description to reflect that.
Seems like CLI11 2.5.0 is able to deal with the current scenario.
The text was updated successfully, but these errors were encountered: