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

Add breate_doxygen_aliases config value #729

Merged
merged 3 commits into from
Sep 27, 2021

Conversation

BenWhetton
Copy link
Contributor

The configuration option enables users to add custom commands to doxygen
when using autodoxygenindex and autodoxygenfile as documented here:
https://www.doxygen.nl/manual/custcmd.html
Previously, users were limited to the couple of aliases which are hard-coded into
the autodoxygen config file.

I don't think there is way to write an automatic test for this without quite a lot of effort.
I have manually tested it by creating a test sphinx docs repo, creating a couple of
aliases and using them in a doxygen comment in an example c++ file. It works
fine.

If there is a way to automate this which won't take much time (max 30mins), I'm
happy to give it a go. Although, I think it's a simple enough change that it's pretty
low risk to add it without automated tests.

Let me know if anything else needs changing.

@michaeljones
Copy link
Collaborator

I'm not so familiar with this part of the code myself but it looks good to me. Thanks for the clear documentation. I'll try to grab the branch and poke around in a bit.

@michaeljones
Copy link
Collaborator

Other aliases in the generated file have a slightly different quoting pattern. Should we match those instead?

ALIASES = "rst=\verbatim embed:rst"
ALIASES += "endrst=\endverbatim"
ALIASES += "inlinerst=\verbatim embed:rst:inline"

ALIASES += rstref{1}="\verbatim embed:rst:inline :ref:`\1` \endverbatim"

@michaeljones
Copy link
Collaborator

Could you also rebase onto master so that the tests pass? They've been fixed up in another PR that is merged now. Sorry for the trouble.

@BenWhetton
Copy link
Contributor Author

Other aliases in the generated file have a slightly different quoting pattern. Should we match those instead?

ALIASES = "rst=\verbatim embed:rst"
ALIASES += "endrst=\endverbatim"
ALIASES += "inlinerst=\verbatim embed:rst:inline"

ALIASES += rstref{1}="\verbatim embed:rst:inline :ref:`\1` \endverbatim"

I used the quoting pattern which is in the doxygen documentation. I don't have a strong opinion about this though if you'd prefer consistency with the previous hard-coded entries, that's fine. I'll just double check that that quoting style actually works for aliases with arguments.

Could you also rebase onto master so that the tests pass? They've been fixed up in another PR that is merged now. Sorry for the trouble.

Sure, no problem at all.

I'm busy tonight. I will probably get around to this tomorrow evening.

@michaeljones
Copy link
Collaborator

I used the quoting pattern which is in the doxygen documentation. I don't have a strong opinion about this though if you'd prefer consistency with the previous hard-coded entries, that's fine. I'll just double check that that quoting style actually works for aliases with arguments.

Ah, good call. Seems like we should change the others if anything. Thanks for providing the link.

The configuration option enables users to add custom commands to doxygen
when using `autodoxygenindex` and `autodoxygenfile`.

There is no obvious way to add an automatic test for this without
considerable effort. It has been tested manually.
@BenWhetton
Copy link
Contributor Author

BenWhetton commented Sep 11, 2021

I just updated the PR:

  • Rebase onto master (small changes to accommodate change in file layout and changelog location)
  • Change the quoting for all doxygen aliases to match the style from the doxygen docs

I did a quick test with a couple of the hard-coded aliases and they still work fine.

Copy link
Collaborator

@vermeeren vermeeren left a comment

Choose a reason for hiding this comment

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

@michaeljones Current version seems good to me, do you have any items remaining?

(It does need another rebase to relocate the changelog entry for 4.32.0 (unreleased), but that's something one of us Breathe maintainers can do prior to merging if no other changes are needed.)

@vermeeren vermeeren added code Source code enhancement Improvements, additions (also cosmetics) labels Sep 18, 2021
@michaeljones
Copy link
Collaborator

I hope I've not broken anything with those line length changes. Happy to merge when the tests are through.

@michaeljones michaeljones merged commit 09cf23e into breathe-doc:master Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Source code enhancement Improvements, additions (also cosmetics)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants