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

feat: add an option to include rustdoc in ABI #876

Merged
merged 5 commits into from
Aug 4, 2022

Conversation

itegulov
Copy link
Contributor

@itegulov itegulov commented Aug 3, 2022

Relates to near/cargo-near#12

I was debating whether we should run some pre-processing on rustdoc strings (e.g., trimming lines, replacing single non-repeating newline characters with whitespace), but heuristics we write might mangle complex Markdown like code blocks and lists. So I think it makes sense to provide doc strings "as-is" and the consumer can decide what to do with them.

This PR does not affect schemars behavior due to the missing rustdoc on/off switch functionality (it will still include all rustdoc comments in ABI), but I have submitted this issue: GREsau/schemars#166. Hopefully, they are open to adding something like this, and if so, I will submit a follow-up PR.

Comment on lines 44 to 51
#[cfg(feature = "abi-doc")]
let function_doc =
match super::doc::parse_rustdoc(&self.attr_signature_info.non_bindgen_attrs) {
Some(doc) => quote! { Some(#doc.to_string()) },
None => quote! { None },
};
#[cfg(not(feature = "abi-doc"))]
let function_doc = quote! { None };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm skeptical this should be a feature at all. This wouldn't be compiled to wasm32 so only really comes at the cost of trivial compile time for native targets. Feels a little unwieldy to enforce specific feature usage to enable rather than something that can be enabled/disabled when the ABI is generated (cargo-near). WDYT? Feels like there is a use case where someone might want to output docs and not output docs for two different outputs for different use cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Post-processing this makes more sense and even allows us to remove schemars generated doc strings. You can try this cargo-near branch in conjunction with this PR: https://github.com/near/cargo-near/tree/daniyar/rustdoc.

@austinabell austinabell merged commit 496b797 into master Aug 4, 2022
@austinabell austinabell deleted the daniyar/abi-rustdoc branch August 4, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants