Skip to content
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

RFC Choice: expand Choice token normalization + remove str requirement #2796

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AndreasBackx
Copy link
Collaborator

@AndreasBackx AndreasBackx commented Nov 2, 2024

When looking into #2210, I wasn't sure it was a good idea to show an enum's name to the user. For example:

class MyEnum(enum.Enum):
	ONE = 1
	TWO = 2
	THREE = 3
	ALSO_ONE = 1

Would result the following if an invalid value is shown:

Error: Invalid value for '{ONE|TWO|THREE|ALSO_ONE}': 'meh' is not one of ONE, TWO, THREE.

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 of ALSO_ONE or also_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.

@click.option(
	"--method",
	type=click.Choice(
		["SCREAMING_SNAKE_CASE", "snake_case", "PascalCase", "kebab-case"]
	)
)

Would in the background casefold the values to screaming-snake-case, snake-case, pascalcase, and kebab-case. Though still show the following message:

Error: Invalid value for '{SCREAMING_SNAKE_CASE,snake_case,pascalcase,kebab-case}':
'meh' is not one of SCREAMING_SNAKE_CASE, snake_case, pascalcase, kebab-case.

This makes token normalization expand to cover normalization output to the user so it would now be:

Error: Invalid value for '{screaming_snake_case,snake_case,pascalcase,kebab-case}':
'meh' is not one of screaming_snake_case, snake_case, pascalcase, kebab-case.

Note that screaming_snake_case and snake_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 into snake-case.


In addition to this normalization, I'm introducing generics into Choice and removing the requirement to have choices be str. A funny side-effect is that enums can be supported more easily. Right now you'd have to use click.Choice(list(MyEnum)) though that would result in enum aliases not showing up. We could allow one to do click.Choice(MyEnum) and handle the aliasing for them. 🤔 We could prevent people from incorrectly using click.Choice(list(MyEnum)) by failing if we detect a list of Enum types.

Let me know what you think!

Resolves #605.

@AndreasBackx AndreasBackx changed the title RFC Choice: expand Choice token normalization RFC Choice: expand Choice token normalization + remove str requirement Nov 2, 2024
@AndreasBackx AndreasBackx mentioned this pull request Nov 2, 2024
6 tasks
@AndreasBackx AndreasBackx self-assigned this Nov 2, 2024
@AndreasBackx AndreasBackx added this to the 8.2.0 milestone Nov 2, 2024
@AndreasBackx AndreasBackx force-pushed the choices/normalization branch from ea2cffa to 3167037 Compare November 9, 2024 18:43
@AndreasBackx AndreasBackx marked this pull request as ready for review November 9, 2024 18:55
@AndreasBackx AndreasBackx requested a review from davidism November 9, 2024 19:03
@AndreasBackx AndreasBackx force-pushed the choices/normalization branch from 6a03b04 to f46d049 Compare December 3, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python Enum support for click.Choice
1 participant