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

[Bug]: Headlines from includes not included in contents menu #1052

Open
linawolf opened this issue Jul 28, 2024 · 5 comments
Open

[Bug]: Headlines from includes not included in contents menu #1052

linawolf opened this issue Jul 28, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@linawolf
Copy link
Contributor

Summary

..  contents:: Table of Contents:

..  include:: _Part1.rst.txt
..  include:: _Part2.rst.txt

Code snippet that reproduces the problem

No response

Expected output

Headlines within the parts should appear in contents menu

@linawolf linawolf added bug Something isn't working triage labels Jul 28, 2024
@jaapio jaapio self-assigned this Aug 29, 2024
@jaapio jaapio removed the triage label Aug 29, 2024
@jaapio
Copy link
Member

jaapio commented Aug 29, 2024

I checked, this example doesn't work. however when we use:

=========
Document
=========

..  contents:: Table of Contents:

..  include:: _Part1.rst.txt
..  include:: _Part2.rst.txt

The contents table is filled.

@jaapio
Copy link
Member

jaapio commented Aug 29, 2024

I think I found the origin of this issue. In your example the document doesn't have a main section with a title. The includes are added as a collection of nodes to the document which is perfectly fine. However the getTitle method on the document does the assumption that each document has sections with titles.

I think the best approach is to rewrite this function to a simple get. Which will return a property title from the document. This property will be populated by a new compiler pass which has the knowledge of the items a document may contain to find the correct title of a document. I'm not aware of a situation that could replace the title. But that will be an option this way as well.

@jaapio
Copy link
Member

jaapio commented Aug 29, 2024

Rethinking the above... that could cause invalid cases... getTitle should be dynamic, I will change the method to be aware of containers.

@jaapio
Copy link
Member

jaapio commented Sep 3, 2024

I noticed while working on this that the actual issue is a bit deeper... The collection node does not work as expected when it's just for an include. Multiple situations are not working anymore when sections of a document are in a collection. So I'm gonna take a new approach in this by unpacking the collection in the compiler.

@jaapio
Copy link
Member

jaapio commented Sep 3, 2024

I'm going to write down my thoughts about the issue regarding the includes. Which is a quite complicated topic, and seems to be the root cause of this issue.

Right now includes are parsed as part of a normal document, but with some kind of sub parser using the same parser context as the current document. However it will return a include a documentnode which is unpacked in the directive handler and all children are put into a CollectionNode. In the basis this works perfect. However the rest of the application does make some assumptions.

  1. a document has just 1 root section, all other sections are children of this node.
  2. A document's first title is the title of a document, same level titles are ignored
  3. at first titles and sections are always a flat list when a document is parsed, we do create a tree of sections in the compiler.

However when using includes we can create a set of nodes that does not follow those rules. Especially when we are using includes to add sections to a document like in the reported example.

In a normal situation the parser would produce:

  • section
  • title
  • section
  • title

after compilation we have a tree like this:

  • section
    • title
    • section
      • title

However when we use includes we can have a parser result like this:

  • section
  • title
    • collection
      • section
      • section

This does not result in issues when we focus on correct rendering. However when we are building a menu structure this can get more complicated. Especially when the included document does not only provide sections but also other body elements. Like a sidebar or footer element, those elements are most likely not positioned in the correct location.

This gets even worse when we have includes that are nested in tables or other compound nodes. That would result in a section in a table?

My first idea was to simply look for sections in a collection to find the title of a document when the first node is a collection. This does solve part of the described issue. Documents do get a title which makes sure they show up in the table of contents. Sections from the include are also showing up in the TOC. No real deal.

However this does not work when the include is part of a section:

.. include:: parts1.rst.txt

Subsection
----------

.. include:: parts2.rst.txt

In this case the sections defined in parts1 and parts2 do show up in the TOC, but subsection is gone. I think this is caused by some logic in the compiler that does not handle the document in the correct way, as the first section is seen as the title of the document. Which is not true here. The first title comes from parts1 And is not subsection But that only applies when parts1 is actually containing 1 section. if there are multiple other issues might pop up.


In short, I think this is a very complex system. And we need some more context knowledge when we are unpacking the includes. Ideally an include would just result in an raw include of the nodes produced by the parser. but as the nodes produced by an include may also contain root-level only elements like a sidebar or footer it will never work in all situation. I assume includes producing multiple nodes especially in more complex situations like tables will never work very well and there will always be cases where includes will break the rendered document structure.

The sort term solution could be to make it work for the reported case. Ignore all others, and accept that there will be more dragons on this path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants