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 C++ #19

Closed
Shatur opened this issue Nov 6, 2021 · 27 comments
Closed

Add support for C++ #19

Shatur opened this issue Nov 6, 2021 · 27 comments
Labels
enhancement New feature or request

Comments

@Shatur
Copy link
Contributor

Shatur commented Nov 6, 2021

The plugin is currently supports C, can we enable it for C++?

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Hello, sure we could, but I'm not sure the tree sitter queries are the same. I'll try to look into that

@danymat danymat added the enhancement New feature or request label Nov 8, 2021
@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Should be partially same because C++ contains ; inherits: c in treesitter highlighting:
https://github.com/nvim-treesitter/nvim-treesitter/blob/0e25e0e98990803e95c7851236e43b3ee934d443/queries/cpp/highlights.scm#L1

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Hmm, can you try adding c configuration for cpp and see if you can generate some annotations ?

You can modify setup as so, to include support for cpp

require('neogen').setup {
            enabled = true,
            languages = {
                cpp = require("neogen.configurations.c")
            }
        }

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

It works, thanks!
But some additional stuff missing, like information about templates.

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Nice, so that means we can use the c configuration language, and apply more stuff for cpp.

But some additional stuff missing, like information about templates.

What do you mean ? I need some extra information, or a snippet to reproduce it

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

And related question. I noticed that all params generated with [in]. Is this expected? For example, the following function:

    void test(int &value);

Generates:

    /**
     * @brief 
     *
     * @param[in] value 
     */

But it should be:

    /**
     * @brief 
     *
     * @param[out] value 
     */

But I think that you can't detect it with Treesitter. Could we generate it without direction annoation?

    /**
     * @brief 
     *
     * @param value 
     */

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Quick example for testing: https://github.com/crow-translate/crow-translate/blob/400c2432e37f64e7c65b5fd87add02ce5b34a005/src/mainwindow.h#L159

Should contains tparam.

Added support for variadic functions in cpp (as simple as doing this: e4e09d6 !), you can try it and tell me.
You can even delete the snippet I specified here: #19 (comment), as it's now default

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

And related question. I noticed that all params generated with [in]. Is this expected? For example, the following function:

    void test(int &value);

Generates:

    /**
     * @brief 
     *
     * @param[in] value 
     */

But it should be:

    /**
     * @brief 
     *
     * @param[out] value 
     */

But I think that you can't detect it with Treesitter. Could we generate it without direction annoation?

    /**
     * @brief 
     *
     * @param value 
     */

About this, I don't really now, it was being worked in this PR: #14
Do you have any links I could use to know what's [in] and [out] ?

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Added support for variadic functions in cpp (as simple as doing this: e4e09d6 !), you can try it and tell me.

Thank you, It works!
But I meant to add tparam: https://www.doxygen.nl/manual/commands.html#cmdtparam
(used in the same example with variadic arguments)

Do you have any links I could use to know what's [in] and [out] ?

That is the problem. It is not always possible to tell. If you are passing something by pointer, it can be either input or output. I would suggest to remove direction specification at all. It is optional.

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

That is the problem. It is not always possible to tell. If you are passing something by pointer, it can be either input or output.

I see. I think it'll be very difficult to get the information, as I would need to see if the parameter is being written to.
It's feasible, but not quite at the moment because it means adding some checks in the generator.
I will remove them, but if someone wants to create a basic fetcher for it, I'm totally okay

danymat added a commit that referenced this issue Nov 8, 2021
@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

I will remove them

Yep, this would be the best solution for now.

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Found another issue. If I try to generate the documentation for the following function:

https://github.com/crow-translate/crow-translate/blob/400c2432e37f64e7c65b5fd87add02ce5b34a005/src/mainwindow.h#L69

I get this:

    /*  */
    QKeySequence closeWindowShortcut() const;

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Quick example for testing: https://github.com/crow-translate/crow-> translate/blob/400c2432e37f64e7c65b5fd87add02ce5b34a005/src/mainwindow.h#L159>

Should contains tparam.

Is it supposed to supercharge a function ?

I get this from Treesitter:

Capture d’écran 2021-11-08 à 20 22 19

If it's a type by it's own, I can create a new type for this, called template (see https://github.com/danymat/neogen#usage for more details about types in neogen)

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Found another issue. If I try to generate the documentation for the following function:

https://github.com/crow-translate/crow-translate/blob/400c2432e37f64e7c65b5fd87add02ce5b34a005/src/mainwindow.h#L69

I get this:

    /*  */
    QKeySequence closeWindowShortcut() const;

What should I get instead ?

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

What should I get instead ?

    /**
     * @brief 
     *
     * @return 
     */

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Looks like here are two separate issues.

  1. Generation of empty comment /* */ happens if the function doesn't have any arguments.
  2. If return type isn't void, the documentation should have @return after all params.

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

Looks like here are two separate issues.

  1. Generation of empty comment /* */ happens if the function doesn't have any arguments.
  2. If return type isn't void, the documentation should have @return after all params.

I don't think it's an issue, it's a feature:

doxygen = {
{ nil, "/* $1 */", { no_results = true } },
{ nil, "/**" },
{ nil, " * @brief $1" },
{ nil, " *" },
{ "parameters", " * @param %s $1" },
{ "return_statement", " * @returns $1" },
{ nil, " */" },
},

I mean, Treesitter does not tell me there is a return in your code snippet:
Capture d’écran 2021-11-08 à 20 31 37

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

I mean, Treesitter does not tell me there is a return in your code snippet:

If a declaration contains a type first, then it's a return value. Or do you think that this is a Treesitter bug?

Generation of empty comment /* */ happens if the function doesn't have any arguments.

But this probably shouldn't happen? We should always have a @brief even if the function doesn't have any arguments.

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

But this probably shouldn't happen? We should always have a @brief even if the function doesn't have any arguments.

Here is an example of a function without a return type and without arguments: https://github.com/crow-translate/crow-translate/blob/400c2432e37f64e7c65b5fd87add02ce5b34a005/src/mainwindow.h#L145

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

To be more clear I expecting to get the following:

    /**
     * @brief 
     */

Instead of

/* */

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

If a declaration contains a type first, then it's a return value. Or do you think that this is a Treesitter bug?

No I think I should add this feature on my end. But I do want to check if this is written in doxygen. Can you hint me a place where they confirm this ?

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

To be more clear I expecting to get the following:

    /**
     * @brief 
     */

Instead of

/* */

Modified template in last commit e87b4a2

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Modified template in last commit e87b4a2

Thanks, now looks as expected!

No I think I should add this feature on my end. But I do want to check if this is written in doxygen. Can you hint me a place where they confirm this ?

If a function have void return type, then there shouldn't be @return. In other case it should contains @return.
Here is an example: https://github.com/crow-translate/QOnlineTranslator/blob/33a53bf29a096c967b8e51ebccf70c02a94ab1c3/src/qonlinetranslator.h#L243
The documentation of this repo is validated on CI.

@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

If a function have void return type, then there shouldn't be @return. In other case it should contains @return.
Here is an example: https://github.com/crow-translate/QOnlineTranslator/blob/33a53bf29a096c967b8e51ebccf70c02a94ab1c3/src/qonlinetranslator.h#L243
The documentation of this repo is validated on CI.

Should I close it and open another issue about @return?

@danymat
Copy link
Owner

danymat commented Nov 8, 2021

If a function have void return type, then there shouldn't be @return. In other case it should contains @return.
Here is an example: https://github.com/crow-translate/QOnlineTranslator/blob/33a53bf29a096c967b8e51ebccf70c02a94ab1c3/src/qonlinetranslator.h#L243
The documentation of this repo is validated on CI.

Should I close it and open another issue about @return?

Yes, be sure to reference useful snippets to alleviate testing

@Shatur Shatur closed this as completed Nov 8, 2021
@Shatur
Copy link
Contributor Author

Shatur commented Nov 8, 2021

Done, thanks!

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

No branches or pull requests

2 participants