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 support for content-only flag when rendering pages #645

Merged
merged 2 commits into from
Mar 29, 2021

Conversation

gmarull
Copy link
Contributor

@gmarull gmarull commented Feb 22, 2021

Support for pages was added in #596. This PR adds support for the content-only flag when rendering pages, so that when creating an rst file with only

.. doxygenpage: test
    :content-only:

the entire page is rendered as if it was a native rst page.

Note that this is my first contribution to breathe, so expect a non-optimal/wrong solution. Suggestions are welcome :-)

@carlescufi
Copy link

cc @utzig

Indicate support for the content-only option.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Copy link
Contributor

@utzig utzig left a comment

Choose a reason for hiding this comment

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

LGTM. One interesting change is that adding :content-only: creates a new section with the title from the \page element, which IMO is correct and was not done previously. Would be interesting to eventually also update non :content-only: to also add this section.

@utzig
Copy link
Contributor

utzig commented Feb 26, 2021

LGTM. One interesting change is that adding :content-only: creates a new section with the title from the \page element, which IMO is correct and was not done previously. Would be interesting to eventually also update non :content-only: to also add this section.

Btw, I tested a previously fetched version from a few days ago and missed that you force pushed a change, so now it seems you made it consistent without the title; I personally liked the version that had the title extracted from the \page, although having the same behavior with or without the setting seems like a better solution.

@gmarull
Copy link
Contributor Author

gmarull commented Mar 1, 2021

@utzig the reason I removed title inclusion is because in some occasions, it is helpful to include content between title and the actual content, e.g. to insert a toctree to reference doxygen subpages and so create the same tree structure as in doxygen.

@gmarull
Copy link
Contributor Author

gmarull commented Mar 8, 2021

@vermeeren can you review? thanks

@gmarull
Copy link
Contributor Author

gmarull commented Mar 15, 2021

@michaeljones can you review? thanks

@vermeeren
Copy link
Collaborator

@gmarull (and others) I should be able to get to all pending reviews and issues this week, some unexpected events ended up taking a significant amount of time in the past month.

@vermeeren vermeeren self-requested a review March 29, 2021 00:00
@vermeeren vermeeren self-assigned this Mar 29, 2021
@vermeeren vermeeren added code Source code enhancement Improvements, additions (also cosmetics) labels Mar 29, 2021
Copy link
Collaborator

@vermeeren vermeeren left a comment

Choose a reason for hiding this comment

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

@gmarull Looks good, thanks for the patches!

michaeljones pushed a commit that referenced this pull request Mar 29, 2021
@michaeljones michaeljones merged commit ef57deb into breathe-doc:master Mar 29, 2021
@gmarull gmarull deleted the feature/content-only-pages branch March 30, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Source code enhancement Improvements, additions (also cosmetics)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants