docs icon indicating copy to clipboard operation
docs copied to clipboard

Polish autogen script

Open PaulRBerg opened this issue 2 years ago • 6 comments
trafficstars

I got the autogen.sh script in a pretty good shape, but it still suffers from a number of bugs and missing features:

  • [x] See discussion below; ~Events are not hyperlinked, e.g. {TransferAdmin} event should be turned into [TransferAdmin][URL] event~
  • [x] Core references in Periphery are not hyperlinked, e.g. {ISablierV2Lockup.cancel} should be turned into [ISablierV2Lockup.cancel][URL]#cancel
  • [ ] There are many stale _ italics that we don't want (for context, see https://github.com/foundry-rs/foundry/issues/4540)
  • [ ] Where it says "See the documentation in ...", we should put the actual referenced documentation (this might be easier to do once @inheritdoc becomes usable with contracts: https://github.com/ethereum/solidity/issues/14045)

PaulRBerg avatar Mar 15 '23 18:03 PaulRBerg

The git source links to the docs directory

docs/contracts/v2/reference/periphery/interfaces

https://github.com/sablier-labs/v2-periphery/blob/05c331e79e05886c7837dfda1bc21197c1c3c748/docs/contracts/v2/reference/periphery/interfaces

sambacha avatar Sep 30 '23 01:09 sambacha

Pandoc filters may be able to help here along with the other natspec issues you have posted about @PaulRBerg https://pandoc.org/filters.html#but-i-dont-want-to-learn-haskell

sambacha avatar Sep 30 '23 01:09 sambacha

Thanks @sambacha, will have a look

PaulRBerg avatar Sep 30 '23 16:09 PaulRBerg

Future note for self:

If https://github.com/sablier-labs/v2-docs/issues/119 gets implemented first, we might want to discard the first task above.

PaulRBerg avatar Jan 01 '24 12:01 PaulRBerg

I have been playing with it for the last few days and here are my thoughts. However, the first task in itself is very complicated.

  • On inline doc links generated by Foundry: It only works if the event name is different from the function name. Take this example, function setNFTDescriptor emits SetNFTDescriptor. Docusaurus generates the #setnftdescriptor URL for the function and #setnftdescriptor-1 for the event (note -1). This breaks inline doc links for events. I did some research to force Docusaurus to use a different URL for all the events, such as #setnftdescriptor-events, but couldn't find a way to do so.

  • Since there is no mapping of the file in which events are placed, to be able to hyperlink them correctly, one would have to identify the URL of the event which is quite difficult.

So how can we fix it?

  • One solution is to modify the NatSpecs in the code to use the filename if the event is not defined in the same file. For example,
/// @dev Emits {TransferAdmin} event

can be modified to

/// @dev Emits {IAdminable.TransferAdmin} event
  • Use a different name for event than the function name. Such as, do not use SetNFTDescriptor for event name if function name is SetNFTDescriptor in the same file. So that the inline doc does not break. It would also help us to identify the location of the file in which event is defined. This would make it easy to hyperlink them.

Regarding second, I could not find any such reference. Can you please point me to an example?

smol-ninja avatar Dec 15 '24 07:12 smol-ninja

Thanks for looking into this.

first task is complicated .. So how can we fix it?

{IAdminable.TransferAdmin} is ugly and verbose.

We should prioritize brevity in the Solidity code over this hyperlinking feature in the docs site, so let's cross this item off the list.

Regarding second, I could not find any such reference

I can't remember. I will tick if off for now, assuming that it's been fixed in the meantime.

PaulRBerg avatar Dec 16 '24 15:12 PaulRBerg