-
Notifications
You must be signed in to change notification settings - Fork 25
Allow usage of global configuration values for TryExamples
directive if provided by user
#161
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
Allow usage of global configuration values for TryExamples
directive if provided by user
#161
Conversation
Happy to add a CHANGELOG entry for this if needed! |
Oops, let me run |
No need! The change log gets autogenerated by the prep release GitHub workflow. |
b12996d
to
244de38
Compare
Thanks @agriyakhetarpal! Looking good so far. One thing: I think that all of the global config options should be propagated if a custom option isn't given, not just |
Right, I had thought of doing I agree about |
I think making a breaking change here is fine. jupyterlite-sphinx/docs/directives/try_examples.md Lines 3 to 4 in a0903f5
It would be surprising to me if some global config options were respected for manually inserted directives but not others. Since we're working with an experimental directive, we can make breaking changes to ensure consistent behavior. I think the default for global warning text is that there should be none. If someone sets a global warning text, and wants to override it in a few places, then I don't think it's a big deal for them to have to manually set the warning text to |
Fair point, didn't consider that the directive was/is still experimental – I have added in the requested changes! Now, one should be able to use the global warning text and use the global name for the theme/CSS container styling. |
try_examples_global_button_text
if provided for TryExamples
directiveTryExamples
directive if provided by user
Co-Authored-By: Albert Steppi <[email protected]>
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.
Thanks @agriyakhetarpal. This is looking better. A few minor suggestions.
Do not include a warning text by default, but if a value exists in conf.py, use it. If a directive has its own warning text, then override the global value. Co-Authored-By: Albert Steppi <[email protected]>
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.
Thanks @agriyakhetarpal. this is in good shape now. Just a couple minor suggestions on language.
Co-authored-by: Albert Steppi <[email protected]>
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.
Thanks @agriyakhetarpal! This looks good now.
Description
This PR updates the
TryExamplesDirective
class to use the configuration value inconf.py
if specified, and otherwise use the "Try it with JupyterLite" text. This way, buttons that are created manually with the.. try_examples::
directive in Sphinx will receive the same configuration value, and adding a
:button_text
input across several.. try_examples::
directives to customise the button text will now not be required. Thetry_examples_global_button_text
string will be used by default, instead of the current"Try it with JupyterLite!"
text.To explain this behaviour, an additional sentence has been added to the documentation for the
TryExamples
directive page.Closes #157
Footnotes
This is essentially a breaking change of sorts for dependents who are not using a custom
:button_text
input already, since it will now change the text to what they have set intry_examples_global_button_text
(in case they have set that). But it's surely not a very big change and I would consider it as more of an enhancement (cc: @steppi)