CEED / libCEED

CEED Library: Code for Efficient Extensible Discretizations
https://libceed.org
BSD 2-Clause "Simplified" License
203 stars 47 forks source link

sphinx-hoverxref not showing tooltips' content #506

Closed humitos closed 4 years ago

humitos commented 4 years ago

Hello! I'm one of the authors of sphinx-hoverxref and I just saw that the tooltips' are not working in your docs.

I just wanted to let you know that the problem(*) you are having is already solved in master branch of the project and I'm preparing a new release with this fix among others and new features (hopefully released this week). You shouldn't need to do anything to fix the tooltips after the release.

Although, if you want to fix it in your current docs, you can install it from master by changing it in your requirements.txt (like, git+https://github.com/readthedocs/sphinx-hoverxref@master#egg=sphinx-hoverxref==0.1b-master)

(*) it didn't support using HTMLDir's Sphinx builder (builder: dirhtml config).

valeriabarra commented 4 years ago

Thanks for reporting on this.

jedbrown commented 4 years ago

News that bugs were fixed upstream is the best kind of news. I put this in a branch for testing. https://github.com/CEED/libCEED/tree/jed/doc-hoverxref

jedbrown commented 4 years ago

Looks nice; thanks.

Screenshot_20200406_135719

jedbrown commented 4 years ago

@humitos I think there is still a bug. Go to the page screenshot above, hover over Examples, and try to click on them. https://libceed.readthedocs.io/en/latest/libCEEDapi/

The links go to pages like https://libceed.readthedocs.io/en/latest/libCEEDapi/notation/ (broken)

but should go to https://libceed.readthedocs.io/en/latest/examples/notation/

Same issue with Sphinx-3.0 at this link: https://libceed.readthedocs.io/en/jed-sphinx-3.0/libCEEDapi/

humitos commented 4 years ago

Relative links inside the tooltip were broken already. See https://github.com/readthedocs/sphinx-hoverxref/issues/10 --this was a problem in the Read the Docs backend returning incorrect links.

The fixes was merged and deployed yesterday (both, in RTD but also in the extension, adding a new path= attribute in the request). Could you please trigger a new build using the extension from master?

Also, I see that your site is using dirhtml Sphinx builder (URLs does not end with .html). I'll double check if there is a problem in the backend or the extension itself with that type of builder that I haven't tested to much yet.

Thanks for reporting this back!

humitos commented 4 years ago

I just saw that the links from the Example page are like href="notation/", and it seems that this case is not handled in the RTD's backed.

humitos commented 4 years ago

FYI, I just fixed this issue upstream and it will be deployed next week hopefully. All the links from inside the tooltip should be correct by that time.

jedbrown commented 4 years ago

Thanks, @humitos. How does one force pip (as run on RTD) to upgrade a requirement? It isn't using your version even when I name the commit

git+https://github.com/readthedocs/sphinx-hoverxref@9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20#egg=sphinx-hoverxref==0.1b-master

yet its still stuck at the old version

Collecting sphinx-hoverxref==0.1b-master
  Cloning https://github.com/readthedocs/sphinx-hoverxref (to revision 9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20) to /tmp/pip-install-h1ubjo7k/sphinx-hoverxref
  Running command git clone -q https://github.com/readthedocs/sphinx-hoverxref /tmp/pip-install-h1ubjo7k/sphinx-hoverxref
  WARNING: Requested sphinx-hoverxref==0.1b-master from git+https://github.com/readthedocs/sphinx-hoverxref@9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20#egg=sphinx-hoverxref==0.1b-master (from -r doc/sphinx/requirements.txt (line 9)), but installing version 0.1b1

Edit: perhaps related to this closed issue: https://github.com/pypa/pip/issues/17

jedbrown commented 4 years ago

I'm leaving this issue open until there is a hoverxref release or we can otherwise get RTD to use a current enough version.

humitos commented 4 years ago

Hi @jedbrown! RTD is installing the version from your commit properly. The warning message that you are seeing there is a normal pip message and it happens because the version that you are specifying in does not match with the version that the package says it contains.

Running that outside RTD, produces the same output:

$ pip install git+https://github.com/readthedocs/sphinx-hoverxref@9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20#egg=sphinx-hoverxref==0.1b-master
Collecting sphinx-hoverxref==0.1b-master
  Cloning https://github.com/readthedocs/sphinx-hoverxref (to revision 9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20) to /tmp/pip-install-zt__sc6q/sphinx-hoverxref
  Running command git clone -q https://github.com/readthedocs/sphinx-hoverxref /tmp/pip-install-zt__sc6q/sphinx-hoverxref
  WARNING: Requested sphinx-hoverxref==0.1b-master from git+https://github.com/readthedocs/sphinx-hoverxref@9fb3c6652e40d8e2bd98a2a9421ab44d2572eb20#egg=sphinx-hoverxref==0.1b-master, but installing version 0.1b1
Installing collected packages: sphinx-hoverxref
    Running setup.py install for sphinx-hoverxref ... done
Successfully installed sphinx-hoverxref-0.1b1

I'm preparing a new release of hoverxref and also a PR that will fix the issue on the RTD backend. Both things are needed to fully fix the issue around broken relative links from the embedded content.

jedbrown commented 4 years ago

Hmm, okay. The build was failing on RTD and I couldn't reproduce locally. I guess I'll wait until the fixes are merged before reactivating.

humitos commented 4 years ago

We did a deploy today of the server. I think your issue with the links should be solved. Make sure that you update sphinx-hoverxref extension in your requirements as well.

Please, let me know if it works as you expected or any other feedback you may have. Thanks!

jedbrown commented 4 years ago

@humitos Is it necessary to disable for LaTeX builds?

/home/docs/checkouts/readthedocs.org/user_builds/libceed/envs/jed-activate-hoverxref/lib/python3.8/site-packages/sphinx/deprecation.py:49: RemovedInSphinx40Warning: sphinx.builders.html.DirectoryHTMLBuilder is deprecated. Check CHANGES for Sphinx API modifications.
  return getattr(self._module, name)
/home/docs/checkouts/readthedocs.org/user_builds/libceed/envs/jed-activate-hoverxref/lib/python3.8/site-packages/readthedocs_ext/readthedocs.py:14: RemovedInSphinx40Warning: sphinx.builders.html.SingleFileHTMLBuilder is deprecated. Check CHANGES for Sphinx API modifications.
  from sphinx.builders.html import (DirectoryHTMLBuilder, SingleFileHTMLBuilder,
/home/docs/checkouts/readthedocs.org/user_builds/libceed/envs/jed-activate-hoverxref/lib/python3.8/site-packages/recommonmark/parser.py:65: UserWarning: Container node skipped: type=document
  warn("Container node skipped: type={0}".format(mdnode.t))

Exception occurred:
  File "/home/docs/checkouts/readthedocs.org/user_builds/libceed/envs/jed-activate-hoverxref/lib/python3.8/site-packages/hoverxref/domains.py", line 50, in _get_docpath
    docpath = builder.get_outfilename(docname)
AttributeError: 'LaTeXBuilder' object has no attribute 'get_outfilename'
The full traceback has been saved in /tmp/sphinx-err-pcm7054k.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
humitos commented 4 years ago

@jedbrown hrm... This looks like a bug in hoverxref. It should be disabled automatically on non-html builds. I don't know why I didn't hit this by myself previously. I will fix it soon. Thanks for the report.