Rewrite

[frangio]

solidity-docgen is being rewritten to be simpler and easier to maintain and contribute to. These goals haven't been realized yet mainly because there is pending documentation and the code lacks some explanatory comments, but the point of this rewrite is that there should be a lot less to explain!

The work is mostly done and can be found in the rewrite branch.

This is a breaking release, your existing templates will not work without some changes. The CLI is no longer available (but may be brought back later), a Hardhat plugin is available instead, as well as standalone use as a library.

There are some pending tasks:

• [ ] Finish default templates. Must look good and do a good job of presenting all info and docs in the contracts.
• [ ] Write detailed usage documentation including template customization.
• [ ] Ensure there are explanatory comments in important parts of the source code.
• [ ] Write tutorial to set up together with Vuepress.
• [ ] Add changelog section for new version. Emphasize breaking changes.

A beta can be installed from npm as solidity-docgen@next, but this is experimental and can fail. If you try it out, report any issues.

[0xVolosnikov]

This regex doesn't clear correctly lines with multiple spaces at the beginning. Like this:

/**
*  @notice foo
*  @dev baz
*/

may be worth replacing it like that: .replace(/^[ \t]+/mg, '');

[frangio]

@vladyan18 The reason I didn't do that is that indentation can be significant. For example if the contents of the comment contain a Solidity example:

/**
 * This function should be protected against reentrancy:
 * ```
 * function foo() nonReentrant {
 *     _bar();
 * }
 * ```
 */

The indentation inside the function would be removed by the regex you suggested.

An alternative would be to find the shortest common whitespace prefix across all lines. In my example it would be a single space, in your example it would be two spaces.

But I also think the current behavior is quite reasonable. :slightly_smiling_face: Do you think it's worth changing this?

[0xVolosnikov]

@frangio Indeed, your example is correct. Another option would be to use something like .replace(/^[ \t]?@/mg, '@');, but it can make a mess.

So it's probably best to really leave it as it is.

[0xVolosnikov]

@frangio Also, I had a problem when I used inheritdoc with public state variables.

This check forbids it.

[frangio]

Thanks. I have a fix for that I'll publish later.

[frangio]

@vladyan18 Just published the full suite of templates in a new beta. If you're testing this, I'd love to know what you think.

[PaulRBerg]

Hi @frangio! Great work with this refactor. I upgraded to the next version but I got stuck trying to understand how the new version works. A simple migration guide would be helpful. Specifically the issue is that I don't know how to port my current template:

https://github.com/hifi-finance/hifi/blob/dc748c7ff66d652a00463dc57ea7c0cc12f0b289/templates/contract.hbs

To the newer format of v0.6. Should I provide one Handlebars template for each type of item? if yes, how can I do this? Do I have to work with the pages setting in Hardhat?

[frangio]

Hi @paulrberg, glad you've tried it. It's taking me longer than I wanted to finish this new version and work on the documentation including a migration guide, but it's pretty stable by now. To customize the templates you need to set the templates parameter in the Hardhat config to name a directory of templates to override the default ones (can be just a subset and the rest use the default). As a starting point you can just copy the ones you need to customize from themes/markdown to your templates directory.

Yes, each type of item has its own template, this is because items are not necessarily inside a contract now (e.g. free functions).

I took a look at your repo and migrated it to the new version to see what that was like. Some of the template field names are kind of different, I'm wondering if I should add a compatibility layer to ease the migration.

Please take a look at https://github.com/frangio/hifi/commit/abea0bce61d2df02443782fe84a93750ff295399 and let me know if you have any feedback about the config and the output.

[PaulRBerg]

Whoah, thanks so much for taking the time to migrate our repo for us. This is super helpful - it would have taken me ages to achieve the same result.

Feedback:

• Looks like the bug whereby the returned value types were generated incorrectly is now gone! In the past solidity-docgen used to generate mistaken types for the returned values of functions (e.g. address instead of uint256).
• It isn't super obvious that returning "undefined" in the pages function in the Hardhat config means there's no page generated for that item. What if instead of that one could return "none" (as a string)?
• It would be nice if the behavior that you implemented via the printParams function would be available by default, without the need for a custom helpers.js file.
• The one-contract-one-page organization mode should be common enough to warrant a default option in solidity-docgen. I made a PR for this: https://github.com/OpenZeppelin/solidity-docgen/pull/365

And finally:

Some of the template field names are kind of different, I'm wondering if I should add a compatibility layer to ease the migration.

Yes, a compatibility layer would be nice. I actually forked the old template from Uniswap, so it's likely that many solidity-docgen users are relying on the previous fields.