Readthedocs.org: Allow to override html_context values

Created on 26 Jun 2017  路  24Comments  路  Source: readthedocs/readthedocs.org

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.

design decision

Most helpful comment

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 godot: https://readthedocs.org/projects/godot/

All 24 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 https://github.com/platformio/platformio-docs/pull/19#issuecomment-340908571 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
Resulting link still points to the 3.0 branch. This is quite annoying as our stable 3.0 branch and development master branch are still very similar, and there is no reason in our workflow for PRs to be made against the stable branch (but that's where all contributors are led by the "Edit on GitHub" link).

github_version is set here to remote_version: https://github.com/rtfd/readthedocs.org/blob/d4c21b95e6fcaf69e914aea6b5ed0e0cef1c29d8/readthedocs/doc_builder/backends/sphinx.py#L73

@agjohnson could you fix that? Thanks!

RTD overrides the html_context

https://github.com/rtfd/readthedocs.org/blob/e923c0cdf7547e63b4d2a9ab2b5e69b527bb482b/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl#L76-L133

I think some values can't be allowed to be overridden by the user, but some values like

https://github.com/platformio/platformio-docs/blob/58574c074149656f1cfc5290122251ea6a5344c6/conf.py#L281-L285

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 https://github.com/rtfd/readthedocs.org/pull/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:

Take into account that the Read the Docs context is injected after your definition of html_context so, it's not possible to override Read the Docs context values.

Wouldn't it be better for the Read the Docs context to be prepended to the user-defined html_context, to let users override it as they wish? If they break their RTD build with a bad custom context, it's their responsibility.

We have a flag that allows projects to override the html_context from rtd (we need to setup this on the db)

https://github.com/rtfd/readthedocs.org/blob/c37b00995c1bbc5ee51d3552ef176546373bb912/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl#L119-L125

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 DONT_OVERWRITE_SPHINX_CONTEXT (https://docs.readthedocs.io/en/latest/guides/feature-flags.html) and can be closed at this point.

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 :+1: 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 godot: https://readthedocs.org/projects/godot/

I'd +1 on changing this behavior if it does not break projects

I think it will only break projects that are already broken locally, I mean building outside rtd.

Still, in the meantime, I'd be glad if it could be enabled for godot: https://readthedocs.org/projects/godot/

@akien-mga I've enabled this in godot. Please, let me know if it works as you expected. Otherwise, open a new issue so we can track it there. Thanks!

:) 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 html_context let us know, and we can activate a flag for your project. We are going to sample some projects before making this the default.

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cagataycali picture cagataycali  路  4Comments

jaraco picture jaraco  路  4Comments

lennartkoopmann picture lennartkoopmann  路  4Comments

boscorelly picture boscorelly  路  4Comments

humitos picture humitos  路  4Comments