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

[Draft] Use dicts to write configs #149

Merged

Conversation

dwreeves
Copy link
Collaborator

Main changes:

  • Add support for dicts to be passed into @rich_config(help_config=...).
    • The current behavior, which I think I'm going to keep (but need to document), is to merge the dict into the parent context's help config.
    • This still leaves a lot of questions unanswered about the config resolution but I am overall satisfied with this API decision, since the typical user won't notice a difference between merge and instantiating a new object, whereas users looking for more customization haven't had a nice way to access merging. And it just seems to make sense that it would do this behavior: you set a large RichHelpConfiguration() at the top, and each sub-command may have a tiny tweak. That makes sense!

Misc. changes:

  • Make typing more "Rich". At the end of the day the command class's type is Type[RichCommand], and this typing is covariant, so it cannot ever return a super-type. The should can be safely updated to return that.
  • Deprecate RichCommand.help_config. It didn't feel like this made sense back in 1.7, now it feels less like this makes sense, since rich help configs should be resolved in the context level (since they need to inherit from their parents).
  • Add stacklevel=2s to the deprecation warnings.

@dwreeves dwreeves changed the title Use dicts to write configs [Draft] Use dicts to write configs Dec 11, 2023
@dwreeves dwreeves force-pushed the add-dict-support-for-rich-config-decorator branch from fffc01a to 059e4f2 Compare December 11, 2023 04:40
@dwreeves
Copy link
Collaborator Author

I'm likely going to keep this open for a while. I want to get down a sensible and flexible system for resolving the config, aimed at:

  1. Zero difference to the vast majority of users who just want pretty outputs without much effort.
  2. Lots of convenience, sensibility, zero astonishment for users looking for lots of customization.

And I'd rather do too few changes and too few API guarantees than too many.

Also, see #134.

@dwreeves dwreeves force-pushed the add-dict-support-for-rich-config-decorator branch from c6e0390 to 30f3999 Compare December 11, 2023 04:49
@dwreeves dwreeves force-pushed the add-dict-support-for-rich-config-decorator branch from 30f3999 to 9c19935 Compare December 11, 2023 04:51
@dwreeves dwreeves force-pushed the add-dict-support-for-rich-config-decorator branch from 0e00e0b to 2054fbf Compare December 29, 2023 21:59
@dwreeves dwreeves force-pushed the add-dict-support-for-rich-config-decorator branch from d084fac to 0ab5aea Compare December 29, 2023 22:13
@dwreeves
Copy link
Collaborator Author

I think I am starting to settle on this configuration logic. I think it makes sense and is the least bad / least "astonishing" of all other alternatives. #134 (comment)

@dwreeves dwreeves merged commit 377504c into ewels:main Dec 29, 2023
32 checks passed
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.

1 participant