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

Docs: NatSpec and/or style guide should specify how multi-line NatSpec works #11359

Closed
4 tasks
fulldecent opened this issue May 7, 2021 · 9 comments
Closed
4 tasks
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. documentation 📖 easy difficulty good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. stale The issue/PR was marked as stale because it has been open for too long. style guide Modification of the Solidity Style Guide

Comments

@fulldecent
Copy link
Contributor

fulldecent commented May 7, 2021

Regarding: https://docs.soliditylang.org/en/v0.8.4/natspec-format.html

Many people choose to limit source code line lengths to 79 characters wrap (or some other number). This is an identified best practice in the Solidity Style Guide. And comments can (SHOULD!) be more than just several characters. So...

Let's specify how longer comments should work:

//------10--------20--------30--------40--------50--------60--------70--------80

// Is this preferred❓ (align to tags)

/// @notice You can use this to instantiate a complete circus, including some
///         animals that dance, sing and jump.
/// @dev    Animals are instantiated only once each
contract Circus {
}

// Or this❓ (align to tags and params)

/// @notice You can use this to instantiate a complete circus, including some
///         animals that dance, sing and jump.
/// @dev    Animals are instantiated only once each
/// @param  a       the amount of stuff
/// @param  tokenID which token to operate on
function doThing(uint256 a, uint tokenID b) {
}

// Or this❓ (no indent)

/// @notice You can use this to instantiate a complete circus, including some
/// animals that dance, sing and jump.
/// @dev Animals are instantiated only once each
contract Circus {
}

// Or this❓ (indent to tag-end column)

/// @notice You can use this to instantiate a complete circus, including some
///         animals that dance, sing and jump.
/// @dev Animals are instantiated only once each
contract Circus {
}

// Or this❓ (four-space indent)

/// @notice You can use this to instantiate a complete circus, including some
///     animals that dance, sing and jump.
/// @dev Animals are instantiated only once each
contract Circus {
}

// Or this❓ (repeat tag name) (⚠️ this may be ambiguous for @return and others)

/// @notice You can use this to instantiate a complete circus, including some
/// @notice animals that dance, sing and jump.
/// @dev Animals are instantiated only once each
contract Circus {
}

References

Discussion

  • Align-to-tags should be most familiar to people with experience using Doxygen on various platforms.
  • Align-to-tags indentation also has utility in lining up your @params.
  • We should specify that any extra indentation spaces will be surpassed from the JSON output.

Recommended work plan:

  • Choose our preferred style (recommend align-to-tags-and-params).
  • Update all examples in Solidity documentation to use that style.
  • Decide if this should be a public recommendation
    • E.g. the /// format is listed before /** in the NatSpec, and all examples use ///, not /**, but we do NOT publicly recommend /// over /**.
  • If there is indentation, show in the JSON output how it is truncated. And specify in text how whitespace is truncated in natspec-format.rst.
    • I.e. "/// @notice Hello\n/// world" is "Hello world", not "Hello world" or "Hello\nworld".
@Abhijnan-Bajpai
Copy link

Hey @cameel, I would like to work on this issue if it isn't resolved yet. Could I be assigned?

@cameel
Copy link
Member

cameel commented Sep 12, 2021

Sure, just be aware that this is a task that requires some discusion and making a decision on which style we should recommend (and if we should recommend a single style at all). I think that submitting a draft PR with a particular proposal is a good way to get that discussion started though so you can go ahead if you want to try. Just be prepared that there might be some going back and forth in the PR before we agree on a final version :)

I'd suggest to start with just describing the style and only updating all the examples when it's clear which style we want (though updating one or two examples might also be a good way to show off the style you choose).

It would also be good to check if there's one clearly preferred style used in contracts found in the wild. Or what existing linters already recommend - I see that for example solhint does not lint natspec yet and there's a very recent feature request from @fulldecent to add support for it: protofire/solhint#298.

@fulldecent
Copy link
Contributor Author

Thank you for your interest.

Also want to note my main interest in this issue. Overall my goal is that all Solidity functions should be commented, consistently (no dog shedding), by default, and complained loudly when missing, and have consistent/specified interpretation. And when that happens we might hope that a browser-based web3 wallet will make this information available to you every time you sign something.

@Abhijnan-Bajpai Abhijnan-Bajpai removed their assignment Sep 19, 2021
@cameel cameel added the style guide Modification of the Solidity Style Guide label Feb 16, 2022
@fulldecent fulldecent moved this to Todo in Community service Feb 23, 2022
@cameel cameel added good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. and removed good first issue labels Dec 5, 2022
@AndriianChestnykh
Copy link

@fulldecent, is there any progress on the task or it is deprecated now?

@jtakalai
Copy link

I think it's not just "style guide" thing; also the compiler should generate correct multi-line spec with the suggested multi-lining!

@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 29, 2023
@github-actions
Copy link

github-actions bot commented Jul 6, 2023

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Jul 6, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Community service Jul 6, 2023
@hcheng826
Copy link

is there any best practices recommendation for this case?

@fulldecent
Copy link
Contributor Author

My personal recommendation is still align with tags and params.

But can't say this is universally recognized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. documentation 📖 easy difficulty good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. stale The issue/PR was marked as stale because it has been open for too long. style guide Modification of the Solidity Style Guide
Projects
None yet
Development

No branches or pull requests

6 participants