Skip to content

Allow noindex option in addition to no-link #1000

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hojalot
Copy link

@hojalot hojalot commented Oct 23, 2024

The Sphinx renderer maps the "no-link" option to "noindex" but a warning is generated since "noindex" is not in option_specs.

Add "noindex" where "no-link" is allowed.

The Sphinx renderer maps the "no-link" option to "noindex" but
a warning is generated since "noindex" is not in option_specs.

Add "noindex" where "no-link" is allowed.
@BrennanGit
Copy link

We've just upgraded our breathe to the new version (thanks @AA-Turner for getting it released).

The PR #959 has introduced a change which means ALL our directives get a toc/index entry now.

An example that shows the change follows:

.. tab:: C API

    .. doxygenfunction:: adsp_volume_control

    .. doxygenfunction:: adsp_volume_control_set_gain

    .. doxygenfunction:: adsp_volume_control_mute

    .. doxygenfunction:: adsp_volume_control_unmute

.. tab:: Python API

    .. autoclass:: audio_dsp.dsp.signal_chain.volume_control
        :noindex:

        .. automethod:: process
            :noindex:

        .. automethod:: set_gain
            :noindex:

        .. automethod:: mute
            :noindex:

        .. automethod:: unmute
            :noindex:

In version v4.35 this makes a contents like:
image

And in v4.36:
image

We'd like this to be configurable and it seems this PR might be half of the story (which is why I'm commenting here to keep discussion in one place).

Adding :no-link: is certainly an option to get :noindex: behaviour but then we lose the cross-referencing links which are pretty important to us.

I think adding the :noindex: option to all the directives and then gating the block added in #959 will give the desired behaviour but I'm not super familiar with breathe. I'm guessing a change like this would require some documentation changes (and maybe tests too).

I'm happy to create a PR with these changes or we can build on this one if @hojalot is amenable.

@hojalot
Copy link
Author

hojalot commented Mar 10, 2025

@BrennanGit I'm fine with additions to this branch, or feel free to incorporate the change into another branch/PR. My intention was just to silence a warning so any more general work beyond that seems like a good thing!

@@ -26,6 +26,7 @@ class _DoxygenClassLikeDirective(BaseDirective):
"show": unchanged_required,
"outline": flag,
"no-link": flag,
"noindex": flag,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noindex": flag,
"noindex": flag,
"no-index": flag,

Newer Sphinx releases support the syntax with - (it actually has become the default but without deprecating the original spelling).

@@ -20,6 +20,7 @@ class _DoxygenBaseItemDirective(BaseDirective):
"project": unchanged_required,
"outline": flag,
"no-link": flag,
"noindex": flag,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"noindex": flag,
"noindex": flag,
"no-index": flag,

Newer Sphinx releases support the syntax with - (it actually has become the default but without deprecating the original spelling).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants