Skip to content

[doc] Add autolabels so intersphinx can link to anchors like index#install #1787

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

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

kinow
Copy link
Member

@kinow kinow commented Jan 24, 2023

Related to common-workflow-language/user_guide#334

The HTML build of Sphinx creates an inverted list of links (like a hashmap? or a dict), which can be used by extensions like intersphinx for linking between Sphinx sites.

In the issue linked, it is not possible to link to the cwltool installation anchor section, as it's not included in the inverted objects list (test with python -m sphinx.ext.intersphinx https://cwltool.readthedocs.io/en/latest/objects.inv).

With this change, the anchor items must be unique (I believe they are all unique at the moment, no errors after this change), and they are included in the .inv file during the HTML build, and the linked PR can successfully use it as a link.

Cheers
Bruno

@@ -55,3 +55,6 @@ value

# Folder created when using make
cwltool_deps
docs/_build/
docs/autoapi/
Copy link
Member Author

Choose a reason for hiding this comment

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

Annoyed by these files that appeared in my git status while testing this PR. Happy to drop this if it's better 👍

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #1787 (aae2544) into main (dbc4c4c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1787   +/-   ##
=======================================
  Coverage   83.19%   83.19%           
=======================================
  Files          44       44           
  Lines        8130     8130           
  Branches     2224     2224           
=======================================
  Hits         6764     6764           
  Misses        874      874           
  Partials      492      492           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kinow kinow changed the title Add autolabels so intersphinx can link to anchors like index#install [doc] Add autolabels so intersphinx can link to anchors like index#install Jan 24, 2023
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

This adds over 100 warnings about duplicate labels; maybe it needs to be configured to not touch the autoapi pages?

https://readthedocs.org/projects/cwltool/builds/19256779/

@kinow
Copy link
Member Author

kinow commented Jan 24, 2023

This adds over 100 warnings about duplicate labels; maybe it needs to be configured to not touch the autoapi pages?

https://readthedocs.org/projects/cwltool/builds/19256779/

Ah, spoke too fast, sorry @mr-c. Was looking at the user_guide & cwltool logs and I think I didn't see these warnings. Let me see what to do about them...

@kinow kinow marked this pull request as draft January 24, 2023 12:38
@kinow
Copy link
Member Author

kinow commented Jan 24, 2023

Phew... heaps of warnings (cc @Mackenzie-OO7 see?! I told that happens a lot with me 😆 ).

maybe it needs to be configured to not touch the autoapi pages?

Thanks for the suggestion @mr-c, sounds like the simplest approach (or find another way to create intersphinx links without the autolabel).

@@ -46,6 +47,8 @@
"sphinxcontrib.autoprogram",
]

autosectionlabel_prefix_document = True
Copy link
Member Author

Choose a reason for hiding this comment

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

This adds prefixes to each label, using the document name, reducing risks of conflicts.

https://www.sphinx-doc.org/en/master/usage/extensions/autosectionlabel.html

@kinow kinow marked this pull request as ready for review January 24, 2023 16:46
@mr-c mr-c merged commit 8978ca2 into common-workflow-language:main Jan 24, 2023
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.

2 participants