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

feat: Add assists for macro documentation comments #78

Merged
merged 6 commits into from
Dec 17, 2024

Conversation

naipaka
Copy link
Contributor

@naipaka naipaka commented Dec 9, 2024

🔗 Related Issues

  • None

🙌 What's Done

  • Add assists for macro documentation comments

Background

https://altive.slack.com/archives/C072SHBR4DT/p1733300766521879

Since public_member_api_docs is true, you need to include doc comments for constructors as well. Until now, I’ve been writing comments like /// Creates an instance of [Xxx].. However, I felt such comments don’t provide much value when checking them in the reference.

Therefore, I thought it might be a good idea to insert the same doc comment as the class using macros, unless there’s something specific to add to the constructor's comment.

To make this adjustment easier, I’ve created an assist feature with custom_lint!

✍️ What's Not Done

  • None

🖼️ Image Differences

Add macro template documentation comment Add macro documentation comment Wrap with macro template documentation comment
スクリーンショット 2024-12-09 10 01 51 スクリーンショット 2024-12-09 10 02 31 スクリーンショット 2024-12-09 10 02 11

🤼 Desired Review Method

  • Correction Commit
  • Pair programming

Note

It is possible that a reviewer's will may cause a method to be implemented that is not selected.

📝 Additional Notes

Pre-launch Checklist

  • I have reviewed my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I updated/added relevant documentation (doc comments with ///).

@naipaka naipaka requested a review from a team as a code owner December 9, 2024 01:04
@naipaka naipaka requested review from boywithdv and removed request for a team December 9, 2024 01:04
@naipaka naipaka marked this pull request as draft December 9, 2024 01:05
@naipaka naipaka marked this pull request as ready for review December 9, 2024 01:05
@naipaka naipaka requested review from riscait and k-nkmr and removed request for boywithdv December 9, 2024 01:05
/// ```dart
/// /// {[@]template packageName.className}
/// ///
/// /// {[@]endtemplate}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the example, using @endtemplate as is would be interpreted as the end of the template, so I’ve enclosed it in [] to avoid this!

Copy link
Contributor

@k-nkmr k-nkmr left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you for adding this assist!

Comment on lines 55 to 61
if (!target.intersects(node.sourceRange)) {
return;
}

if (!node.isDocumentation) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

badge

Suggested change
if (!target.intersects(node.sourceRange)) {
return;
}
if (!node.isDocumentation) {
return;
}
if (!target.intersects(node.sourceRange)) {
return;
}
if (node.toSource().contains('{@template') &&
node.toSource().contains('{@endtemplate}')) {
return;
}
if (!node.isDocumentation) {
return;
}

I thought it might be worth considering adding this early return.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!
It certainly seems like a good idea. ✍️

Since toSource was an empty string and couldn't be evaluated, I changed it to use tokens for evaluation!
feat: Prevent wrapping of existing macro template documentation comments

      final currentComment = node.tokens.join();
      if (currentComment.contains('{@template') &&
          currentComment.contains('{@endtemplate}')) {
        return;
      }

Copy link
Member

@riscait riscait left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for new feature🚀

@naipaka naipaka enabled auto-merge (squash) December 16, 2024 05:08
@naipaka naipaka merged commit 8126ddc into main Dec 17, 2024
1 check passed
@naipaka naipaka deleted the add-dartdoc-macros-assist branch December 17, 2024 00:58
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.

3 participants