-
-
Notifications
You must be signed in to change notification settings - Fork 199
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
Pass function CV-qualifiers to Sphinx #247
Conversation
Previously they were added to the rendered output manually. Without CV-qualifiers Sphinx cpp domain generates incorrect IDs because of incomplete function signatures (#246).
I'll try to check this over tomorrow if you'd like a second pair of eyes but I more than trust you if you want to merge. |
Switch to Sphinx 1.4 is a fairly significant change, so I'd better wait till you have a chance to look at it. |
Hey, the diffs look good to me. The code works with 1.4. There are some points where we get better code highlighting in the code blocks and some point where it seem perhaps worse? Keywords like 'if' and 'for' are now being highlighted in some of the code examples on the tinyxml test page. But the class name and function are no longer highlighted and linked in the first code example on the doxygen test page. Though that latter example is less than ideally fragmented to multiple blocks in both cases anyway. All in all very happy for you to merge. Your discovery and use of the NodeVisitor is really excellent. Feels much cleaner than the indexing we had before. |
Thanks for reviewing, merged. Are you talking about this page and which classes and functions are not linked? |
Pass function CV-qualifiers to Sphinx
This PR fixes #246 by changing the way function CV-qualifiers are handled. Instead of adding them to the rendered Sphinx nodes, they are added to a function signature which is passed to the Sphinx cpp domain. This way proper IDs are generated which is tested using the new test infrastructure (yay!)
This won't work with Sphinx 1.3 or older, so I changed the required version to 1.4 as discussed in #243.