Add support for innergroup #556
Conversation
utzig
force-pushed
the
add-innergroup-support
branch
from
40845d6
to
3e36e59
Compare
utzig
changed the title
utzig
changed the title
|
I'm not sure if I am missing something for a review to happen, but would be glad to get some feedback! |
|
I will try to review as soon as I can, a bit busy right now as a side effect of having been sick for a while. The same applies to other issues and PRs in the past week and a half or so. Sorry for the delay. |
|
As a first pass it looks good to me, nice work / good addition! We need a test / example case for this, can you please add a simple innergroup example / make sure to include it in the sphinx docs? https://github.com/michaeljones/breathe/tree/master/examples That will help us confirm it works as desired, and also show users how to use it. I would like to help review this further for @vermeeren (feel better soon! ❤️) but I don't really have any experience with doxygen groups... The example will help me confirm it works correctly 🙂 |
| @@ -1024,6 +1024,12 @@ def render_derivedcompoundref(node): | |||
| addnode('innerclass', lambda: self.render_iterable(node.innerclass)) | |||
| addnode('innernamespace', lambda: self.render_iterable(node.innernamespace)) | |||
|
|
|||
| if 'inner' in options: | |||
| for node in node.innergroup: | |||
There was a problem hiding this comment.
In the event that inner is requested but there are no inner groups, should we emit a warning along the lines of inner groups requested for group {group name} but no inner groups found!?
There was a problem hiding this comment.
Sure, and there's also the other way around. If there are innergroups but no inner keyword was given in the options. To preserve compatibility this should obviously not trigger a warning. I am honestly not sure if someone would like the behavior of generating a warning in the case you pointed, but I will assume so and update accordingly. It can always be removed later! Btw, adding an inner in the documentation set I am working on, already triggers lots and lots of warnings because of the double references, because we basically had to create a reference to each group individually due to the lack of innergroup parsing.
There was a problem hiding this comment.
Thinking a bit better about this, turns out that options is part of a "global" context in a sense. So if I have an innergroup B inside another group A, and I set inner, both A and B will be checked for the existence of innergroups. This will end up being an empty list for the innergroup B (assuming it has no innergroups itself!); but it should not trigger a warning.
Thanks Stephen, I added a test: a127be6. An example already exists that creates an innergroup: https://github.com/michaeljones/breathe/blob/master/examples/doxygen/group.cpp#L26. It just so happens that it is currently ignored! I am not exactly sure what you mean by "sphinx docs", is adding this 3e36e59#diff-fb6911bd4f08815018ea2c65e04aa85cR45 not enough? |
utzig
force-pushed
the
add-innergroup-support
branch
from
a127be6
to
df2ba9c
Compare
Doxygen groups that are defined inside another group, either by scope or by using \ingroup, are not automatically added to the documentation unless an explicit reference is created. This commit adds parsing of innergroup elements inside compound defs. When using a group directive, .. doxygengroup:: myGroup, a new option ":inner:" must be added to include all innergroup elements. This allows maintaining compatibility with previous projects. Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
Add :inner: usage example and update group.h header to include a new group inside mygroup. Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
Add a new test to check that an innergroup is parsed when the "inner" keyword is given in the options. Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
utzig
force-pushed
the
add-innergroup-support
branch
from
df2ba9c
to
70fcda1
Compare
I added an example with |
|
@vermeeren Could you take a look? |
|
@vermeeren The official documentation does not yet reflect the changes, is it published automatically? |
|
@utzig There is a problem with the RTD docs being out of sync since quite a while, see #475. I will try asking Michael again and hopefully get a reply, I don't have permission on RTD I think. If this isn't resolved soon I think I will self-host the breathe docs, because it is indeed quite a big problem. |
Leftover from PR breathe-doc#556; updated directives.rst to include :inner: Signed-off-by: Fabio Utzig <fabio.utzig@nordicsemi.no>
Doxygen groups that are defined inside another group, either by scope or by using \ingroup, are not automatically added to the documentation unless an explicit reference is created. This commit adds parsing of innergroup elements inside compound defs.
Fixes #555