-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Allow to override html_context values #2971
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
Comments
Does this work locally for you? If not, this is an issue with our theme, otherwise, an issue with an override we are applying here. |
Yes, it works locally. |
Do you have any news? Thanks! We have related report platformio/platformio-docs#19 (comment) by @giogziro95 |
The issue is still valid, it works locally with sphinx + sphinx_rtd_theme but not on rtd.org. Example: https://github.com/godotengine/godot-docs/blob/3.0/conf.py#L69-L76
|
@agjohnson could you fix that? Thanks! |
RTD overrides the I think some values can't be allowed to be overridden by the user, but some values like make sense to allow the user do it. Probably this could be solved having a black list of settings that the user can't override. Something like required_context = {
'using_theme': using_rtd_theme,
'html_theme': html_theme,
'MEDIA_URL': "{{ settings.MEDIA_URL }}",
'PRODUCTION_DOMAIN': "{{ settings.PRODUCTION_DOMAIN }}",
...
}
context = {
'github_user': '{{ github_user }}',
'github_repo': '{{ github_repo }}',
'github_version': '{{ github_version }}',
'display_github': {{ display_github }},
...
}
if 'html_context' in globals():
context.update(html_context)
context.update(required_context)
html_context = context |
We are trying to build a new context for similar issues at #3490 . Would you like to take a look at and give us some feedback? In fact, I added this note https://github.com/rtfd/readthedocs.org/pull/3490/files#diff-bcc75ef2283eeb5c14031ea7d530d5a8R155 exactly for the same reason that this issue raised here. So, if we want to allow changing this behaviour we are still on time, I guess :) |
I've had a quick look over the new documentation, but indeed as long as this stands, we can't customize much:
Wouldn't it be better for the Read the Docs context to be prepended to the user-defined |
We have a flag that allows projects to override the html_context from rtd (we need to setup this on the db) I also think that rtd should allow to override this values by default, not sure how else we can fix this issue. |
We use a temporary solution with replacing links via DOM |
I think this issue is solved by the feature flag Please, let me know if you want to enable this feature flag in any of your projects and I will do it for your. If there is anything actionable in this issue, I will close it soon. |
@humitos I'm still not sure why we override by default. Feature flag feels like the default behavior for me |
It's legacy. I suppose they should not avoid other people to override RTD internals here. I'd 👍 on changing this behavior if it does not break projects --which is hard to know. |
This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further. Thanks! |
I still think this feature flag should be the default behaviour, or at least something users can enable themselves in their conf.py or RTD dashboard... Still, in the meantime, I'd be glad if it could be enabled for |
I think it will only break projects that are already broken locally, I mean building outside rtd. |
@akien-mga I've enabled this in |
:) Oh... Why did a bot close this issue? :) Could someone summarize what should be done on our https://docs.platformio.org side? |
@ivankravets it was closed automatically because it didn't have any activity. If you need to override |
Aha, got you. Please activate for our project too. Currently, we overwrite "Edit page" with JS hook. Thanks! |
@ivankravets done! |
Thanks! It works now! |
We are keeping this issue open till we came with a final decision if enabling the flag by default doesn't break anything :) |
The original issue is solved. The discussion if enabling it by default or not is going to happen in another place since we need more data to make it. There is no need to keep this issue open here until then. There are not too many users with this problem and we have a solution (feature flag) for the ones that need it (just need to ask for it). I'm closing it. |
Details
Expected Result
@platformio documentation is located in separate repository which is "mounted" to core repository.
We override "html_context" with own settings for Github, see conf.py.
Also, see build log and
cat docs/conf.py
.So, the "Edit" links still follow to "platformio-core" repository instead of "platformio-docs".
Actual Result
"Edit" links should follow to "platformio-docs" repository.
The text was updated successfully, but these errors were encountered: