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 support for function prototype annotation #14

Merged
merged 5 commits into from
Oct 1, 2021

Conversation

davidgranstrom
Copy link
Contributor

Hi,

Thanks for publishing this plugin! I wanted to use it to annotate function prototypes in C header files so I added support for this. The original functionality for the C language type should still be preserved and I've tested some edge cases locally. Will do more extensive testing tomorrow, so I'll leave this as draft for now.

@danymat
Copy link
Owner

danymat commented Sep 28, 2021

This is very cool @davidgranstrom, thanks a lot for contributing !
I see that you used stylua for formatting, I think I will generate a stylua config file in root file, and add some check to it.

Regarding to the actual implementation, would you mind add a comment on this PR with one or two c snippets for me to test if it's working well ?

By the way (this is documentation related), do you have some remarks/insights for the Adding Languages documentations? If you used it of course 😄

@davidgranstrom
Copy link
Contributor Author

Sure! Here's a quick test with annotations generated using the default doxygen template.

test.h

#include <stdlib.h>

/**
 * @brief 
 *
 * @param[in] dst 
 * @param[in] src 
 * @param[in] len 
 * @returns 
 */
int func_with_retval(char *dst, char *src, size_t len);

/**
 * @brief 
 *
 * @param[in] s 
 * @param[in] len 
 */
void func_with_no_retval(char *s, size_t len);

/**
 * @brief 
 *
 * @returns 
 */
int func_with_no_args_and_retval(void);

@davidgranstrom
Copy link
Contributor Author

I'm not 100% sure its worth having the extra logic for checking void return types. I just thought of this edge case (except for the obvious one if someone should typedef void).

Maybe it would be best to always generate the @returns statement (for function prototypes) since its easier to remove a line compared to type it out manually?

test.h

/**
 * @brief 
 *
 * @param[in] size 
 */
void *getmem(size_t size);

test.c

/**
 * @brief 
 *
 * @param[in] size 
 * @returns 
 */
void *getmem(size_t size)
{
  return malloc(size);
}

@danymat
Copy link
Owner

danymat commented Sep 28, 2021

Maybe it would be best to always generate the @returns statement (for function prototypes) since its easier to remove a line compared to type it out manually?

I think it is the way to go in order for the function definition and the function prototype be consistent.
If I see that more people need to configure the return function, we will add a configurable option for the c language. What do you think ?

By the way (this is documentation related), do you have some remarks/insights for the Adding Languages documentations? If you used it of course 😄

Maybe you did not see this part, don't you mind to share some insights ?

@danymat
Copy link
Owner

danymat commented Sep 28, 2021

By the way, the PR works well, thanks a lot! 🥳
I will wait for you to do more testing, be sure to mark as Ready for review when you think you're good

@danymat
Copy link
Owner

danymat commented Oct 1, 2021

@davidgranstrom Have you done some testings? I think we're good, but I want to hear your response

@davidgranstrom
Copy link
Contributor Author

@danymat I figured out a way to check for pointer return values. Maybe this is the most frictionless solution for function prototypes? The alternative, as mentioned earlier to always generate the @return statement. However, doing that might not always match the function implementation.

With the latest push the following works:

#include <stdlib.h>

/**
 * @brief 
 *
 * @param[in] size 
 * @returns 
 */
void *getmem(size_t size);

/**
 * @brief 
 *
 * @param[in] s 
 */
void to_upper(char *s);

@davidgranstrom
Copy link
Contributor Author

I could also give you some feedback on the "Adding Languages documentations" :)
Perhaps I could open a separate issue for this or send you an email?

@danymat
Copy link
Owner

danymat commented Oct 1, 2021

I just saw your last commit, and this is a simple way to check if the functions returns a pointer. You can keep it like that.

I could also give you some feedback on the "Adding Languages documentations" :)
Perhaps I could open a separate issue for this or send you an email?

Feel free to open an issue, no need to keep in private !

I think I'm good to merge the PR, what do you think?

@davidgranstrom davidgranstrom marked this pull request as ready for review October 1, 2021 14:23
@davidgranstrom
Copy link
Contributor Author

I just saw your last commit, and this is a simple way to check if the functions returns a pointer. You can keep it like that.

Okay, great!

I could also give you some feedback on the "Adding Languages documentations" :)
Perhaps I could open a separate issue for this or send you an email?

Feel free to open an issue, no need to keep in private !

Alright, will do as soon as I get a chance.

I think I'm good to merge the PR, what do you think?

Yes, I think it's ready now. 👍

@danymat danymat merged commit f1f28f1 into danymat:main Oct 1, 2021
@davidgranstrom davidgranstrom deleted the topic/c-headers branch October 1, 2021 14:58
@danymat danymat mentioned this pull request Nov 8, 2021
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