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

refactor: Small fixes to type annotations #470

Merged
merged 1 commit into from Aug 20, 2022
Merged

Conversation

oprypin
Copy link
Member

@oprypin oprypin commented Aug 20, 2022

Also for event type annotations - if, hypothetically, MkDocs started to support them.

Also for event type annotations - if, hypothetically, MkDocs started to support them.
Copy link
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@pawamoy pawamoy merged commit 9214b74 into mkdocstrings:master Aug 20, 2022
role=heading.attrib["data-role"],
uri=f"{page}#{anchor}", # noqa: WPS441
)
if page:
Copy link
Member

@pawamoy pawamoy Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oprypin this condition prevents using cross-refs when rendering API docs in the top-level index page. Can you remember why you added it? I guess it was just for type correctness?

I tested without it, and the utilities computing relative URLs work fine when page == "" too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I'm referring to when page is "". But if it is None it won't work indeed. So maybe we should simply differentiate the two cases with if page is not None:.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah my bad, it definitely should be if page is not None. It was to satisfy type checking.

@oprypin oprypin deleted the anno branch July 18, 2023 16:28
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.

None yet

2 participants