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

Fix crash when visiting typedef #547

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

utzig
Copy link
Contributor

@utzig utzig commented Jun 17, 2020

When using the @typedef <name> doxygen command, the generated XML does not have a proper definition, "using ..." or "typedef ..."; this used to work with versions <= 4.16.0 by assuming everything as a typedef. To fix the issue, check if "using " is found, otherwise fall back to considering that "typedef " was used. If not "typedef " is found in the definition, print a warning.

Fixes #534

@utzig
Copy link
Contributor Author

utzig commented Jun 17, 2020

I know this has issues with using ... but I don't see an obvious way to handle it. Also I am not sure about adding a warning at all; also any particular reason why the Sphinx logger isn't used here?

@vermeeren
Copy link
Collaborator

vermeeren commented Jun 17, 2020

I did mess around a bit with something similar a few weeks ago doing some testing, based upon #534 (comment) and of course other comments on the issue, but somehow I forgot to finalise things and turn it into a PR. Thanks for creating one.

Edit:

Also I am not sure about adding a warning at all; also any particular reason why the Sphinx logger isn't used here?

I suspect very old code / legacy stuff, I have been maintaining since 2018 but there is still a lot of voodoo going on that I can't wrap my head around (yet).

@utzig
Copy link
Contributor Author

utzig commented Jun 17, 2020

I did mess around a bit with something similar a few weeks ago doing some testing, based upon #534 (comment) and of course other comments on the issue, but somehow I forgot to finalise things and turn it into a PR. Thanks for creating one.

I was expecting someone to send it, but got tired of waiting! :-)

I suspect very old code / legacy stuff, I have been maintaining since 2018 but there is still a lot of voodoo going on that I can't wrap my head around (yet).

Well, we have 'Sphinx>=3.0,<3.2' as requirements, so it's OK to change those prints in sphinxrenderer.py to use the Sphinx logger then?

@vermeeren
Copy link
Collaborator

Well, we have 'Sphinx>=3.0,<3.2' as requirements, so it's OK to change those prints in sphinxrenderer.py to use the Sphinx logger then?

Yeah that's definitely fine, if I grep a bit it seems that it is already fixed properly for most occurrences of the word warning. Thanks!

@vermeeren
Copy link
Collaborator

@jakobandersen Could you also take a look just in case?

@vermeeren vermeeren self-assigned this Jun 17, 2020
@vermeeren vermeeren added bug Problem in existing code code Source code regression Something broke that worked in the past labels Jun 17, 2020
When using the @typedef <name> doxygen command, the generated XML does
not have a proper definition, "using ..." or "typedef ..."; this used
to work with versions <= 4.16.0 by assuming everything as a typedef. To
fix the issue, check if "using " is found, otherwise fall back to
considering that "typedef " was used. If not "typedef " is found in the
definition, print a warning.

Signed-off-by: Fabio Utzig <[email protected]>
@jakobandersen
Copy link
Collaborator

Looks good to me.

@vermeeren vermeeren merged commit 1828e04 into breathe-doc:master Jun 17, 2020
vermeeren added a commit that referenced this pull request Jun 17, 2020
@utzig utzig deleted the fix-typedef-crash branch June 17, 2020 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem in existing code code Source code regression Something broke that worked in the past
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renderer crash when visiting typedef
3 participants