-
Notifications
You must be signed in to change notification settings - Fork 17
Pass through all admonitions without enforcing "tip". #289
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
base: master
Are you sure you want to change the base?
Conversation
…e cases without title by itself.
Summary of ChangesHello @lrnv, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the processing of admonitions by removing a restrictive check that previously converted non-standard admonition types to a default "tip" category. The primary goal is to enhance flexibility and ensure that external rendering engines, such as VitePress, can correctly interpret and display a broader range of admonition styles without interference from the internal processing logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request removes the logic that was forcing unknown admonition types to be rendered as 'tip'. By removing this restriction, any admonition category can be passed through to VitePress, allowing it to handle rendering. This change correctly implements the intended feature and also removes what appears to be a bug in the old code where the admonition title was not being correctly updated. The change is clean and improves flexibility. I have no further suggestions.
|
I think we might need some css magic to catch unknown admonitions and make sure they render in a neutral way. But otherwise this looks good! Could also use a "test" i.e. putting this in the markdown page of the docs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #289 +/- ##
=========================================
+ Coverage 7.73% 7.77% +0.03%
=========================================
Files 6 6
Lines 892 888 -4
=========================================
Hits 69 69
+ Misses 823 819 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
We need examples in the docs, to see that this actually works. I don't think it does at the moment. |
|
I am doing experiments there : lrnv/Copulas.jl#304, but i have trouble adjusting the CSS. I am not sure this will work without the patch to the config.mts that i did (same pr), but this should at least "let things go to vitepress", and Vitepress might yell at you if it sees unknown containers (did not test that). If you want |
|
any updates here? |
|
The end of our previous discussion is unclear to me: what would be necessary for this to get merged / Which directions do you want to take ? |
| category = "tip" | ||
| end | ||
| title = admonition.title | ||
| if !(category ∈ ("tip", "warning", "danger", "caution")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still want the old categories right? and a default one, as is, but we want to render a new if a different name category is provided.
|
some examples in the docs, with steps on how to incorporate the new css for those. Don't know exactly what is your workflow for that to work. see (similar to): |
This PR simply allow admonitions that are not in the vitepress list to be passed through, and let vitepress handle them.
Should help with #95 and #288.