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

Add controlslist attribute #56

Merged
merged 8 commits into from
Feb 5, 2025
Merged

Add controlslist attribute #56

merged 8 commits into from
Feb 5, 2025

Conversation

nkr0
Copy link
Contributor

@nkr0 nkr0 commented Jan 22, 2025

@@ -5,6 +5,7 @@ video
:alt: You cannot display videos
:autoplay:
:nocontrols:
:controlslist: nodownload nofullscreen noremoteplayback
Copy link
Member

Choose a reason for hiding this comment

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

Usually, Sphinx list parameter are set as comma separated values in Sphinx directives, can you adpt your code to fit this unwritten and undocumented standard ? 😄

nkr0 and others added 3 commits January 28, 2025 07:21
Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>
Copy link
Member

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

@nkr0 you now need to remove the bad value example from the documentation as it's raising an error. And catch it in the tests as it will raise an error as well.

@nkr0
Copy link
Contributor Author

nkr0 commented Jan 29, 2025

@nkr0 you now need to remove the bad value example from the documentation as it's raising an error. And catch it in the tests as it will raise an error as well.

It feels a bit odd that an error in one rst file can cause all the other tests in that directory to fail as well. Seems like sphinx reads every file in a directory even after changing from build_all to build_specific.

@12rambau 12rambau merged commit 15c8224 into sphinx-contrib:master Feb 5, 2025
1 check passed
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.

2 participants