feat(doc): do not italicize the @dev NatSpec

PaulRBerg commented 1 year ago

Component

Forge

Describe the feature you would like

Issue

forge doc adds an asterisk * to the Markdown paragraphs generated for the @dev NatSpec tag.

This is not great because it screws up the formatting for anything more complicated than a line, e.g.:

/// @notice Creates a stream with the provided start time and end time as the range of the stream. The stream is
/// funded by `msg.sender` and is wrapped in an ERC-721 NFT.
///
/// @dev Emits a {CreateLockupLinearStream} and a {Transfer} event.
///
/// Notes:
/// - As long as they are ordered, it is not an error to set a range that is in the past.
///
/// Requirements:
/// - `params.recipient` must not be the zero address.
/// - `params.totalAmount` must not be zero.
/// - `params.range.start` must not be greater than `params.range.cliff`.
/// - `params.range.cliff` must not be greater than `params.range.end`.
/// - `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
/// - If set, `params.broker.fee` must not be greater than `MAX_FEE`.
///
/// @param params Struct that encapsulates the function parameters.
/// @return streamId The id of the newly created stream.
function createWithRange(LockupLinear.CreateWithRange calldata params) external returns (uint256 streamId);

forge doc generates the following Markdown for the function above:

### createWithRange

Creates a stream with the provided start time and end time as the range of the stream. The stream is
funded by `msg.sender` and is wrapped in an ERC-721 NFT.

*Emits a {CreateLockupLinearStream} and a {Transfer} event.
Notes:
- As long as they are ordered, it is not an error to set a range that is in the past.
Requirements:
- `params.recipient` must not be the zero address.
- `params.totalAmount` must not be zero.
- `params.range.start` must not be greater than `params.range.cliff`.
- `params.range.cliff` must not be greater than `params.range.end`.
- `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
- If set, `params.broker.fee` must not be greater than `MAX_FEE`.*

It adds that extra *, which (i) doesn't work in this case because this is a paragraph with a list, and (ii) even for cases where it works (single lines), it looks ugly, IMO.

Solutions

Completely remove this asterisk.
Let users choose whether they want the asterisk via a config option under [doc].

Additional context

No response

mds1 commented 1 year ago

Ah interesting. For context the rationale here was:

@notice and @dev are for different audiences, so it would be nice to distinguish them somehow so each type of reader knows what's relevant
The primitive-dodoc hardhat plugin puts @dev tags in italics as shown there, which is why we went that route (ref https://github.com/foundry-rs/foundry/pull/2701#issuecomment-1376009358).

Wonder if there's any alternative solutions that keep the visual separation but doesn't break formatting like this

PaulRBerg commented 1 year ago

Thanks for the context.

@notice and @dev are for different audiences,

Well, they get bundled together in the auto-generated Markdown content, so it's hard to disentangle them. It took me a few moments (the author of my NatSpec comments) to figure out what was happening; a third-party reader of the docs website wouldn't have a clue that one section is for general audience, and the other is for devs.

Wonder if there's any alternative solutions that keep the visual separation but doesn't break formatting like this

At a minimum, forge doc should handle the case when the content of the @dev tag is split into multiple lines. Adding asterisks doesn't work in this case.

But users should ideally be able to tweak this on and off either via an option in foundry.toml, or by a more robust templating functionality à la solidity-docgen (which can generate different Markdown based on a template provided by the user).

Generally speaking, I am in favor of forge doc making as few opinionated choices as possible, because a simpler Markdown is easier to customize later on.

sambacha commented 1 year ago

Ah interesting. For context the rationale here was:

@notice and @dev are for different audiences, so it would be nice to distinguish them somehow so each type of reader knows what's relevant

The primitive-dodoc hardhat plugin puts @dev tags in italics as shown there, which is why we went that route (ref feat(forge): doc #2701 (comment)).

Wonder if there's any alternative solutions that keep the visual separation but doesn't break formatting like this

The primitive dodoc does not generate well formatted output to begin with. Look at the markdown tables it generates versus forge doc.

notice and dev are not exclusionary audiences, though. It's impossible for the author to presage what the reader think is relevant. Its up to the author to document what they assume to be relevant.

Styling should be decoupled from how the output is produced. Providing a template like mentioned by @PaulRBerg would be a great start. mdbook is already severely limited in terms of producing a usable documentation website. Better to focus on producing usable markdown output and leaving the rest of the styling to the end user.

sambacha commented 1 year ago

Ah interesting. For context the rationale here was:

@notice and @dev are for different audiences, so it would be nice to distinguish them somehow so each type of reader knows what's relevant

The primitive-dodoc hardhat plugin puts @dev tags in italics as shown there, which is why we went that route (ref feat(forge): doc #2701 (comment)).

Wonder if there's any alternative solutions that keep the visual separation but doesn't break formatting like this

The primitive dodoc does not generate well formatted output to begin with. Look at the markdown tables it generates versus forge doc.

notice and dev are not exclusionary audiences, though. It's impossible for the author to presage what the reader think is relevant. Its up to the author to document what they assume to be relevant.

Styling should be decoupled from how the output is produced. Providing a template like mentioned by @PaulRBerg would be a great start. mdbook is already severely limited in terms of producing a usable documentation website. Better to focus on producing usable markdown output and leaving the rest of the styling to the end user.

foundry-rs / foundry