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

Change HelpTheme type to allow sub-classing #159

Closed
kdeldycke opened this issue Jul 7, 2023 · 3 comments · Fixed by #163
Closed

Change HelpTheme type to allow sub-classing #159

kdeldycke opened this issue Jul 7, 2023 · 3 comments · Fixed by #163
Labels
enhancement New feature or request

Comments

@kdeldycke
Copy link
Contributor

kdeldycke commented Jul 7, 2023

In Click Extra, I have a default theme lying around that try to extends cloup.HelpTheme with additional categories of styles. Like log-level colors and other help screen details.

The problem is that cloup.HelpTheme is a NamedTuple. And I cannot easily sub-class it. As you can see in my code, I had to redefine the whole properties and methods.

I also encountered issues with MyPy which cannot solve references to sub-styles:

click_extra/version.py:58: error: Cannot determine type of "default_theme"  [has-type]
        default_package_name_style: IStyle | None = default_theme.invoked_command
                                                    ^~~~~~~~~~~~~
click_extra/version.py:59: error: Cannot determine type of "default_theme"  [has-type]
        default_prog_name_style: IStyle | None = default_theme.invoked_command
                                                 ^~~~~~~~~~~~~

That is somewhat expected, as NamedTuple subclasses is flagged as wontfix by Python's typing project.

@janluke, would you be open to change the type of HelpTheme so I can reuse it seamlessly?

I naively think that a class is the natural choice, but maybe a @dataclass might be another alternative to NamedTuple. I'm pretty sure you have a better idea of what HelpTheme should look like in that case.

@kdeldycke kdeldycke added the enhancement New feature or request label Jul 7, 2023
kdeldycke added a commit to kdeldycke/click-extra that referenced this issue Jul 7, 2023
@janluke
Copy link
Owner

janluke commented Jul 7, 2023

Weird I used a NamedTuple for HelpTheme. Whatever was my reasoning, it was a bad idea. I'll have a look when I have time.

@janluke
Copy link
Owner

janluke commented Jul 13, 2023

I'll fix this next later today. I'll probably have to bump the major version for this.

@janluke janluke added breaking change This breaks backward compatibility and removed breaking change This breaks backward compatibility labels Jul 14, 2023
@kdeldycke
Copy link
Contributor Author

Thanks @janluke for this refactor! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants