avoid duplicate label warnings for .. include::'ed files

[clalancette]

Sphinx does not like it when a file that is later included
(via .. include directive) has the very same link label inside of
it. To avoid a Sphinx warning, skip link label generation for
the pageindex.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

[hidmic]

LGTM though a comment explaining why the special case is needed would be great.

@svenevs thoughts?

[clalancette]

LGTM though a comment explaining why the special case is needed would be great.

Good call, done in 21ff1af

[svenevs]

I'm in the process of re-tooling a new OS, but not having a link may present issues with the tree view links (when using rst lists). My impression is one of the tests cases for pages should produce the warning we are discussing here, I think it's better to stuff exhale_ in front of the node.link_name and keep the link_declaration as is.

exhale/exhale/graph.py

Line 2435 in 0d1a8a8

node.link_name = "exhale_{kind}_{id}".format(kind=node.kind, id=unique_id)

"page" is in the SPECIAL_CASES above there, I don't see any reason why stuffing exhale_ in front of everything is a problem (e.g., before the if entirely), the main split there is for adjusting the generated file title. There's an issue on windows where you can end up generating filenames that are too long or something iirc. Going to be looking into this later this evening (I use spack to manage things, it takes a number of hours to get everything setup the way I want...).

[svenevs]

Turned out to be a slightly different problem, uncovered by the duplicate label. The index page is the first one that is being included that had a label that was therefore being duplicated, good catch! I've pushed some revert commits (and rebased for the updated testing matrix) with the final commit being the proposed fix. Anything that is .. include::'ed should now have a suffix .rst.include, special-casing for node.refid == "indexpage" in the filename component. Please confirm this eradicates the warnings for you.

tox -e py -- -s -k test_hierarchies_primary_mainpage and add self.app.build() to testing/tests/cpp_nesting.py:CPPNestingPages:test_hierarchies_primary_mainpage. There's a lot of other warnings there, but on master it's the first warning that comes up.

If things are working as expected I'll squash this soon 🙃

[svenevs]

Merging as resolved, the warning is gone for me. Can revisit in followup if it is still a problem, preventing sphinx from processing the documents twice independently (once for the document it is .. include::ed in and once for the document itself) should probably reduce number of warnings and/or increase compile times (e.g., not re-processing the toctree links twice for every file in the generated api). Probably insignificant savings though

[IceflowRE]

I have some questions about this.
For our project this is a breaking change.
We used those files directly in a toctree. Which is not possible anymore due to the .include suffix. Is it still possible to do this?

[svenevs]

Opened #176 to discuss, it should be easy to have our cake and eat it too 🙂