-
Notifications
You must be signed in to change notification settings - Fork 38
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] Fix typing and other bugs #126
Conversation
Some manual testing. Note to self: there is still one more case to handle: import rich_click as click
@click.group("foo")
@click.rich_config(help_config=click.RichHelpConfiguration(use_markdown=True, max_width=60))
@click.option("--x")
@click.pass_context
def foo(ctx: click.Context, x: str) -> None:
"""This uses **markdown**"""
pass
@foo.command
@click.pass_context
def bar(ctx: click.RichContext) -> None:
"""Hello, _world!_"""
foo() In this case, the
The good news? The typing works in strict mode! 😏
|
ec11aed
to
1a4affe
Compare
Yay, all tests passing! I am super excited about this update and we're close to the finish line. 😄😄😄 Last three steps, in no particular order:
|
I'm feeling comfortable merging this at this point. There just isn't a good way around the Mypy strict mode is working and turned on. Very excited about that. I'm feeling very exhausted and don't want to add a test case just yet for the parent context inheritance thing. I left it open as an issue: #127. OK, at this point I think I need to just do some real world integration testing. I may also want to consider tackling any other tiny open issues before releasing |
WOW. This was a lot of work. I am exhausted.
Long story short: I wanted to get the typing in
rich-click
to work with minimal hacking. I'm not 100% done (I think I may want this working in strict mode). But in the process, I ended up uncovering a lot of weird little issues:language: python
and had no third party dependencies. I swap over tolanguage: system
in the pre-commit and now everything is being actually tested for real.@command
,@command()
,@command("name")
etc. API was not being supported. See the test cases now fortest_rich_config_decorator_order
-- I have added every single way I can think of to turn a callbacks into a RichCommand (or subclass thereof, i.e. RichGroup). Some things I uncovered here:rich_click.command
andrich_click.group
decorators. This meant something likeclick.Group(...).command(cls=RichCommand)
did not actually use the rich context correctly! That realization is what motivated adding all thosepytest.param()
s i.e. ways of turning callbacks into RichCommands.__init__
forRichCommand
.help_config
even though the key that was being set elsewhere wasrich_help_config
. 😬--help
output!pass_context
because otherwise the type annotationctx: RichContext
does not work in a function signature, due to the fact that the typing is contravariant. I opened an issue in actual click about it here: Typing forclick.decorators.pass_context
is not covariant and does not supportclick.Context
subclasses pallets/click#2623 But my conclusion is they should not fix it / they should mark as "won't do" or something. I think it is better for us to just fix it on our end.from click import *
, the type annotations are not properly overridden according to mypy! I am not sure what that is about. I manually imported everything in click (notwithstanding anything being deprecated), and add added a__getattr__
to future proof.__init__.py
will require more work. It is not done! I need to revisit this and it's one of the bigger reasons why this PR is a "draft."MultiCommand
andBaseCommand
classes. 😬 So that recent addition is getting slightly rolled back.I'm definitely missing some notes. So many changes in this one! I am going to need to sit on this. Also, I don't expect as I submit this for the Click 7.x tests to pass; haven't gotten to that yet. But Click 8.x tests do pass.