Intersphinx support #86

humitos · 2020-08-20T15:04:17Z

Just an idea of what we can do to support intersphinx in this extension. There are different things to coordinate and decide yet, but I wanted to have something working to see how it could fit.

Rendered version with an example:
https://sphinx-hoverxref--86.org.readthedocs.build/en/86/usage.html#tooltip-on-intersphinx-content

How it works?

Intersphinx subscribes to missing-reference event to use different methods to try to resolve the reference. If it succeed by using one of the intersphinx configurations (from intersphinx_mapping config) it returns a nodes.reference. As we can't hook into the middle of that function, we disconnect the original function called on missing-reference event and we connect a custom one. There we call the original function and add some extra data (CSS classes and data-url) into the node.

Small example

It populates the modal with the content of this page https://docs.readthedocs.io/en/stable/config-file/v2.html#search-ranking

ToDo

allow style customization (tooltip or modal)
config to enable/disable tooltip on intersphinx links
remove the title= HTML property?
write the tests for this cases
document how to use it
add an example into the documentation

Closes #51
Reference https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html
Reference https://github.com/sphinx-doc/sphinx/blob/53c1dff/sphinx/ext/intersphinx.py

Disconnect original `missing-reference` event from `sphinx.ext.intersphinx` to connect our own function. When this event is triggered, we call the original `missing_reference` function and add some extra data (`data-url` and CSS classes) to the node returned by the original function. This data is later used by the transformer to render the node into HTML as we expect.

schandrika · 2020-11-07T02:15:43Z

Would like to use this feature. Will this PR get merged in the near future?

humitos · 2020-11-09T09:49:55Z

Hi, @schandrika! I wrote this PR as a POC to understand if it was possible to make the extension work like we thought. However, there is still some work to do here (tests, documentation, QA, etc) and I haven't have the time to work on it unfortunately.

You can try installing the extension from this PR in your project and decide by yourself if it's usable or not (without warranty 😄). Doing that test would be good to have some feedback about it from a real documentation project.

Note that both projects (original and linked via intersphinx) have to be hosted on Read the Docs for the extension to work.

…itos/support-intersphinx

Config File is a big document to show as example in the modal.

humitos · 2021-05-05T11:56:05Z

Hi @schandrika! I pushed some changes to this PR and I updated the documentation. Would you like to install the extension from this branch and let me know if it works for you? I'd be happy to fix any issue that could appear before merging. Thanks!

It's confusing to use the default browser's behavior when hovering a HTML tag with `title=` attribute and immediately after show the our own modal/tooltip.

ericholscher

This looks great 💯

docs/conf.py

docs/configuration.rst

docs/usage.rst

hoverxref/extension.py

ericholscher · 2021-05-19T19:28:13Z

hoverxref/extension.py

@@ -139,6 +140,57 @@ def setup_sphinx_tabs(app, config):
            app.disconnect(listener_id)


+def setup_intersphinx(app, config):


Should we call this disconnect_intersphinx or something a bit more explicit?

I'm following the pattern I've been using already with setup_ as prefix for each requirement.

hoverxref/extension.py

ericholscher · 2021-05-19T19:31:18Z

hoverxref/_static/js/hoverxref.js_t

-        'path': docpath,
-        'section': section,
+function getEmbedURL(project, version, doc, docpath, section, url) {
+    if (url) {


If we can detect there is data coming from Intersphinx, I wonder if we could note that in the embed UI? I wanted a Sphinx: or similar as a context clue about the content when I used the PR build.

I like the idea to differentiate a link that shows content from another project! We could add an extra class, e.g. intersphinx, to differentiate these from other regular hoverxref links and do something specific in the frontend for them. I'll open an issue to work on this from the FE as an improvement. For now, I'm going to add the extra class to these nodes.

I just realized that Sphinx uses internal and external classes to differentiate this. We may want to rely on that directly.

Cool. I just think it's useful to know where the page is coming from, so maybe just a larger feature later that adds breadcrumbs or something outside the embed frame to give more context to the document.

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

…nx-hoverxref into humitos/support-intersphinx

astrojuanlu · 2021-05-25T14:09:58Z

tests/examples/intersphinx/conf.py

+hoverxref_intersphinx = [
+    'python',
+]


Wouldn't it be more natural to use readthedocs for this test instead, since python is not hosted on RtD at the moment?

Maybe. It doesn't really affect the test here. It's just a test to check that the backend is generating the HTML tag as we expect. It doesn't really make the request or anything.

On the other hand, the client should show a nicer error if the request returns 404 😄

TypeError: can't pickle dict_keys objects

astrojuanlu · 2021-05-25T15:54:38Z

Testing: poliastro/poliastro#1234

…xref#86

chrizzFTD · 2021-05-28T15:20:16Z

Hi @humitos, thanks a lot for this exciting feature!

I was just testing this out for my project at https://grill.readthedocs.io/ and I can't find a way to make it work. The intersphinx links never pop a modal dialog or tooltip when hovering over them. However, links for the same project do work (same as in the master branch for this repo)

I am keen on attempting to debug what is happening, but I honestly don't know where to start :(

Would somebody help me look in the right direction, please?

These are the details I can provide:

Where to look in my docs

The part to test the hoverxref intersphinx support can be found on the grill.write.UsdAsset docs, exactly in the part of:

SEE ALSO
grill.names.CGAsset for a description of available fields and naming.Name for a description of the core API.

These links point to 2 separate RTD hosted projects: https://grill-names.readthedocs.io and http://naming.readthedocs.io.

Current build log

I can confirm that I am building sphinx-hoverxref from this branch. This is my latest build on RTD for my PR thegrill/grill#23:

https://readthedocs.org/projects/grill/builds/13878150/

Collecting sphinx-hoverxref@ git+https://github.com/readthedocs/sphinx-hoverxref@humitos/support-intersphinx
  Cloning https://github.com/readthedocs/sphinx-hoverxref (to revision humitos/support-intersphinx) to /tmp/pip-install-bvynei4w/sphinx-hoverxref_0bc00bc5ef3f4681b89f14bf61ab9dbc
  Running command git clone -q https://github.com/readthedocs/sphinx-hoverxref /tmp/pip-install-bvynei4w/sphinx-hoverxref_0bc00bc5ef3f4681b89f14bf61ab9dbc
  Running command git checkout -b humitos/support-intersphinx --track origin/humitos/support-intersphinx
  Switched to a new branch 'humitos/support-intersphinx'
  Branch 'humitos/support-intersphinx' set up to track remote branch 'humitos/support-intersphinx' from 'origin'.

Worth to mention that I am using a custom sphinx branch (these are the changes) but I have confirmed I still have the issue using the normal published sphinx pypi package.

Collecting sphinx@ git+https://github.com/thegrill/sphinx.git@grill-intersphinx-uniqueref-fix
  Cloning https://github.com/thegrill/sphinx.git (to revision grill-intersphinx-uniqueref-fix) to /tmp/pip-install-bvynei4w/sphinx_94e8d8d8613641ecbc3064d807e1914f
  Running command git clone -q https://github.com/thegrill/sphinx.git /tmp/pip-install-bvynei4w/sphinx_94e8d8d8613641ecbc3064d807e1914f
  Running command git checkout -b grill-intersphinx-uniqueref-fix --track origin/grill-intersphinx-uniqueref-fix
  Switched to a new branch 'grill-intersphinx-uniqueref-fix'
  Branch 'grill-intersphinx-uniqueref-fix' set up to track remote branch 'grill-intersphinx-uniqueref-fix' from 'origin'.

Current configuration

This is the relevant conf.py I have:

intersphinx_mapping = {
    'python': ('https://docs.python.org/3', None),
    'naming': ('http://naming.readthedocs.io/en/latest/', None),
    'grill.names': ('https://grill-names.readthedocs.io/en/latest/', None)
}
hoverxref_auto_ref = True
hoverxref_intersphinx = [
  'grill.names',
  'naming',
]
hoverxref_intersphinx_type = 'modal'
hoverxref_domains = ['py']

How to debug?

I noticed if I build locally, hoverxref does not execute when I open my local files in the browser, is this expected? If not, do you have some tips on how to debug this? So far I've been running the local build like this:

from sphinx.cmd import build
if __name__=='__main__':
    import shutil
    try:
        shutil.rmtree(r"B:\write\code\git\grill\docs\build")
    except FileNotFoundError:
        pass
    build.build_main([r"B:\write\code\git\grill\docs\source", r"B:\write\code\git\grill\docs\build"])

probably there's a different way :P

That was a lot of text, sorry for the book!

Use Sphinx's internals to check if the object requested belongs to one of the inventories defined by the user under ``hoverxref_interpshinx`` and add hoverxref on the node based on that.

humitos · 2021-06-01T09:18:50Z

Thanks @chrizzFTD for your feedback and test!

I found some issues last week and I was able to fix them between yesterday and today. Could you do your test again using the latest commit from this branch? Note that I changed a config hoverxref_intersphinx_type that's now called hoverxref_intersphinx_types (note the final "s") where you can specify which style you want per intersphinx mapping as a dictionary. Let me know how it goes!

…ersphinx

humitos · 2021-06-01T09:46:48Z

@ericholscher I was able to make the feature work as I wanted 😄 --but I had to change some parts of the missing_reference function's code. You may want to take another quick review, probably. Let me know!

astrojuanlu · 2021-06-01T12:25:09Z

Tested it on poliastro it works! See for example this function https://poliastro--1234.org.readthedocs.build/en/1234/autoapi/poliastro/twobody/orbit/index.html#poliastro.twobody.orbit.Orbit.from_horizons

chrizzFTD · 2021-06-01T13:44:00Z

Hi @humitos, thanks a lot for the updates!

I tested it but my builds started failing; build 13902904 last log is:

Click to show log

Using default style (tooltip) for unknown typ (class). Define it in hoverxref_role_types.
Using default style (tooltip) for unknown typ (class). Define it in hoverxref_role_types.
Using default style (tooltip) for unknown typ (class). Define it in hoverxref_role_types.
Using default style (tooltip) for unknown typ (func). Define it in hoverxref_role_types.
_hoverxref attributes: {'class': 'reference external', 'href': 'https://grill-names.readthedocs.io/en/latest/names/CGAssetFile.html#grill.names.CGAssetFile', 'data-url': 'https://grill-names.readthedocs.io/en/latest/names/CGAssetFile.html#grill.names.CGAssetFile'}

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/application.py", line 352, in build
    self.builder.build_update()
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 298, in build_update
    len(to_build))
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 360, in build
    self.write(docnames, list(updated_docnames), method)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 534, in write
    self._write_serial(sorted(docnames))
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 544, in _write_serial
    self.write_doc(docname, doctree)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/builders/html/__init__.py", line 597, in write_doc
    self.docwriter.write(doctree, destination)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/writers/__init__.py", line 78, in write
    self.translate()
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/writers/html.py", line 71, in translate
    self.document.walkabout(visitor)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 227, in walkabout
    if child.walkabout(visitor):
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 227, in walkabout
    if child.walkabout(visitor):
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 227, in walkabout
    if child.walkabout(visitor):
  [Previous line repeated 2 more times]
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 219, in walkabout
    visitor.dispatch_visit(self)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/util/docutils.py", line 468, in dispatch_visit
    method(node)
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/sphinx/ext/inheritance_diagram.py", line 413, in html_visit_inheritance_diagram
    urls[child['reftitle']] = child.get('refuri')
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 652, in __getitem__
    return self.attributes[key]
KeyError: 'reftitle'

Exception occurred:
  File "/home/docs/checkouts/readthedocs.org/user_builds/grill/envs/feature-usd_resources/lib/python3.7/site-packages/docutils/nodes.py", line 652, in __getitem__
    return self.attributes[key]
KeyError: 'reftitle'
The full traceback has been saved in /tmp/sphinx-err-5f1wxo8y.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!

I noticed html_visit_inheritance_diagram in the stacktrace (I am using the sphinx.ext.inheritance_diagram extension) and so I tried commenting out those lines:

sphinx-hoverxref/hoverxref/extension.py

Lines 246 to 247 in b96f217

    
           if 'reftitle' in  newnode: 
        
               del newnode['reftitle']

Which made a new build from my branch successful and I can see the intersphinx links working with hoverxref on my class docs!

Is there another way to achieve the same outcome for # Remove ``title=`` from HTML tag to avoid showing the title of the reference when hovering, which is a little confusing? On my docs, I could not really see what exact part this is referring to :o

Thanks again!

humitos · 2021-06-01T14:23:44Z

Hey @chrizzFTD, thanks a lot for your tests! It's really useful to me.

The problem with reftitle it's because I'm removing it just to avoid showing two different tooltips:

the browser's tooltip immediately you hover the link
and hoverxref's tooltip after that

I added it as a minor style improvement, but it's not super important. I will remove it for now because it may make more projects break (I'm removing a field that it's expected to exist) and open an issue to find a better solution.

#86 (comment) There are other extensions that expect `reftitle` to be an attribute in the node. They are doing `node['reftitle']` without checking if it exists first. This happens in `sphinx.ext.inheritance_diagram` which is a built-in extension. So it's preferrable to not modify the node and find another way to remove the browser's tooltip later. See https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/ext/inheritance_diagram.py#L413

chrizzFTD · 2021-06-02T10:44:28Z

Thanks a lot, @humitos, I can confirm the updates on this branch work fine for me now! 😃

ericholscher

Looks good 💯

ericholscher · 2021-06-03T16:23:58Z

docs/conf.py

@@ -56,8 +56,14 @@
    'readthedocs': ('https://docs.readthedocs.io/en/stable/', None),
    'sphinx': ('https://www.sphinx-doc.org/en/master/', None),
 }
-hoverxref_intersphinx = True
-hoverxref_intersphinx_type = 'modal'
+hoverxref_intersphinx = [


Should this be hoverxref_intersphinx_projects? The name currently doesn't really explain what the data type inside would be.

It seems that we're overloading it to be "enable and list of projects" -- perhaps that still works with projects in the name, since having [] projects effectively means disabled.

It seems that we're overloading it to be "enable and list of projects"

Yes.

Should this be hoverxref_intersphinx_projects? The name currently doesn't really explain what the data type inside would be.

Hrm, yeah. I'm not super happy with the name either. However, the most correct technical name would be hoverxref_intersphinx_mappings since it refers to the keys from the intersphinx_mappings config --but I didn't like it 😞

docs/configuration.rst

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

Similiar to `hoverxref_types` but for each intersphinx mapping.

humitos · 2021-06-07T11:02:28Z

@chrizzFTD I merged this PR and made a new release. Please, update your requirements and configs to be updated. Thanks for all the testing and help on this!

chrizzFTD · 2021-06-10T10:40:12Z

no worries, I'll update during the weekend, thanks a lot for this awesome feature, @humitos ! 😃

humitos added 2 commits August 20, 2020 16:47

Only send ?url= to Embed API when data-url is available

9f30017

humitos added Feature New feature Needed: design decision A core team decision is required Needed: documentation Documentation is required Needed: tests Tests are required labels Aug 25, 2020

humitos added 5 commits May 5, 2021 11:31

Merge branch 'master' of github.com:humitos/sphinx-hoverxref into hum…

25a6936

…itos/support-intersphinx

Add testcase for intersphinx

7eae53b

Make tootltip type for intersphinx configurable

121af16

Update copyright year

29394a8

Docs: for intersphinx

501fba4

humitos removed Needed: tests Tests are required Needed: documentation Documentation is required Needed: design decision A core team decision is required labels May 5, 2021

humitos added 3 commits May 5, 2021 13:40

Docs: fail on warning

3a25b2c

Docs: use :doc: to reference documents via intersphinx

b838f4e

Docs: use automation rules for the tooltip

fab9d6c

Config File is a big document to show as example in the modal.

humitos requested a review from a team May 5, 2021 11:53

Remove title= from HTML tag

3e81de8

It's confusing to use the default browser's behavior when hovering a HTML tag with `title=` attribute and immediately after show the our own modal/tooltip.

ericholscher approved these changes May 19, 2021

View reviewed changes

ericholscher reviewed May 19, 2021

View reviewed changes

humitos and others added 2 commits May 20, 2021 10:29

Update docs/usage.rst

dd1ae8c

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

Update hoverxref/extension.py

bc6ae49

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

humitos mentioned this pull request May 20, 2021

Intersphinx: differentiate tooltips with content comming from other projects #130

Open

humitos added 3 commits May 20, 2021 11:00

Merge branch 'master' into humitos/support-intersphinx

5a94e4d

Merge branch 'humitos/support-intersphinx' of github.com:humitos/sphi…

42d11f5

…nx-hoverxref into humitos/support-intersphinx

Typo on link

35594cd

astrojuanlu reviewed May 25, 2021

View reviewed changes

Make a list() because it can't pickle .keys()

c0f6cb2

TypeError: can't pickle dict_keys objects

humitos mentioned this pull request May 25, 2021

Nicer error when the request produces 404 #132

Open

chrizzFTD added a commit to thegrill/grill that referenced this pull request May 28, 2021

try explicit hoverxref_intersphinx_type from readthedocs/sphinx-hover…

1e0806e

…xref#86

humitos added 5 commits June 1, 2021 10:31

Allow to define what interpshinx mappings have hoverxref enabled

0a28a0a

Use Sphinx's internals to check if the object requested belongs to one of the inventories defined by the user under ``hoverxref_interpshinx`` and add hoverxref on the node based on that.

Docs' typo

1bbb002

Define the tooltip type/style per intersphinx mapping

bdc4431

Configure our documentation with intersphinx

277f801

Test cases for intersphinx support

9f4f46e

humitos added 4 commits June 1, 2021 11:24

Define project/version at docs' conf.py from testcase

d3e9348

Variable typo

ce8fd61

Make tests work with Sphinx 4.x

b96f217

Merge remote-tracking branch 'origin/master' into humitos/support-int…

8b78377

…ersphinx

humitos mentioned this pull request Jun 1, 2021

Figure it out a good way to remove title= attribute on intersphinx nodes #133

Closed

ericholscher approved these changes Jun 3, 2021

View reviewed changes

humitos and others added 2 commits June 7, 2021 11:18

Update docs/configuration.rst

26c1476

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

Allow to specify the tooltip's type for each intersphinx role

6ca6dfd

Similiar to `hoverxref_types` but for each intersphinx mapping.

humitos merged commit 90fdb72 into master Jun 7, 2021

humitos deleted the humitos/support-intersphinx branch June 7, 2021 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intersphinx support #86

Intersphinx support #86

humitos commented Aug 20, 2020 •

edited

Loading

schandrika commented Nov 7, 2020

humitos commented Nov 9, 2020

humitos commented May 5, 2021

ericholscher left a comment

ericholscher May 19, 2021

humitos May 20, 2021

ericholscher May 19, 2021

humitos May 20, 2021 •

edited

Loading

humitos May 20, 2021

ericholscher May 20, 2021

astrojuanlu May 25, 2021

humitos May 25, 2021

humitos May 25, 2021

astrojuanlu commented May 25, 2021

chrizzFTD commented May 28, 2021

humitos commented Jun 1, 2021

humitos commented Jun 1, 2021

astrojuanlu commented Jun 1, 2021

chrizzFTD commented Jun 1, 2021

humitos commented Jun 1, 2021

chrizzFTD commented Jun 2, 2021

ericholscher left a comment

ericholscher Jun 3, 2021

humitos Jun 7, 2021

humitos commented Jun 7, 2021

chrizzFTD commented Jun 10, 2021

		@@ -139,6 +140,57 @@ def setup_sphinx_tabs(app, config):
		app.disconnect(listener_id)


		def setup_intersphinx(app, config):

Intersphinx support #86

Intersphinx support #86

Conversation

humitos commented Aug 20, 2020 • edited Loading

How it works?

Small example

ToDo

schandrika commented Nov 7, 2020

humitos commented Nov 9, 2020

humitos commented May 5, 2021

ericholscher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos May 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astrojuanlu commented May 25, 2021

chrizzFTD commented May 28, 2021

Where to look in my docs

Current build log

Current configuration

How to debug?

humitos commented Jun 1, 2021

humitos commented Jun 1, 2021

astrojuanlu commented Jun 1, 2021

chrizzFTD commented Jun 1, 2021

humitos commented Jun 1, 2021

chrizzFTD commented Jun 2, 2021

ericholscher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos commented Jun 7, 2021

chrizzFTD commented Jun 10, 2021

humitos commented Aug 20, 2020 •

edited

Loading

humitos May 20, 2021 •

edited

Loading