Skip to content

Add admonitions #3501

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.

Sign up for GitHub

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

Jump to bottom
Merged
harshil21 merged 78 commits into from
Jan 22, 2023
Merged

Add admonitions #3501

harshil21 merged 78 commits into from
Jan 22, 2023

Conversation

@lemontree210
Copy link
Member

@lemontree210 lemontree210 commented Jan 13, 2023 •
edited
Loading

addresses #3496

  1. "Returned in" admonitions are generated automatically
  2. "Available in" admonitions are generated automatically
  3. "Use in" admonitions are generated automatically
  4. ..seealso notes were separated and included manually because they contained both shortcuts and links to Wiki

harshil21
harshil21 reviewed Jan 13, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

Nice job. The docs as artifact is already helping.

Can we also make a new icon for this? I have two in mind:

  1. Speedometer - https://icons.getbootstrap.com/icons/speedometer/
  2. Signpost - https://icons.getbootstrap.com/icons/signpost/

@lemontree210
Copy link
Member Author

lemontree210 commented Jan 13, 2023 

2. Signpost - https://icons.getbootstrap.com/icons/signpost/

Done (signpost)

harshil21
harshil21 approved these changes Jan 13, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

I went through the docs and they look really nice on there. Hopefully more people will start using them

@harshil21 harshil21 added enhancement ⚙️ documentation affected functionality: documentation labels Jan 13, 2023
@harshil21 harshil21 added this to the v20.1 milestone Jan 13, 2023
@lemontree210 lemontree210 changed the title replace seealsos with shortcut admonitions Add admonitions Jan 14, 2023
@lemontree210 lemontree210 linked an issue Jan 14, 2023 that may be closed by this pull request
[DOCUMENTATION] Add new admonitions to telegram classes #3496
Closed
Bibo-Joshi
Bibo-Joshi requested changes Jan 14, 2023
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Nice work indeed! 🚀 I left a few smaller comments below.

I would like it very much if we could make the bullet point list collapsed by default if there's more than, say, 5 items. for Message, the list is really rather long … I'd be okay if that's done in a follow-up PR.

@lemontree210

This comment was marked as resolved.

Sign in to view
@harshil21

This comment was marked as resolved.

Sign in to view
@lemontree210

This comment was marked as resolved.

Sign in to view
@harshil21

This comment was marked as resolved.

Sign in to view
@lemontree210

This comment was marked as resolved.

Sign in to view
harshil21
harshil21 reviewed Jan 14, 2023
View reviewed changes
harshil21
harshil21 reviewed Jan 14, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

Amazing change, the docs somehow just keep getting better.

@lemontree210

This comment was marked as resolved.

Sign in to view
Bibo-Joshi
Bibo-Joshi reviewed Jan 15, 2023
View reviewed changes
@lemontree210

This comment was marked as resolved.

Sign in to view
@lemontree210

This comment was marked as resolved.

Sign in to view
harshil21
harshil21 reviewed Jan 21, 2023
View reviewed changes
@lemontree210
refactor: simplify building of class name
17ba721
@lemontree210
generate all admonitions for all subclasses
220d1b8
@lemontree210
refactor: move AdmonitionInserter to separate module
89863f7
@lemontree210
add note on needing to have Python 3.9+ for building docs
444657c
@lemontree210
remove text in brackets for args recognized as str
8fa82ee 
this currently affects nothing but is worth including
@lemontree210
fix: handle custom generics correctly
cce51de 
`Bot` had a "Returned in" admonition linking
to `ApplicationBuilder` because of a custom generic
@lemontree210
refactor: simplify create_returned_in()
d5d32a8 
there is no need to call `typing.get_args()`
and check its return because `self._resolve_arg()`
has recursive calls anyway.
@lemontree210
fix: do not generate admonitions on Appbuilder's doc page
5280037 
they only contain Appbuilder's own methods
harshil21
harshil21 reviewed Jan 21, 2023
View reviewed changes
@harshil21
Copy link
Member

harshil21 commented Jan 21, 2023 •
edited by lemontree210
Loading

Found some more missing links, I think this is the last of it:

Use-in:

  • telegram.ext.{Defaults, ContextTypes, {Base, Pickle, Dict}Persistence, {Base, AIO}RateLimiter-> telegram.ext.ApplicationBuilder

Available-in:

  • ChatMember subclasses do not have the Available in that ChatMember has, Returned in is copied correctly.

@lemontree210
refactor: use variable
4a16ebf
@lemontree210
fix typo: wrong position of closing parenthesis
46557a8 
that led to last `elif` being ignored and classes
whose args were typed as strings
were not added to AppBuilder's "Use in"
@lemontree210
add "Available in" admonitions to subclasses
374b64e
@lemontree210
refactor: use ._resolve_arg_and_add_link for "Available in" admonit…
191e45e 
…ions
@lemontree210
refactor: remove unused types and origins from ._resolve_arg()
01242c2
@lemontree210
add try.. except.. with informative error messages, remove continue
3a4de3c 
* catch `NotImplementedError` propagated from
`._resolve_arg()` and show an informative
error message about which method/attr in which
class triggered the admonition generation error.
* one stray `continue` got left behind
harshil21
harshil21 approved these changes Jan 21, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

Phew, I think I'm happy with all the changes. This feature makes it almost impossible to get lost in the docs!

@harshil21 harshil21 merged commit f36ceaa into doc-fixes Jan 22, 2023
@harshil21 harshil21 deleted the shortcut-admonitions-docfixes-3496 branch January 22, 2023 11:40
@harshil21 harshil21 mentioned this pull request Jan 22, 2023
Merged
9 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2023
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Bibo-Joshi Bibo-Joshi Bibo-Joshi requested changes

@harshil21 harshil21 harshil21 approved these changes

Assignees

@lemontree210 lemontree210

Labels

⚙️ documentation affected functionality: documentation 🔌 enhancement pr description: enhancement

Projects

None yet

Milestone

v20.1

Development

Successfully merging this pull request may close these issues.

[DOCUMENTATION] Add new admonitions to telegram classes

4 participants

@lemontree210 @harshil21 @Bibo-Joshi