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(forge doc): include custom natspec #9075

Merged

Conversation

leovct
Copy link
Contributor

@leovct leovct commented Oct 9, 2024

Motivation

Closes #8488

Include @custom natspec tags when rendering docs with forge doc.

Solution

I've chosen to prefix custom tags with // Comment: for visibility reasons, but I'm open to suggestions.

Example with https://github.com/artgobblers/art-gobblers where I introduced some custom tags.

$ git diff src/ArtGobblers.sol
diff --git a/src/ArtGobblers.sol b/src/ArtGobblers.sol
index d322858..71cb8e9 100644
--- a/src/ArtGobblers.sol
+++ b/src/ArtGobblers.sol
@@ -80,6 +80,9 @@ import {Pages} from "./Pages.sol";
 /// @author FrankieIsLost <frankie@paradigm.xyz>
 /// @author transmissions11 <t11s@paradigm.xyz>
 /// @notice An experimental decentralized art factory by Justin Roiland and Paradigm.
+/// @custom:test-a A first custom tag
+/// @custom:test-b A second custom tag
+/// @custom:test-c A third custom tag
 contract ArtGobblers is GobblersERC721, LogisticVRGDA, Owned, ERC1155TokenReceiver {
     using LibString for uint256;
     using FixedPointMathLib for uint256;
@@ -89,6 +92,7 @@ contract ArtGobblers is GobblersERC721, LogisticVRGDA, Owned, ERC1155TokenReceiv
     //////////////////////////////////////////////////////////////*/
 
     /// @notice The address of the Goo ERC20 token contract.
+    /// @custom:test-b Another custom tag.
     Goo public immutable goo;
 
     /// @notice The address of the Pages ERC721 token contract.
@@ -344,6 +348,7 @@ contract ArtGobblers is GobblersERC721, LogisticVRGDA, Owned, ERC1155TokenReceiv
     /// limit is enforced during the creation of the merkle proof, which will be shared publicly.
     /// @param proof Merkle proof to verify the sender is mintlisted.
     /// @return gobblerId The id of the gobbler that was claimed.
+    /// @custom:test-c Yet another simple custom tag.
     function claimGobbler(bytes32[] calldata proof) external returns (uint256 gobblerId) {
         // If minting has not yet begun, revert.
         if (mintStart > block.timestamp) revert MintStartPending();

Screenshot 2024-10-09 at 10 37 01

Screenshot 2024-10-09 at 10 37 17

@leovct leovct marked this pull request as ready for review October 9, 2024 09:01
@grandizzy
Copy link
Collaborator

that's great, thanks! not a strong opinion but I think would be nicer to have bullet point for them (as Custom key is not relevant for docs) like

Inherits: ....
Authors: ....
An experimental ....
- A first custom tag
- A second custom tag

wdyt?

@zerosnacks
Copy link
Member

Thanks!

Supportive of changing the format, my suggestion would be to additionally add a header:

Notes:

- A first custom tag
- A second custom tag

as in a list with a header stating Notes in bold

@leovct
Copy link
Contributor Author

leovct commented Oct 9, 2024

I also prefer this approach.

Screenshot 2024-10-09 at 16 32 32

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Thanks @leovct!

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

Lgtm!

@zerosnacks zerosnacks merged commit 92702e9 into foundry-rs:master Oct 9, 2024
21 checks passed
@leovct leovct deleted the feat/forge-doc-include-custom-natspec branch October 11, 2024 21:10
@Metaxona
Copy link

Metaxona commented Nov 1, 2024

okay i just now looked at the output for this one, looks good, but it is missing the label after @custom: making the notes section confusing instead of informative

ex.

this

@custom:some-info Hello

outputs this

Notes:

  • Hello

instead of

Notes:

  • some-info : Hello

@leovct
Copy link
Contributor Author

leovct commented Nov 4, 2024

okay i just now looked at the output for this one, looks good, but it is missing the label after @custom: making the notes section confusing instead of informative

ex.

this

@custom:some-info Hello

outputs this

Notes:

  • Hello

instead of

Notes:

  • some-info : Hello

It should be fixed! :)

rplusq pushed a commit to rplusq/foundry that referenced this pull request Nov 29, 2024
* feat(`forge doc`): include @Custom natspec

* chore: make clippy happy

* test: implement test for `is_custom`

* chore: make rustfmt happy

* doc: nit

* chore: format custom tags
@grandizzy grandizzy added T-feature Type: feature C-forge Command: forge labels Dec 18, 2024
@grandizzy grandizzy changed the title feat(forge doc): include @custom natspec feat(forge doc): include custom natspec Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge T-feature Type: feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat(forge doc): include @custom NatSpec
4 participants