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

Fix HTML search not working with Sphinx-1.8. #672

Merged
merged 2 commits into from Oct 1, 2018

Conversation

tk0miya
Copy link
Contributor

@tk0miya tk0miya commented Sep 20, 2018

Unfortunately, Sphinx-1.8 breaks HTML search on sphinx_rtd_theme.
(see: sphinx-doc/sphinx#5460 )

To fix that, could you merge this please?
I'm sorry for trouble you.

Thanks,

Note:
Since Sphinx-1.7, Sphinx has exported search options as
documentation_options.js. This starts to refer it instead of
definitions of JavaScript variables.
In addition, this also uses js_tag() function which added in 1.8
to support additional attributes for scripts (ex. async attribute).

Since Sphinx-1.7, Sphinx has exported search options as
`documentation_options.js`.  This starts to refer it instead of
definitions of JavaScript variables.
In addition, this also uses `js_tag()` function which added in 1.8
to support additional attributes for scripts (ex. async attribute).
Copy link
Contributor

@jessetan jessetan left a comment

Choose a reason for hiding this comment

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

Code looks fine at first glance, but I'm kinda meh about adding a test for a specific Sphinx version in this theme. This may open up a can of version-specific workaround worms.
The alternative is to extend the base theme from Sphinx and include the scripts block, so we just get whatever Sphinx gives us. That does make us more dependent on changes in Sphinx (see comments in #467), but we will have to make adjustments anyway, as shown by this PR.

@tk0miya
Copy link
Contributor Author

tk0miya commented Sep 21, 2018

+1 for using scripts block.

@Blendify
Copy link
Member

I am all for using blocks also.

{%- for scriptfile in script_files %}
<script type="text/javascript" src="{{ pathto(scriptfile, 1) }}"></script>
{%- endfor %}
{% if sphinx_version >= "1.8.0" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think comparing versions as strings is a bad idea. For example, if Sphinx 1.10.0 is released, that version would be considered lower than 1.8.0 with such comparison.

Copy link
Contributor

Choose a reason for hiding this comment

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

Parsing the version number would be better, but for simplicity sake this solution is OK provided that Sphinx will not release a 1.10, but will go to 2.x. (that's what I gather from http://www.sphinx-doc.org/en/master/changes.html)

@tk0miya will Sphinx go from 1.9 to 2.0, or might there be 1.10, 1.11 etc?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, the next version will probably be really 2.0 so in this particular case it's fine. In general it's still a bad idea :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for late. We'll move to 2.0 on next release, not 1.10.

I think comparing versions as strings is a bad idea.

Agreed. But there are no tuple-formed versions inside template.
It would be better if sphinx provides it in future.

@jessetan
Copy link
Contributor

An issue about the broken search was just filed #673 and I don't have time to do a full "inherit stuff from the base theme using blocks" effort, so let's proceed with these changes (once reviewed) so we keep search functionality with Sphinx 1.8+

Refactoring to extend the base theme is a longer term goal that should still happen.

Copy link
Member

@Blendify Blendify left a comment

Choose a reason for hiding this comment

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

Good enough for a hotfix, can we add a comment stating why we did this?

@Blendify Blendify merged commit ebbf3d1 into readthedocs:master Oct 1, 2018
@Blendify
Copy link
Member

Blendify commented Oct 1, 2018

@agjohnson @ericholscher Poke, we need to do a release!

@agjohnson
Copy link
Collaborator

I'm also not super into the conditional logic used here, but agreed it's best for now. I'll open an issue to readdress this in our next release and get a release of 0.4.2 out

@gillesdouaire
Copy link

release v0.4.2 including this sphinx 1.8 fix soon?

@ericholscher
Copy link
Member

I'm release 0.4.2 today to fix this 👍

Odyseus added a commit to Odyseus/sphinx_rtd_theme_mod that referenced this pull request Nov 29, 2018
billsacks added a commit to ESCOMP/CISM-wrapper that referenced this pull request Dec 10, 2018
This cherry-picks a commit that fixes the search function when using the
sphinx_rtd_theme:

git cherry-pick -X subtree=doc/source/_themes/sphinx_rtd_theme ebbf3d1697c6518a3e398144610b9951bd7ab583

See also readthedocs/sphinx_rtd_theme#672
billsacks added a commit to ESCOMP/CISM-wrapper that referenced this pull request Dec 10, 2018
This cherry-picks a commit that fixes the search function when using the
sphinx_rtd_theme:

git cherry-pick -X subtree=doc/source/_themes/sphinx_rtd_theme ebbf3d1697c6518a3e398144610b9951bd7ab583

See also readthedocs/sphinx_rtd_theme#672
papaux added a commit to papaux/til that referenced this pull request Dec 26, 2018
henryaj pushed a commit to henryaj/buildgrid that referenced this pull request Nov 20, 2019
The HTML search problem with RTD + Sphinx 1.8 has been fixed upstream
and released in version 0.4.2:

    readthedocs/sphinx_rtd_theme#672

https://gitlab.com/BuildGrid/buildgrid/issues/114
billsacks added a commit to ESMCI/cime that referenced this pull request Mar 14, 2020
Documentation: provide a version dropdown menu and change theme

The documentation of cime is starting to diverge on the various
important branches (master, maint-5.6, and ufs_release_v1.0). This PR,
along with similar PRs for maint-5.6 and ufs_release_v1.0, and some
changes to the gh-pages branch, support having versioned documentation,
with a dropdown menu. This also changes the documentation theme to a
theme that supports this version dropdown. Note that this requires a
branch of the sphinx_rtd_theme; I will update the cime wiki on building
the documentation to note the new pip install step needed for that. (See
below for more details.)

At a high level, the way the versioned documentation works is to have
separate subdirectories in the gh-pages branch for each version of the
documentation we want to support. There is then a bit of JavaScript code
which uses a json file in the gh-pages branch to determine which
versions exist and how these should be named in the dropdown menu. This
changes the documentation build process slightly; I will update the wiki
to reflect these changes.

@mvertens and I made the various changes that you can see in
`doc/source` in this PR, which mainly borrowed from
ESCOMP/CISM-wrapper#23. That, in turn, is a
slight modification of an implementation provided by @MNLevy for the
MARBL documentation, which in turn borrowed from an implementation put
together by Unidata (credit where credit is due).

As mentioned above, I am not aware of out-of-the-box support for a
version pull-down in out-of-the-box sphinx themes (though the last time
I looked was in Fall, 2018, so there may be something available
now). However, support for a version dropdown exists in an open PR in
the sphinx readthedocs theme repository:
readthedocs/sphinx_rtd_theme#438. I have pushed
this branch to a new repository in ESMCI
(https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
maintenance of this branch in case it disappears from the official
sphinx_rtd_theme repository. I have also cherry-picked a commit onto
that branch, which is needed to fix search functionality in sphinx1.8
(from readthedocs/sphinx_rtd_theme#672) (which
is another reason for maintaining our own copy of this branch). The
branch in this repository is now named version-dropdown-with-fixes
(branching off of the version-dropdown branch in the sphinx_rtd_theme
repository). I will update the wiki to give the additional pip install
command needed to install this repository before doing the build. In the
long-term, I am a little concerned about using this theme that isn't
showing any signs of being merged to the main branch of the readthedocs
theme, but this has been working for us in other projects for the last 2
years, so I feel this is a reasonable approach in the short-medium term.

A few changes were also needed in the gh-pages branch:

- A `versions` directory was introduced at the top level, with
  subdirectories for each documentation version we want to support. We
  are using the convention that these subdirectories should have names
  that match the corresponding cime branches, to avoid uncertainty.

- A top-level index.html page was introduced that redirects to the
  master version of the documentation:

```html
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta http-equiv="refresh" content="0; url=versions/master/html/index.html" />
  </head>
</html>
```

- Other than those files and an empty `.nojekyll` file, everything else
  was removed from the top level directory of the documentation.

- In the versions directory, we also introduced a `versions.json`
  file. This lists the versions and provides a mapping between directory
  names and the names used in the pull-down menu:

```json
{
    "master": "master",
    "maint-5.6": "cime5.6",
    "ufs_release_v1.0": "ufs_release_v1.0"
}
```

- The complete contents of each version subdirectory are generated by
  the documentation build script. Thus, it is safe to remove the
  contents of (e.g.) `versions/master/` before rebuilding the
  documentation for that version. The documentation build is done
  similarly to before, but now providing a BUILDDIR argument to the make
  command in order to build in the correct subdirectory. For example,
  when building the documentation for master, you would provide
  `BUILDDIR=/path/to/cime_gh_pages_repo/versions/master/`.

Test suite: none
Test baseline: n/a
Test namelist changes: none
Test status: bit for bit

Fixes none

User interface changes?: none

Update gh-pages html (Y/N)?: Y (already done)

Code review:
billsacks added a commit to ESCOMP/CTSM that referenced this pull request Apr 7, 2020
Bring documentation source to master

1. Bring documentation source to master: Pulls in the source from
   https://github.com/escomp/ctsm-docs. This is important so that
   documentation can remain in sync with changes in the model
   code. Images are stored here using git-lfs (Git Large File
   Storage). I also made some minor fixes to get the pdf build of the
   tech note working.

2. Use a different documentation theme that supports a version dropdown
   menu, and add the code needed to support this versioning on the
   documentation web pages. At a high level, the way the versioned
   documentation works is to have separate subdirectories in the
   gh-pages branch of the ctsm-docs repository for each version of the
   documentation we want to support. There is then a bit of JavaScript
   code which uses a json file in the gh-pages branch to determine which
   versions exist and how these should be named in the dropdown
   menu. Most of these changes were borrowed from ESMCI/cime#3439, which
   in turn borrowed from ESCOMP/CISM-wrapper#23, which in turn was a
   slight modification of an implementation provided by @mnlevy1981 for
   the MARBL documentation, which in turn borrowed from an
   implementation put together by Unidata (credit where credit is due).

   I am not aware of out-of-the-box support for a version pull-down in
   out-of-the-box sphinx themes (though the last time I looked was in
   Fall, 2018, so there may be something available now). However,
   support for a version dropdown exists in an open PR in the sphinx
   readthedocs theme repository: readthedocs/sphinx_rtd_theme#438. I
   have pushed this branch to a new repository in ESMCI
   (https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
   maintenance of this branch in case it disappears from the official
   sphinx_rtd_theme repository. I have also cherry-picked a commit onto
   that branch, which is needed to fix search functionality in sphinx1.8
   (from readthedocs/sphinx_rtd_theme#672) (which is another reason for
   maintaining our own copy of this branch). The branch in this
   repository is now named version-dropdown-with-fixes (branching off of
   the version-dropdown branch in the sphinx_rtd_theme repository). In
   the long-term, I am a little concerned about using this theme that
   isn't showing any signs of being merged to the main branch of the
   readthedocs theme, but this has been working for us in other projects
   for the last 2 years, so I feel this is a reasonable approach in the
   short-medium term.

The new process for building the documentation is given here:
https://github.com/ESCOMP/CTSM/wiki/Directions-for-editing-CLM-documentation-on-github-and-sphinx

Resolves #239
ekluzek added a commit to ekluzek/CTSM that referenced this pull request Apr 7, 2020
Bring documentation source to master

1. Bring documentation source to master: Pulls in the source from
   https://github.com/escomp/ctsm-docs. This is important so that
   documentation can remain in sync with changes in the model
   code. Images are stored here using git-lfs (Git Large File
   Storage). I also made some minor fixes to get the pdf build of the
   tech note working.

2. Use a different documentation theme that supports a version dropdown
   menu, and add the code needed to support this versioning on the
   documentation web pages. At a high level, the way the versioned
   documentation works is to have separate subdirectories in the
   gh-pages branch of the ctsm-docs repository for each version of the
   documentation we want to support. There is then a bit of JavaScript
   code which uses a json file in the gh-pages branch to determine which
   versions exist and how these should be named in the dropdown
   menu. Most of these changes were borrowed from ESMCI/cime#3439, which
   in turn borrowed from ESCOMP/CISM-wrapper#23, which in turn was a
   slight modification of an implementation provided by @mnlevy1981 for
   the MARBL documentation, which in turn borrowed from an
   implementation put together by Unidata (credit where credit is due).

   I am not aware of out-of-the-box support for a version pull-down in
   out-of-the-box sphinx themes (though the last time I looked was in
   Fall, 2018, so there may be something available now). However,
   support for a version dropdown exists in an open PR in the sphinx
   readthedocs theme repository: readthedocs/sphinx_rtd_theme#438. I
   have pushed this branch to a new repository in ESMCI
   (https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
   maintenance of this branch in case it disappears from the official
   sphinx_rtd_theme repository. I have also cherry-picked a commit onto
   that branch, which is needed to fix search functionality in sphinx1.8
   (from readthedocs/sphinx_rtd_theme#672) (which is another reason for
   maintaining our own copy of this branch). The branch in this
   repository is now named version-dropdown-with-fixes (branching off of
   the version-dropdown branch in the sphinx_rtd_theme repository). In
   the long-term, I am a little concerned about using this theme that
   isn't showing any signs of being merged to the main branch of the
   readthedocs theme, but this has been working for us in other projects
   for the last 2 years, so I feel this is a reasonable approach in the
   short-medium term.

The new process for building the documentation is given here:
https://github.com/ESCOMP/CTSM/wiki/Directions-for-editing-CLM-documentation-on-github-and-sphinx

Resolves ESCOMP#239
ekluzek added a commit to ekluzek/CTSM that referenced this pull request Apr 20, 2020
Bring documentation source to master

1. Bring documentation source to master: Pulls in the source from
   https://github.com/escomp/ctsm-docs. This is important so that
   documentation can remain in sync with changes in the model
   code. Images are stored here using git-lfs (Git Large File
   Storage). I also made some minor fixes to get the pdf build of the
   tech note working.

2. Use a different documentation theme that supports a version dropdown
   menu, and add the code needed to support this versioning on the
   documentation web pages. At a high level, the way the versioned
   documentation works is to have separate subdirectories in the
   gh-pages branch of the ctsm-docs repository for each version of the
   documentation we want to support. There is then a bit of JavaScript
   code which uses a json file in the gh-pages branch to determine which
   versions exist and how these should be named in the dropdown
   menu. Most of these changes were borrowed from ESMCI/cime#3439, which
   in turn borrowed from ESCOMP/CISM-wrapper#23, which in turn was a
   slight modification of an implementation provided by @mnlevy1981 for
   the MARBL documentation, which in turn borrowed from an
   implementation put together by Unidata (credit where credit is due).

   I am not aware of out-of-the-box support for a version pull-down in
   out-of-the-box sphinx themes (though the last time I looked was in
   Fall, 2018, so there may be something available now). However,
   support for a version dropdown exists in an open PR in the sphinx
   readthedocs theme repository: readthedocs/sphinx_rtd_theme#438. I
   have pushed this branch to a new repository in ESMCI
   (https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
   maintenance of this branch in case it disappears from the official
   sphinx_rtd_theme repository. I have also cherry-picked a commit onto
   that branch, which is needed to fix search functionality in sphinx1.8
   (from readthedocs/sphinx_rtd_theme#672) (which is another reason for
   maintaining our own copy of this branch). The branch in this
   repository is now named version-dropdown-with-fixes (branching off of
   the version-dropdown branch in the sphinx_rtd_theme repository). In
   the long-term, I am a little concerned about using this theme that
   isn't showing any signs of being merged to the main branch of the
   readthedocs theme, but this has been working for us in other projects
   for the last 2 years, so I feel this is a reasonable approach in the
   short-medium term.

The new process for building the documentation is given here:
https://github.com/ESCOMP/CTSM/wiki/Directions-for-editing-CLM-documentation-on-github-and-sphinx

Resolves ESCOMP#239
jtruesdal added a commit to jtruesdal/ctsm that referenced this pull request May 1, 2020
Bring documentation source to master

1. Bring documentation source to master: Pulls in the source from
   https://github.com/escomp/ctsm-docs. This is important so that
   documentation can remain in sync with changes in the model
   code. Images are stored here using git-lfs (Git Large File
   Storage). I also made some minor fixes to get the pdf build of the
   tech note working.

2. Use a different documentation theme that supports a version dropdown
   menu, and add the code needed to support this versioning on the
   documentation web pages. At a high level, the way the versioned
   documentation works is to have separate subdirectories in the
   gh-pages branch of the ctsm-docs repository for each version of the
   documentation we want to support. There is then a bit of JavaScript
   code which uses a json file in the gh-pages branch to determine which
   versions exist and how these should be named in the dropdown
   menu. Most of these changes were borrowed from ESMCI/cime#3439, which
   in turn borrowed from ESCOMP/CISM-wrapper#23, which in turn was a
   slight modification of an implementation provided by @mnlevy1981 for
   the MARBL documentation, which in turn borrowed from an
   implementation put together by Unidata (credit where credit is due).

   I am not aware of out-of-the-box support for a version pull-down in
   out-of-the-box sphinx themes (though the last time I looked was in
   Fall, 2018, so there may be something available now). However,
   support for a version dropdown exists in an open PR in the sphinx
   readthedocs theme repository: readthedocs/sphinx_rtd_theme#438. I
   have pushed this branch to a new repository in ESMCI
   (https://github.com/ESMCI/sphinx_rtd_theme) to facilitate long-term
   maintenance of this branch in case it disappears from the official
   sphinx_rtd_theme repository. I have also cherry-picked a commit onto
   that branch, which is needed to fix search functionality in sphinx1.8
   (from readthedocs/sphinx_rtd_theme#672) (which is another reason for
   maintaining our own copy of this branch). The branch in this
   repository is now named version-dropdown-with-fixes (branching off of
   the version-dropdown branch in the sphinx_rtd_theme repository). In
   the long-term, I am a little concerned about using this theme that
   isn't showing any signs of being merged to the main branch of the
   readthedocs theme, but this has been working for us in other projects
   for the last 2 years, so I feel this is a reasonable approach in the
   short-medium term.

The new process for building the documentation is given here:
https://github.com/ESCOMP/CTSM/wiki/Directions-for-editing-CLM-documentation-on-github-and-sphinx

Resolves ESCOMP#239
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.

None yet

7 participants