Closed yudai-nkt closed 3 years ago
I'm afraid I cannot figure out how to fix mypy failure...
First of all thank you for your on this PR! I as a user of this project would also highly appreciate table style options. After seeing your PR I looked into the mypy issue in effort to be helpful, but sadly as of now I could also not find a solution.
In the process howerever I took a closer look at clicks usage parsing code. As a result I have a few notes in regards to implementation, but feel free to ignore this.
It seems click uses get_help_record(ctx)
(linked code is the code for 7.1.2). for its on parsing. This function does most of the custom parsing for types you are implementing for this PR already. From what I can see it returns Tupel('[-<short option>,] --<long option> <type>', '<help>')
which might be why you decided to parse the information yourself.
For example for your test cases it would return:
('-d, --debug TEXT', 'Include debug output')
('--choice [foo|bar]', 'The given help text')
('--date [%Y-%m-%d]', 'The given help text')
For float range 7.* sadly does not have support for displaying the range yet, but the current alpha version of 8.0 supports this already:
('--range-a FLOAT RANGE', 'The given help text')
('--range-a FLOAT RANGE', 'The given help text [0<=x<=1]')
Note how in contrast to choice and date the range is part of the help text and not the command + type part of the returned tuple. Also there is no indication of if clam
is true or false.
Also something to note about the upcoming 8.0 release is that it will introduce ctx.to_info_dict()
for click.Context
giving developers direct access to everything so custom documentation will be easier etc. (https://github.com/pallets/click/issues/461) Below you will find an example dict generated using the context of your test case.
Now my suggestion for the table feature is two fold:
get_help_record(ctx)
. To get formatted option types. This will be compatible with 7. and also with the future release of 8.. If you wish to support meaningful type doc for things not supported by 7.* (such as FloatRange etc.) you could do custom formatting only for these types while leaving click to handle all other types (e.g., choice, datetime).**edit: note about custom support for FloatRanges
Context Dict:
@yudai-nkt OK I have now found the reason and a fix for the mypy errors
The problem seems to stem from the fact that the typeshed for click types (third party type hints) is missing the type hints for the attributes.
e.g., FloatRange is currently defined as
class FloatRange(FloatParamType):
def __init__(self, min: Optional[float] = ..., max: Optional[float] = ..., clamp: bool = ...) -> None: ...
but should be
class FloatRange(FloatParamType):
min: Optional[float]
max: Optional[float]
clamp: bool
def __init__(self, min: Optional[float] = ..., max: Optional[float] = ..., clamp: bool = ...) -> None: ...
Now this means to ultimately fix the problem one would have to make a PR for typeshed and add the missing attribute type hints in third_party/2and3/click/types.pyi
(ideally for all click types and not only the 4 that are used in this PR). Now since this is not an immediate fix (with mypy having to release with the typeshed changes for it to work) the only other option I see is to ignore the mypy errors on this for now.
I don't have much time now (it's 2:00 am here and I was about go to bed haha), but I'd like to answer to some of your comment which I can react to right now.
It seems click uses
get_help_record(ctx)
(linked code is the code for 7.1.2). for its on parsing. This function does most of the custom parsing for types you are implementing for this PR already. From what I can see it returnsTupel('[-<short option>,] --<long option> <type>', '<help>')
which might be why you decided to parse the information yourself.For example for your test cases it would return:
('-d, --debug TEXT', 'Include debug output')
('--choice [foo|bar]', 'The given help text')
('--date [%Y-%m-%d]', 'The given help text')
I actually once used get_help_record()
in yudai-nkt/mkdocs-click@6f89573, but it wasn't handy for me because it mixes option names and type annotation into a single element. Another disadvantage is the lack of whitespaces around |
delimiters in choice and datetime. This prevents the Type column from line-breaking and might result in a bad table appearance.
For float range 7.* sadly does not have support for displaying the range yet, but the current alpha version of 8.0 supports this already:
- click==7.*:
('--range-a FLOAT RANGE', 'The given help text')
- click==8.*:
('--range-a FLOAT RANGE', 'The given help text [0<=x<=1]')
Range support in v8 seems good, but I like being verbal (i.e., between, above, and below) rather than using mathematical expression given that HTML documentation has more room than terminal emulator. It's just a matter of preference of course.
Thank you for your suggestions and information anyway, @max-frank! I'll read it thoroughly later and consider which direction I should go (pallets/click#461 especially looks quite promising).
I actually once used get_help_record() in yudai-nkt/mkdocs-click@6f89573, but it wasn't handy for me because it mixes option names and type annotation into a single element. Another disadvantage is the lack of whitespaces around | delimiters in choice and datetime. This prevents the Type column from line-breaking and might result in a bad table appearance.
I was suspecting this was the case. I should have also looked at your old PR sorry.
Range support in v8 seems good, but I like being verbal (i.e., between, above, and below) rather than using mathematical expression given that HTML documentation has more room than terminal emulator. It's just a matter of preference of course.
Indeed its just a matter of preference one thing to note though starting with v8 FloatRange and IntRange will support configuring if the range is inclusive or exclusive for both ends. Just something to keep in mind so that the code will be easily adaptable for future 8.* support.
お疲れ様
@max-frank Great one on submitting a PR to typeshed
to fix the typing errors.
@yudai-nkt For now we can just # type: ignore
attribute accesses such as param_type.min
, and perhaps add a link to https://github.com/python/typeshed/pull/4813 to serve as a TODO.
@florimondmanca I updated the branch according to your comments.
@max-frank After some thought, I'd like to keep the initial direction but will try the v8 features you introduced me after the stable version is released. Then I'll submit a PR if it turns out to be worth it.
Rewrite of #12 in a backward-compatible manner.
One difference from what @florimondmanca and I discussed is that I named the option
:option-style:
. This is more descriptive thanstyle
and easy to extend when we want to define styles for other sections such as usage.Fix #11.