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

Let Sphinx handle more things #412

Merged
merged 6 commits into from
Feb 5, 2019

Conversation

jakobandersen
Copy link
Collaborator

This PR simplifies some of the code by using more of the (semi-)recent features in core Sphinx. As I only tested it on the Breathe documentation I suggest that some people with more experience with using Breathe double checks the effects of the changes.
The following are the changes I either expect or have observed:

  • Enums and unions are now using the corresponding directives, instead of the type directive.
    This means enumerators are now injected in the parent scope as in C++. I'm not sure what happens with enum structs/classes though.
    It also means that permalinks from before enum/unions were supported in Sphinx probably will break.
  • For enumerators the 'enumerator' prefix is currently removed (see the visit_enumvalue method). This was from before where they use another directive. I suggest removing the update_signature part there, so make enumerators consistent with other directives.
  • Anonymous enums are now using the Sphinx support. It looks like anon structs/classes and unions are not handled in Breathe currently. Name lookup can skip through anon names, as in C++. Permalinks will for sure break.
  • Initializers for variables/enumerators are now parsed by Sphinx, so identifiers will be hyperlinked. (See e.g., the bottom of https://breathe.readthedocs.io/en/latest/tinyxml.html for a missing link in TIXML_ENCODING_UNKNOWN).

@jakobandersen
Copy link
Collaborator Author

Found another commit that could be applied now:

  • Template declaration prefixes, i.e, those starting with template< are now parsed by Sphinx.
    This will fix some/many/most? of the "Too many template argument lists compared to parameter lists" errors.
    When template parameters are used, they will now be hyperlinked.
    This will probably break all permalinks for those declarations.

@vermeeren vermeeren self-assigned this Feb 5, 2019
@vermeeren
Copy link
Collaborator

Sorry for the wait, had been busy. The diff seems good to me, thanks for the patches. Just to make sure: is it ok to merge this right now or are there some further things that need doing prior to doing so?

@jakobandersen
Copy link
Collaborator Author

No problem. Apart from the reservations mentioned I think it is ready to merge.

@vermeeren vermeeren merged commit 722cc45 into breathe-doc:master Feb 5, 2019
vermeeren added a commit that referenced this pull request Feb 5, 2019
@jakobandersen jakobandersen deleted the 1.8-simplifications branch March 18, 2020 09:38
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.

2 participants