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

Add include_rubrics_in_toc object description option #136

Merged
merged 1 commit into from
Jul 23, 2022

Conversation

mhostetter
Copy link
Contributor

Fixes #126. I believe I correctly implemented this feature. Please let me know your thoughts. I tested this locally on my project and on a standalone foo project.

The option is disabled by default. When enabled, all rubrics (that haven't already been converted to fields or admonitions by Napoleon) are added to the TOC. Here's an example in this foo module. foo-example.zip

image

@mhostetter
Copy link
Contributor Author

mhostetter commented Jul 22, 2022

The only slightly non-ideal thing I noticed was that rubrics from the class definition (not the __init__() signature) are included in the class TOC on the left-hand side.

@jbms have any thoughts on how that might be avoided?

I noticed that when you click on the class FLFSR on the left side, the "Notes", "References", and "Examples" TOC entries are not shown on the left. (which I prefer)

image

But when you select an individual method / property, then the "Notes", "References", and "Examples" appear on the left. (which I don't prefer)

image

new_node = _make_section_from_rubric(source)
if new_node is not None:
target += new_node
target = new_node
Copy link
Owner

Choose a reason for hiding this comment

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

No need to assign to target, instead should return as in the term case before since rubrics cannot contain children (because they are just headings, not sections). If you want them to be able to have chosen that would require more work.

@jbms
Copy link
Owner

jbms commented Jul 22, 2022

The only slightly non-ideal thing I noticed was that rubrics from the class definition (not the __init__() signature) are included in the class TOC on the left-hand side.

@jbms have any thoughts on how that might be avoided?

I agree that this is not good. Does this issue occur with all sections, or just rubrics? If it occurs with all sections, the fix will be in nav_adapt.py

@mhostetter
Copy link
Contributor Author

I agree that this is not good. Does this issue occur with all sections, or just rubrics? If it occurs with all sections, the fix will be in nav_adapt.py

By "all sections" do you mean the fields you convert into TOC headings, like "Parameters" and "Returns"? If so, yes. (I'm not sure how else to add section headings in a docstring.)

I just added "Parameters" and "Returns" into the class docstring and they manifest the same way the rubric TOC entries do.

image

@mhostetter mhostetter force-pushed the include-rubrics-in-toc branch from 01368d7 to 3a3fa74 Compare July 22, 2022 14:13
@jbms
Copy link
Owner

jbms commented Jul 22, 2022

Looks like nav_adapt.py needs to be fixed to exclude from the global toc section headings that don't themselves contain other pages as descendants.

@jbms
Copy link
Owner

jbms commented Jul 22, 2022

Thanks, this looks perfect. The issue with the global toc is unrelated so it could be fixed separately I suppose.

@mhostetter
Copy link
Contributor Author

@jbms thanks. I'm looking through the nav_adapt.py code and trying to figure it out. If I get too stuck, I may ask for help.

@mhostetter
Copy link
Contributor Author

Ok... so... there's a lot going on in nav_adapt.py. Perhaps handing the global TOC should be a standalone issue. I'll open a new issue to track that. I'm ready to merge this PR whenever you all are ready. Thanks.

@jbms jbms merged commit 5e70669 into jbms:main Jul 23, 2022
@jbms
Copy link
Owner

jbms commented Jul 23, 2022

Thanks

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.

Add additional fields in python-apigen?
2 participants