Make hidden toctrees append their content after everything else in their section

[eric-wieser]

Fundamentally, this is a workaround to the fact that the following is not possible in rst

Heading 1
=======

Content under heading 1

Heading 2
----------

Content under heading 2

.. something magic

Content under heading 1 again

.. toctree::                                                                    
   :hidden:                                                                     

   heading3
   heading4

In particular, it seems to be a common thing to want that last piece of content to be a hidden toctree linking to other pages:

http://stackoverflow.com/q/25276415/102441 http://stackoverflow.com/q/39110429/102441

Right now, the above produces

• Heading 1
  • Heading 2
    • Heading 3
    • Heading 4

It doesn't seem acceptable to change the behaviour of visible toctrees, but for users of hidden toctrees, this was almost certainly intended behaviour anyway. This makes the following produce the intended TOC tree, with the headings in order

Heading 1
=======

Content under heading 1

.. toctree::                                                                    
   :hidden:                                                                     

   heading3
   heading4

Heading 2
----------

Content under heading 2

which before this patch gives

• heading 1
  • heading 3
  • heading 4
  • heading 2

and after this patch gives

• heading 1
  • heading 2
  • heading 3
  • heading 4

Not clear to me whether this is a feature or a bug fix.

If changing the behaviour of :hidden: is not ok, would you prefer adding a :show-last: field?

[eric-wieser]

Test failure is garbage

[enkore]

This would be really neat.

[mattbator]

I would also like to see this workaround merged

[eric-wieser]

Does that mean this has been implemented?

[enkore]

I didn't find anything in the commit or change logs.

[tk0miya]

Sorry, It was my mistake. I just removed stable branch last night... I reopened this again and change target branch to 1.7 (we decided to rename stable branch to versioned one). Thanks,

[eric-wieser]

@tk0miya: Interesting - does the PR close itself when you delete its target branch?

#3584 may have been similarly affected

[tk0miya]

Yes, it looks like that.

[eric-wieser]

I'm obviously not involved enough to make a judgement here, but I'm unsure if that's a good idea - it means that every time you do a release, you need to find all the old PRs and update their target branch to make them useful, or at least remind their owners to.

Is there a way to search by target branch?

In terms of cleaning up the accidentally-closed PRs like this one, I reckon you can just search by "most recent" in the closed queue, since they'll all be simultaneous.

[eric-wieser]

@tk0miya: Was this closed accidentally again, or are you rejecting this change?

[enkore]

1.7 branch was deleted.

[eric-wieser]

@tk0miya: Retargeted at master. @tk0miya, can you reopen?

Also, it would be great if you could weigh in on the patch itself.

[eric-wieser]

@tk0miya: Ping, can you reopen?

[tk0miya]

Oh, sorry...

[eric-wieser]

I've learnt my lesson - I should target master, not the versioned releases in future

[eric-wieser]

Just rebased this, as I was running into the issue again. Would appreciate some feedback on whether the idea sounds good.

~~If it does, then I'll try to add a test somewhere.~~ There are no tests for :hidden: that I can find, so I'm inclined not to bother.

[topiolli]

+1. Two days into Sphinx and I was immediately bitten by this. If this isn't merged, is there a work-around? To me, an alternative would have been to hide section headings from the toc, but I couldn't find a way to do that automatically.

[sidneycadot]

@tk0miya Any chance this could be reviewed? The PR has been in the pipeline for few years now.

This feature is pretty vital -- the fact that you cannot sanely combine headings and a hidden ::toctree directive on the same page is quite annoying.

[sidneycadot]

@eric-wieser @tk0miya

The patch works as promised for the HTML backend, but there seems to be an ordering issue with the latexpdf backend. Please verify this before merging the patch.

With this patch I can achieve my desired document layout in HTML, but it feels a bit weird to insert the hidden ToC at the top of the page, while the actual ToC entries will be inserted at the end.

In my opinion, it would be cleaner to implement a fix where a ToC can remain at the end (so the order of the rendering eg. in LaTeX will remain linear), but then have the option to raise the ToC level of the toc-tree relative to its current tree depth,

My guess is that the patch proposed by @topiolli tried to do something like that but it was rejected.

Now that I have at least an idea where to look thanks to this PR, I will tinker a bit with the sphinx.environment.adapters.toctree.TocTree to see if I can reach a solution that does this, and is well-behaved both in HTML and PDF.

[sidneycadot]

I toyed around with the approach of PR #8847 (which was rejected), hoping that this would behave nicer in PDF. However, there too, the Latex-rendered version doesn't pick up on the re-ordering that happens in the patched adapter TocTree.

It is as if LaTeX decides on a document order for its ToC before the adapter.TocTree has run. Really strange.

Anyway, this shows that any patch addressing this will need to check how non-HTML backends fare with them.

Also, I prefer the basic approach of #8847 as it doesn't require me to put the toctree directive in an 'unnatural' place (top of the page). But at this point, I'd be happy with any sanctioned approach that can be made to work, warts and all.

[sidneycadot]

Seems like something deeper is going wrong. The LaTeX ToC isn't properly updated, but the same is true for a HTML ToC, it seems; they both go wrong in much the same way.

The only thing that /does/ get properly updated is the HTML ToC pane.

[topiolli]

Would you mind taking a fresh look at #8847? It tackles the same problem but doesn't change existing semantics of :hidden:.

[sidneycadot]

@lsaffre @boypeet you both approved this PR; but there is a known issue with at least the LateX backend (see https://github.com/sphinx-doc/sphinx/pull/3622#issuecomment-848804244).

Much as I like a good solution to the issue at hand (being able to combine headings and toctrees on the same page with some level of usefullness) it would not be a good idea to merge something into master that doesn't properly work with a major backend.

[lsaffre]

Yes, wow, seems that the issue is more complex than I thought when I approved. I don't use the LaTeX builder, so I cannot comment further.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 0.00%. Comparing base (99f9209) to head (d0e2ef2). Report is 428 commits behind head on master.

:exclamation: Current head d0e2ef2 differs from pull request most recent head 568fc63. Consider uploading reports for the commit 568fc63 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #3622       +/-   ##
==========================================
- Coverage   86.99%       0   -87.00%     
==========================================
  Files         276       0      -276     
  Lines       52445       0    -52445     
  Branches     9206       0     -9206     
==========================================
- Hits        45627       0    -45627     
+ Misses       5163       0     -5163     
+ Partials     1655       0     -1655     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[AA-Turner]

@eric-wieser -- I added a test, though I'm not entirely sure it captures what you wrote (I have had to use .parent.parent.parent rather than .parent). Please would you be able to check?

A