-
Notifications
You must be signed in to change notification settings - Fork 392
App::_process: add conditional logic for old vs. new help processing #1204
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
base: main
Are you sure you want to change the base?
Conversation
Some use cases require altering subcommand/option states (and thus the help output) based on a flag stored in the config file. In this case, running `_process_callbacks()` before `_process_help_flags()` is necessary. A new compile-time option `CLI11_USE_OLD_HELP_PROCESSING` restores this “old” behavior by executing callbacks prior to help flag processing. Extends CLIUtils#1186
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1204 +/- ##
===========================================
- Coverage 100.00% 99.98% -0.02%
===========================================
Files 17 19 +2
Lines 4546 5211 +665
Branches 0 1066 +1066
===========================================
+ Hits 4546 5210 +664
- Misses 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
should this be configurable at runtime vs the compile time switch |
That’s a very good question. If the old help procedure is needed, I’d assume it applies to the entire project. So it probably wouldn’t make much sense to expose this as a simple runtime flag on individual commands. What do you think? |
I don't think the formatter could play a role in this, as that doesn't come into play until after the help Exception is triggered. The question is in _process() method in the app _process_help_flags();
try {
// the config file might generate a FileError but that should not be processed until later in the process
// to allow for help, version and other errors to generate first.
_process_config_file();
// process env shouldn't throw but no reason to process it if config generated an error
_process_env();
} catch(const CLI::FileError &) {
// callbacks can generate exceptions which should take priority
// over the config file error if one exists.
_process_callbacks();
throw;
}
_process_callbacks();
_process_requirements(); The current ordering came about through a variety of issues and conditions and I think I am happy with it as a default. But other than config and env processing needing to be before callbacks and requirements any other ordering is possible. help, callbacks, requirements All will have slightly different behavior in the mixed cases, and determine if you want to prioritize help, or other errors. And also as a look closer there is going to be some deviations in behavior in the ordering for processing subcommands and help and requirements checking, that should be made consistent at some point. |
@VolkerChristian are you going to do anything further with this in the very near future. I am looking to get a release out soon, and would like to know if I should plan on this being in a new release or should wait until after that? |
Oh, I see! I thought a lot about this patch and how to cleanly add all of the mentioned six combinations and make that configurable. I didn't find a elegant and clean solution. As I am obviously the only one who suffers from this new processing order I decided that I stay with my local patched version. Thus, this pull request can be closed for now. Nevertheless, I prepared two further pull request which would be fine to find their way into the next release. |
@VolkerChristian I am trying out an alternative to this with fine grained control on the options to determine when the option callbacks execute with respect to help, and requirements processing. I think that addresses the issue you raised in this PR and the issue in #1221 |
Some use cases require altering subcommand/option states (and thus the help output) based on a flag stored in the config file. In this case, running
_process_callbacks()
before_process_help_flags()
is necessary.A new compile-time option
CLI11_USE_OLD_HELP_PROCESSING
restores this “old” behavior by executing callbacks prior to help flag processing.Extends #1186