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

Intersphinx references work without a namespace/identifier but fail if used #9682

Open
umarcor opened this issue Sep 27, 2021 · 13 comments
Open

Comments

@umarcor
Copy link

umarcor commented Sep 27, 2021

Describe the bug

We are trying to cross-reference several sites built with Sphinx. We found that some references are properly resolved, but others fail. Precisely, if we reference labels in other sites without specifying the namespace/identifier, it does work. However, that is ambiguous, because multiple sites might have the same label. Unfortunately, when using the namespace/identifier, references are not resolved. To our surprise, that is not consistent. It works for some sites, but not for others.

I'm not sure about this being a bug or a misconfiguration of some of the sites.

How to Reproduce

$ git clone https://github.com/edaa-org/edaa-org.github.io
$ cd edaa-org.github.io
$ git clone -b v1.0.0-rc1 https://github.com/buildthedocs/sphinx.theme _theme
$ make html
$ # open _build/html/index and see "Conceptual Model > 3 | Language Model".

As seen, refs to python or OSVB namespaces/identifiers are resolved, but references to pyVHDLModel or pySystemVerilogModel do fail.

Expected behavior

Refs to pyVHDLModel or pySystemVerilogModel Glossary labels should work as python:comparisons or OSVB:API:Core.

Your project

https://github.com/edaa-org/edaa-org.github.io

Screenshots

image

OS

Linux and MSYS2

Python version

3.9

Sphinx version

4.2

Sphinx extensions

https://github.com/edaa-org/edaa-org.github.io/blob/19f14102835c2fc1224363ee46296058c06a1753/conf.py#L168-L172

Extra tools

No response

Additional context

Same issue in VHDL/pyVHDLModel#32.

/cc @Paebbels

@umarcor
Copy link
Author

umarcor commented Sep 27, 2021

Upon further testing, this issue seems rather contrived:

  • If the intersphinx mapping identifier starts with py, it seems that none of the references to that namespace will work. Is it possible that value[0:2] == 'py' is compared somewhere in the code without checking for the length of value?

  • If the intersphinx mapping identifier is lowercase:

    • The identifier can be used in role doc in lowercase only.
    • In role ref, it does not matter if the identifier is uppercase, lowercase or camelcase.
  • If the intersphinx mapping identifier is camelcase, all roles fail.

@astrojuanlu
Copy link
Contributor

Not sure if related to the problems that #9459 hopefully addresses

@umarcor
Copy link
Author

umarcor commented Sep 29, 2021

@astrojuanlu I gave #9459 a try. Unfortunately, none of the issues seems to be fixed consistently. Problems with 'py' and CamelCase remain.

@jakobandersen
Copy link
Contributor

Just to be sure, this is not in the default master branch but in umarcor/cross-refs, right?

@umarcor
Copy link
Author

umarcor commented Oct 2, 2021

@jakobandersen, that is correct. I found a workaround in main, but I created umarcor/cross-refs as a reproducer of this issue. I'm sorry about not notifying it explicitly here.

@jakobandersen
Copy link
Contributor

No problem, I'll see what I can find out.

@jakobandersen
Copy link
Contributor

Hmm, this is icky. As you also got towards: I think culprit is an unintended interaction between the way intersphinx repositories are specified and how the ref role is defined. It is defined to convert its target to lower case:

'ref': XRefRole(lowercase=True, innernodeclass=nodes.inline,

But your intersphinx_mapping uses mixed case (which I think is reasonable). If you change the inventory names to lower case the goals will work.
However, the Glossary references are not labels in the remote inventories, but documents, so the doc role should be used. But changing to that role does not lower-case its argument, so the mixed-case inventory name now doesn't work, but changing them in the :doc: to lower case makes them work.
So the changes are:
in conf.py:

intersphinx_mapping = { 
    'python':    ('https://docs.python.org/3', None),
    'osvb':      ('https://umarcor.github.io/osvb', None),
    'pyvhdlmodel': ('https://vhdl.github.io/pyVHDLModel', None),
    'pysystemverilogmodel':   ('https://edaa-org.github.io/pySystemVerilogModel', None),
}

and in ConceptualModel.rst:

    * ``Glossary``                               :doc:`Glossary`
    * ``pyVHDLModel:Glossary``                   :doc:`pyvhdlmodel:Glossary`
    * ``pySystemVerilogModel:Glossary``          :doc:`pysystemverilogmodel:Glossary`

But the very last reference, :ref:`pySystemVerilogModel:vhdlmodel` , I can't find that in that inventory (you can use python3 -m sphinx.ext.intersphinx https://edaa-org.github.io/pySystemVerilogModel/objects.inv to get a textual output of it).

This is a bit of a mess with how the inventory specifications are specified. We are sort-of in a process of deprecating that syntax and adding a dedicated role for these cases #9062, which should sort most of this out. That is, don't mess with the case of the inventory name.
I hope we can get it ready for the next minor version, 4.3, but there are a few other things to sort out as well (#9459).

@Paebbels
Copy link

Paebbels commented Oct 2, 2021

@jakobandersen so to summarize:

  • Currently Sphinx / Intersphinx is a bit messy about lower and mixed cases, so the user gets a strange user experience when and where identifiers get change / need to be be changed compared how they where defined, right?
  • It's planned to fix this in future (maybe in 4.3.x) so the user experience can be great again?

@jakobandersen
Copy link
Contributor

Basically yes.
I'm not sure exactly when this weird behaviour has started, it may be been there all the time.
Perhaps it could be fixed in the current scheme, but I don't have any good ideas that doesn't risk breaking other cases, hence my suggestion to fix it with the new role. Maybe someone else has a good idea.

@umarcor
Copy link
Author

umarcor commented Oct 2, 2021

@jakobandersen, thanks so much for looking into this!

I updated the branch to better use ref and doc. This is the result with CamelCase identifiers:

image

And this is the result with lowercase identifiers:

image

It seems that doc might search for lowercase identifiers if the exact match (CamelCase) is not found. That would force the identifiers in the sphinx mapping to be lowercase, but users would be free to use any casing in the text.

@Paebbels
Copy link

Any updates on this?

@jakobandersen
Copy link
Contributor

Any updates on this?

Not much. We need #9459 merged first (which I forgot to re-request review for). Hopefully that can be done by next weekend where 4.3 is scheduled for. Then #9062 can be updated which will provide the new intersphinx role.

@jakobandersen
Copy link
Contributor

There is now #9822, which implements the new role. I tried to use it to use it in your example and got to something like

3 | Language Model
    Syntax/design Document Object Model (DOM) of the language(s).

    * ``comparison manual <python:comparisons>`` :external:python+ref:`comparison manual <comparisons>`
    * ``osvb:API:Core``                          :external:osvb+ref:`API:Core`
    * ``API:Core``                               :ref:`API:Core`
    * ``goals``                                  :ref:`goals`
    * ``pyvhdlmodel:goals``                      :external:pyvhdlmodel+ref:`goals`
    * ``pyVHDLModel:goals``                      :external:pyVHDLModel+ref:`goals`
    * ``Title <pyVHDLModel:goals>``              :external:pyVHDLModel+ref:`Title <goals>`
    * ``Glossary``                               :ref:`Glossary`
    * ``pyVHDLModel:glossary``                   :external:pyVHDLModel+ref:`glossary`
    * ``pySystemVerilogModel:Glossary``          :external:pySystemVerilogModel+ref:`Glossary`
    * ``vhdlmodel``                              :ref:`vhdlmodel`
    * ``pyVHDLModel:vhdlmodel``                  :external:pyVHDLModel+ref:`vhdlmodel`
    * ``pySystemVerilogModel:vhdlmodel``         :external:pySystemVerilogModel+ref:`vhdlmodel`

This works without lower-casing the intersphinx_mappings keys. Some of these references don't seem to exist, but maybe I just didn't adapt the example properly.
It would be great if you could try it out and see if it fulfills your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants