Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
Resolves: #2748
Proposed Changes
clapcurrently makes it very difficult to overwrite cli arguments. It might be more feasible withclap v3.0.0which is currently in beta, because that introduces the methodApp::mut_arg, which we might be able to use to update default argument values at runtime. AndApp::get_matches_mut(), which allows you to get argument matches without consumingApp.So rather than using native
clapfunctionality, this PR creates a wrapper struct aroundArgMatches, with fallback logic to check a given config file for missing args (as @paulhauner suggested) . The glaring weakness of this though, is that we lose out onclap's validation for any arguments we get from file. So far I've found that our error handling is covering a lot of this validation well anyways, but it does fail with less information thatclapwould. And I haven't yet tested out all of lighthouse's flags and options.Another potential option is to move towards a declarative style like
structopt, which is being rolled intoclap_deriveinclap v3.0.0(clap-rs/clap#1661). This style does make it simpler to update args at runtime.