readthedocs.org icon indicating copy to clipboard operation
readthedocs.org copied to clipboard

Autopopulate VCS variables used by `sphinx-basic-ng`

Open pradyunsg opened this issue 2 years ago • 2 comments

So... with sphinx-basic-ng, a bunch of the theme developers in the Sphinx ecosystem are trying to get a shared base theme that we all build upon.

Notably, this has reusable implementations of various design "components" (https://sphinx-basic-ng.readthedocs.io/en/latest/usage/components/). In a few of them[^1],

html_theme_options = {
    "source_repository": "https://github.com/pradyunsg/sphinx-basic-ng/",  # you can skip the trailing /
    "source_branch": "main",
    "source_directory": "docs",  # or "docs/"
}

It would be great if builds on Read the Docs could auto-populate these theme variables, when they're not provided by the user.

That would make the community-maintained themes that are using these components "just work" on Read the Docs, similar to how the Read the Docs theme works! Since this is designed to be a shared implementation across themes (Furo and Lutra already use it, there's efforts underway to get this for pydata-sphinx-theme and sphinx-book-theme as well), it's somewhat theme-agnostic and would be a nice user experience improvement!

Notably, it would allow themes to remove custom code for supporting VCS information conditionally on Read the Docs (Furo does this!).

[^1]: edit-this-page and view-this-page

pradyunsg avatar Jul 10 '22 11:07 pradyunsg

FWIW, this is (in effect) a replacement for https://github.com/readthedocs/readthedocs.org/blob/main/docs/user/guides/edit-source-links-sphinx.rst .

If you'd prefer to provide a fully-formatted link instead, that's coming in https://github.com/pradyunsg/sphinx-basic-ng/pull/34 -- which has the relevant documentation updates too. :)

pradyunsg avatar Jul 10 '22 11:07 pradyunsg

It would be great if builds on Read the Docs could auto-populate these theme variables, when they're not provided by the user.

Would it be useful if Read the Docs passes these as environment variables (e.g. READTHEDOCS_SOURCE_REPOSITORY, etc) and it's the theme itself that defines them to the Sphinx context? We are moving away of defining things automagically behind the scenes for the users because it creates confusing scenarios.

humitos avatar Jul 11 '22 09:07 humitos

Would it be useful if Read the Docs passes these as environment variables (e.g. READTHEDOCS_SOURCE_REPOSITORY, etc) and it's the theme itself that defines them to the Sphinx context?

That works!

pradyunsg avatar Sep 30 '22 16:09 pradyunsg

READTHEDOCS_SOURCE_REPOSITORY works for me, too, but I think it should be READTHEDOCS_SOURCE_REPOSITORY_WEB_URL since the source repository has other properties that we can also expose over time if requested? I think that READTHEDOCS_SOURCE_REPOSITORY_URL is ambiguous :) We can also trim _SOURCE.

  • READTHEDOCS_GIT_WEB_URL: Indicates the root path of the repo website, i.e. https://github.com/org/repo
  • READTHEDOCS_GIT_PROVIDER: "github"/"bitbucket"/"gitlab" - is that useful for you, @pradyunsg ?
  • READTHEDOCS_GIT_IDENTIFIER: Populated with name of tag, branch or commit hash (only one of them and selected in that priority - if tag exists, uses tag, if branch uses branch otherwise commit hash)
  • READTHEDOCS_SOURCE_DIR: If the Sphinx builder is active, we can put the directory here (note @humitos maybe good to call the new READTHEDOCS_OUTPUT READTHEDOCS_OUTPUT_DIR instead?)

benjaoming avatar Feb 16 '23 14:02 benjaoming

I'm not against additional environment variables, those could be very handy in some situations, but I wanted to mention that the RTD build process already provides some (all?) of the mentioned information in html_context.

The source repository can be found by looking at display_gitlab, combined with gitlab_user and gitlab_repo (same for bitbucket and github).

I don't know if the actual branch name is available, and I'm not sure how reliable it would be anyway, but the commit is available in the field commit.

The source directory is available in conf_py_path (strictly speaking, conf.py doesn't have to be in the source directory, but normally it is).

I'm using all this information to create a source link at the bottom of each page in my insipid theme, see https://insipid-sphinx-theme.readthedocs.io/en/0.4.1/configuration.html#confval-html_show_sourcelink

mgeier avatar Feb 18 '23 16:02 mgeier

@mgeier

I'm not against additional environment variables, those could be very handy in some situations, but I wanted to mention that the RTD build process already provides some (all?) of the mentioned information in html_context.

We are moving away from built-in Sphinx-only features and trying to support other tools in a cleaner and generic way. So, the additional environment variables would be useful for these other documentation tools as well (e.g. MkDocs, Docusaurus, etc).

I think the right way to keep supporting Sphinx-only features is by creating more Sphinx extensions than adding them into the Read the Docs' build process itself. We have tested this already with some extensions and we are happy with the results, so we will probably keep moving in that direction.

BTW, thanks for pointing out where to get this information from with the current build process! 👍🏼

humitos avatar Feb 22 '23 11:02 humitos

@benjaoming

I wouldn't use the word GIT in the variables, I think it just add noise and we currently support other as well. Yes, we want to deprecate them too 😄

These are my suggestions following your notes:

  • READTHEDOCS_REPOSITORY_URL (GitHub URL, like https://github.com/readthedocs/readthedocs.org/)
  • READTHEDOCS_REPOSITORY_IDENTIFIER (e.g. main or v1.0.0 or a1b2c3 --matching what's used in the Version object)

Thinking a little more about READTHEDOCS_SOURCE_DIR, what should be its value for different doctools? How we will get it correctly? What's the value for Docusaurus, or Gatsby, for example? I'm not sure we can commit ourselves to populate it with the right value. However, this is related to https://github.com/readthedocs/readthedocs.org/issues/9088

I'd move forward with the first two that are the ones that we already know, and leave the third one to continue discussing the deciding how to implement it.

humitos avatar Feb 22 '23 11:02 humitos