fix(cli): crash when using --verbosity option
#1429
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves: #1428 When using `--verbosity` option, Nextclade panicked with error: ``` The application panicked (crashed). Message: Mismatch between definition and access of `verbosity`. Could not downcast to log::LevelFilter, need to downcast to alloc::string::String ``` (in release builds the message can be different due to type mangling) This is due to particularities of how `value_parser` works in `clap` v4, which is a replacement for `possible_values` functionality we used in `clap` v3, but which was removed in v4. In Nextclade the `verbosity` filed is of type `log::LevelFilter`. It seems `clap` v4 can read a list of string values passed to `value_parser` and displays them correctly in the `--help` text, but when trying to actually pass one of these values to the `--verbosity` arg, `clap` would panic with a downcast error somewhere deep inside the library. It's unclear to me why this is now suddenly needed, because `possible_values` worked just fine, but it seems that using an an iterator to the `clap`'s `PossibleValue`s constructed out of `LevelFilter` instead of strings work. I found this solution here: clap-rs/clap#4264. There are many more issues and discussions in clap repo about that. Maybe there's a better solution. At the same time, I could not resist to clean up our `verbosity.rs` file, because most of the stuff we copied from `clap-verbosity-flag` crate and never actually use. I also use clap's `default_value` parameter to set a default value for the `verbosity` now, rather than setting a default manually on runtime.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves: #1428
When using
--verbosityoption, Nextclade panicked with error:(in release builds the message can be different due to type mangling)
This is due to particularities of how
value_parserworks inclapv4, which is a replacement forpossible_valuesfunctionality we used inclapv3, but which was removed in v4.In Nextclade the
verbosityfiled is of typelog::LevelFilter. It seemsclapv4 can read a list of string values passed tovalue_parserand displays them correctly in the--helptext, but when trying to actually pass one of these values to the--verbosityarg,clapwould panic with a downcast error somewhere deep inside the library.It's unclear to me why this is now suddenly needed, because
possible_valuesworked just fine, but it seems that using an an iterator to theclap'sPossibleValues constructed out ofLevelFilterinstead of strings work. I found this solution here: clap-rs/clap#4264. There are many more issues and discussions in clap repo about that. Maybe there's a better solution.At the same time, I could not resist to clean up our
verbosity.rsfile, because most of the stuff we copied fromclap-verbosity-flagcrate and never actually use. I also use clap'sdefault_valueparameter to set a default value for theverbositynow, rather than setting a default manually on runtime.