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

Don't reverse names in get_qualification #540

Merged

Conversation

martinus
Copy link
Contributor

@martinus martinus commented Jun 6, 2020

E.g. a function a::b::c::foo() is otherwise incorrectly displayed at c::b::a::foo().

Fixes #539.

@vermeeren vermeeren added the bug Problem in existing code label Jun 6, 2020
@vermeeren
Copy link
Collaborator

@jakobandersen Could you confirm this is the correct thing to do? To me it looks good, but perhaps this is pointing at another bug elsewhere regarding some assumptions in the data received?

@jakobandersen
Copy link
Collaborator

Good find! However the suggested fix would break other cases. See the code comments.

E.g. a function a::b::c::foo() is otherwise incorrectly displayed at c::b::a::foo()
@martinus martinus force-pushed the fix-reversed-nested-namespaces branch from 898b405 to 0618acc Compare June 7, 2020 09:25
@martinus
Copy link
Contributor Author

martinus commented Jun 7, 2020

Thanks, I've updated the PR as you suggested

@jakobandersen
Copy link
Collaborator

Nice. I think the PR is ok for merging, and the fix is semantically correct, but I actually can't immediately construct an example right now that distinguishes it from the original PR.
At some point it may be worth revisiting the whole XML node stack logic, though probably not before the PHP domain support is updated/removed.

@vermeeren vermeeren self-requested a review June 7, 2020 12:39
@vermeeren vermeeren self-assigned this Jun 7, 2020
@vermeeren vermeeren merged commit 07826d8 into breathe-doc:master Jun 7, 2020
@vermeeren vermeeren added the code Source code label Jun 7, 2020
vermeeren added a commit that referenced this pull request Jun 7, 2020
@vermeeren
Copy link
Collaborator

Just released v4.19.0 with the fix, all should be good now.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nested namespaces wrongly ordered
3 participants