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 doc support of lambda parameters and return value for cpp,swift,dart #1545

Conversation

limingchina
Copy link
Contributor

@limingchina limingchina commented Aug 4, 2023

see: #1544

Currently, the documentation of parameters and return values for lambdas only works for java. All the other generators don't work. This is because that Java's lambda declaration is called functional interface and is similar like a function declaration. However, for the other generators, the mustache template file only uses normal documentation instead of function documentation.

For the return type, the issue is that the LimeParser doesn't have doc support for the return type of a lambda.

This change addresses the two issues.

The fix still has some limitation, though. In java, one can use specify a name for the lambda parameter and use that in the parameter documentation. With the current change, only the types will appear in the lambda declaration and docs. To limit the scope of this change. This is left for future improvement.

@limingchina limingchina force-pushed the add_doc_support_for_lambda_param_and_return_value branch from 849865c to 5b3815e Compare August 4, 2023 17:59
@limingchina limingchina changed the title Add documentation support of lambda parameters and return value Add doc support of lambda parameters and return value for cpp,swift,dart Aug 4, 2023
…pp,swift,dart.

The android generator(java) already works

see: heremaps#1544
Signed-off-by: Ming Li <[email protected]>
@limingchina limingchina force-pushed the add_doc_support_for_lambda_param_and_return_value branch 2 times, most recently from 3d6217e to ecabe32 Compare August 4, 2023 21:50
Also removed the added functional test case since smoke test
already covers it.
@limingchina limingchina force-pushed the add_doc_support_for_lambda_param_and_return_value branch from ecabe32 to 349cdee Compare August 4, 2023 21:51
@@ -84,6 +85,7 @@ internal class CppNameResolver(
is LimeType -> resolveTypeName(element, isFullName = false)
is LimeTypeRef -> resolveTypeRef(element)
is LimeReturnType -> resolveTypeRef(element.typeRef)
is LimeLambdaParameter -> resolveTypeRef(element.typeRef)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this addition, it would go to the throw statement.

@@ -18,7 +18,9 @@
! License-Filename: LICENSE
!
!}}
{{>dart/DartDocumentation}}{{>dart/DartAttributes}}
{{>dart/DartLambdaDocs}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to leave a blank line here. Otherwise, the lambda declaration will be in the same line as the empty comment, which would hide the lambda declaration.

@@ -18,23 +18,8 @@
! License-Filename: LICENSE
!
!}}
{{#resolveName comment}}{{#unless this.isEmpty}}{{prefix this "/// "}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to split this the doc part and attribute part. Otherwise, the lambda gets some duplicated attributes here

@limingchina limingchina force-pushed the add_doc_support_for_lambda_param_and_return_value branch from 46a3055 to 64d7aac Compare August 5, 2023 07:33
* \private
* \param[in] ::std::string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one lost the const&. Would need some fix.

@limingchina
Copy link
Contributor Author

#1546 has a more complete fix. Close this PR.

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.

1 participant