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

javadoc: add support for tparam #210

Closed
wants to merge 1 commit into from

Conversation

stephanlachnit
Copy link
Contributor

Documentation of template parameters is supported in both doxygen and sphinx, so let's add it to the javadoc converter.

https://www.doxygen.nl/manual/commands.html#cmdtparam
https://www.sphinx-doc.org/en/master/usage/domains/cpp.html#info-field-lists

@stephanlachnit
Copy link
Contributor Author

The same also be done for retval, throw, throws and exception

@jnikula
Copy link
Owner

jnikula commented Nov 8, 2023

I'll share some of my thoughts around this.

First, I don't mind merging this, or further improvements to the currently a bit clumsy Javadoc support.

However, if there are more things to come, I might like copying the Javadoc code from hawkmoth.util.doccompat to hawkmoth.ext.javadoc, developing the Javadoc support there, and deprecating hawkmoth.util.doccompat altoghether.

Finally, if you don't specifically need Javadoc/Doxygen style comments (and I acknowledge there are valid reasons to stick to them) I'd always urge people to use reStructuredText directly. The conversion will always be a potential failure point, especially since reStructuredText is fussy about whitespace and indentation.

@stephanlachnit
Copy link
Contributor Author

However, if there are more things to come, I might like copying the Javadoc code from hawkmoth.util.doccompat to hawkmoth.ext.javadoc, developing the Javadoc support there, and deprecating hawkmoth.util.doccompat altoghether.

Sounds reasonable. I extended the MR for the other keywords, feel free to move to code wherever.

Finally, if you don't specifically need Javadoc/Doxygen style comments (and I acknowledge there are valid reasons to stick to them) I'd always urge people to use reStructuredText directly. The conversion will always be a potential failure point, especially since reStructuredText is fussy about whitespace and indentation.

While I do fully agree, this is unfortunately not really an option.

@jnikula
Copy link
Owner

jnikula commented Nov 15, 2023

Okay, I fully acknowledge this is not the way to treat contributors... and I'm sorry. But in the end I decided the current javadoc/doxygen parser is just too hard to extend or maintain properly.

So here's an overhauled parser: #216. It should cover what you ask here, and much, much, more. I haven't written tests for it yet (besides what we already have), but I've looked at the results manually.

Would you mind giving it a spin, please?

Oh, one thing it doesn't do is @exception. Seems to me the doxygen command is not the same as @throws. https://www.doxygen.nl/manual/commands.html#cmdexception

@stephanlachnit
Copy link
Contributor Author

Okay, I fully acknowledge this is not the way to treat contributors... and I'm sorry. But in the end I decided the current javadoc/doxygen parser is just too hard to extend or maintain properly.

No worries!

So here's an overhauled parser: #216. It should cover what you ask here, and much, much, more. I haven't written tests for it yet (besides what we already have), but I've looked at the results manually.

Would you mind giving it a spin, please?

Wow, that PR seems amazing! I will find some time to go through the code and test it out!

Oh, one thing it doesn't do is @exception. Seems to me the doxygen command is not the same as @throws. https://www.doxygen.nl/manual/commands.html#cmdexception

throws => throw => exception according to doxygen documentation

@jnikula
Copy link
Owner

jnikula commented Nov 16, 2023

Wow, that PR seems amazing! I will find some time to go through the code and test it out!

Thanks! Much appreciated!

throws => throw => exception according to doxygen documentation

You're right. My mistake.

@jnikula
Copy link
Owner

jnikula commented Nov 17, 2023

I merged #216 after some improvements. Closing this one. Please check it out and file issues and/or improve on it!

@jnikula jnikula closed this Nov 17, 2023
@stephanlachnit stephanlachnit deleted the patch-1 branch November 19, 2023 09:45
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.

2 participants