RFC Choice: expand Choice
token normalization + remove str
requirement
#2796
+238
−71
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.
When looking into #2210, I wasn't sure it was a good idea to show an enum's name to the user. For example:
Would result the following if an invalid value is shown:
Though it's not uncommon practice to have kebab-case, imo it's the most convenient typing for the CLI. Then you'd ideally want the user to type
also-one
instead ofALSO_ONE
oralso_one
.I realised that we have token normalization since 2.0. I feel like this is not very obvious to users as we don't make the normalization transparent to the user.
Would in the background casefold the values to
screaming-snake-case
,snake-case
,pascalcase
, andkebab-case
. Though still show the following message:This makes token normalization expand to cover normalization output to the user so it would now be:
Note that
screaming_snake_case
andsnake_case
retain their underscores because we're casefolding, not converting to kebab-case. I think we should in the future make kebab-case the default, and/or allow extending the normalization behaviour more easily so this could be converted intosnake-case
.In addition to this normalization, I'm introducing generics into
Choice
and removing the requirement to have choices bestr
. A funny side-effect is that enums can be supported more easily. Right now you'd have to useclick.Choice(list(MyEnum))
though that would result in enum aliases not showing up. We could allow one to doclick.Choice(MyEnum)
and handle the aliasing for them. 🤔 We could prevent people from incorrectly usingclick.Choice(list(MyEnum))
by failing if we detect a list ofEnum
types.Let me know what you think!
Resolves #605.