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.

Sign up for GitHub

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

Jump to bottom

Make toctree accept special docnames #10673

Merged
merged 5 commits into from Sep 12, 2022
Merged

Make toctree accept special docnames #10673

merged 5 commits into from Sep 12, 2022

Conversation

brechtm
Copy link
Contributor

@brechtm brechtm commented Jul 16, 2022

Subject: Make toctree accept 'genindex', 'modindex' and 'search' docnames

Feature or Bugfix

Bugfix

Relates

Fixes #8438

@brechtm brechtm mentioned this pull request Jul 16, 2022
Closed
@brechtm
Copy link
Contributor Author

brechtm commented Jul 16, 2022

The PR works when building a project, but there's obviously still something missing. I'll need to look into the failing tests...

@brechtm brechtm force-pushed the toctree-index branch 3 times, most recently from 8497ffc to e6bf886 Compare July 18, 2022 23:10
@brechtm
Copy link
Contributor Author

brechtm commented Jul 18, 2022

I have some questions on how to continue with this:

  1. Should this target master or 0.5?
  2. mypy: sphinx/environment/adapters/toctree.py:142:51: error: "Domain" has no attribute "labels"; maybe "label"? [attr-defined]
    Domain definitely has a labels attribute, but perhaps not at object initialization time. Can this be somehow flagged for mypy to ignore this?

jbms
jbms reviewed Jul 26, 2022
View reviewed changes
sphinx/directives/other.py Outdated
@@ -118,7 +119,7 @@ def parse_content(self, toctree: addnodes.toctree) -> List[Node]:
docname = docname_join(self.env.docname, docname)
if url_re.match(ref) or ref == 'self':
toctree['entries'].append((title, ref))
elif docname not in self.env.found_docs:
elif docname not in (self.env.found_docs | generated_documents):
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to move this union computation out of the loop here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@brechtm
Copy link
Contributor Author

brechtm commented Sep 8, 2022 •
edited

Is there anything I can do at this point? Thanks!

@n-peugnet n-peugnet mentioned this pull request Sep 8, 2022
Merged
AA-Turner
AA-Turner approved these changes Sep 8, 2022
View reviewed changes
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Please add an entry to CHANGES.

A

sphinx/environment/adapters/toctree.py Outdated Show resolved Hide resolved
sphinx/environment/adapters/toctree.py Outdated Show resolved Hide resolved
sphinx/environment/collectors/toctree.py Outdated Show resolved Hide resolved
@AA-Turner
Copy link
Member

mypy: sphinx/environment/adapters/toctree.py:142:51: error: "Domain" has no attribute "labels"; maybe "label"? [attr-defined]

Domain definitely has a labels attribute, but perhaps not at object initialization time. Can this be somehow flagged for mypy to ignore this?

Add # type: ignore[attr-defined] as a quick fix.

A

@AA-Turner AA-Turner added this to the 5.2.0 milestone Sep 8, 2022
@AA-Turner AA-Turner added type:enhancement enhance or introduce a new feature internals:toctree labels Sep 8, 2022
@brechtm brechtm force-pushed the toctree-index branch 2 times, most recently from 669984b to af1570b Compare September 8, 2022 18:30
@AA-Turner
Copy link
Member

Looks good, I updated the implementation so as not to hard-code the "special" in more places. The only open question is the glob behaviour (see linked issue).

A

@AA-Turner AA-Turner changed the title Make toctree accept 'genindex', 'modindex' and 'search' docnames Make toctree accept special docnames Sep 12, 2022
@AA-Turner AA-Turner merged commit d924acf into sphinx-doc:5.x Sep 12, 2022
@AA-Turner
Copy link
Member

Thanks @brechtm!

A

@brechtm
Copy link
Contributor Author

brechtm commented Sep 15, 2022

Many thanks, @AA-Turner, for bringing this to completion!

@AA-Turner AA-Turner mentioned this pull request Oct 5, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jbms jbms jbms left review comments

@AA-Turner AA-Turner AA-Turner approved these changes

Assignees
No one assigned
Labels
internals:toctree type:enhancement enhance or introduce a new feature
Projects
None yet
Milestone
5.2.0
Development

Successfully merging this pull request may close these issues.

toctree contains reference to nonexisting document 'genindex', 'modindex', 'search'
3 participants
@brechtm @AA-Turner @jbms