This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[contracts] Add docs generator for the contracts API to the #[define_env]
macro
#13032
Merged
Merged
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
04769ea
macro to expand traits for host functions documentation
agryaznov e7f6b78
other way: same Doc trait in seal modules
agryaznov 606d58d
added docs for macro, and remove `doc` attribute
agryaznov 6cf881d
fmt
agryaznov c558c5e
Apply suggestions from code review
agryaznov cd2007b
make docs to be generated into re-exported `api_doc` module; fix
agryaznov ce9031b
make it compile without `doc` attr passed to macro
agryaznov dced64c
make alias functions indicated explicitly in docs
agryaznov 98a6c63
tidy up docs
agryaznov ab36942
refactored a bit
agryaznov f8def8c
macro to auto-add doc warning for unstable functions
agryaznov f118f74
invoke macro with no doc generation by default
agryaznov fff947e
addressed review comments
agryaznov 1af74cc
hide api_doc module behind cfg(doc)
agryaznov 1272108
Merge branch 'master' into ag-host-fn-docs
agryaznov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename
doc
toemit_doc
. I think this makes it clearer what is happening. I won't die on that hill if you disagree, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to me, concise variant is more elegant and could hardly be misinterpreted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But isn't it the same attribute name used for rust doc comments itself? To be that seems confusing. Also, why doesn't this PR add this attribute to the
define_env
invocation in runtime.rs?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's exactly the point. The idea is to make it short, appropriate and familiar to the user: at the end of the day, it is for generating the rustdoc. I don't see it is confusing, as here
doc
attr is used as input todefine_env(doc)
rather than solo (as when it's for a single doc comment)I was thinking to leave it off by default. But could be done vice versa, why not. Added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough.
Most important point for me is that the standard docs.rs builds of this crate contain this documentation. Therefore it should be on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. The only objection comes to my mind is that it probably should be turned off for production use not to bloat runtime size in vain. Should we add a note to the macro docs at least?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not bloat the runtime size as it is dead code and hence removed by the compiler. At least when using lto.
But you have a point there. I think you should just add
#[cfg(doc)]
to the emittedapi_doc
module (and its re-rexports). Then this code will be compiled out for every target but rustdoc.