-
Notifications
You must be signed in to change notification settings - Fork 219
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
Skip first doc comment line in path macro description expansion #881
Skip first doc comment line in path macro description expansion #881
Conversation
Previously, the entire doc comment was used as the description, in addition to the summary using the first line of the doc comment, resulting in duplicated text. This commit changes the path macro doc comment extraction logic to split the first line from the rest of the body, and skip all lines until the first non-whitespace line. This results in the description no longer containing the summary. In degenerate cases, such as a doc comment consisting of new lines, this will invoke a full linear search over all lines in the doc comment. This should be relatively acceptable as most reasonable forms of doc comments shouldn't have more than a couple of newlines between the first line and subsequent body. For testing and verification, I've tested this in a private project using the `patch` override section in `Cargo.toml`, and tests needed to be changed to account for this change.
Looks like CI failure is related to not providing a feature flag? I don't believe that it's related to these changes. |
Yeah, seems like utoipa-gen requires features query+json to compile (#893) |
Yeah, probably some upstream crate feature changes that hasn't been propagated here has caused the error. |
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.
Great, 🎉 Thanks for the contribution.
Priot to this PR the summary and description was blindly split by the first line of the doc comment of path operaiton. This caused long multi line summaries to be split from illogically bleeding part of the summary to the description. More context can be found from issue #942. This commit fixes this as from now one comments as follows will be correctly split by paragraph. Where first paragraph will resolve to summary and rest will become the description. ```rust /// This is test operation long multiline /// summary. That need to be correctly split. /// /// Additional info in long description /// /// With more info on separate lines /// containing text. /// /// Yeah ``` Follow up for #881 and #439 Fixes #942
Priot to this PR the summary and description was blindly split by the first line of the doc comment of path operaiton. This caused long multi line summaries to be split from illogically bleeding part of the summary to the description. More context can be found from issue #942. This commit fixes this as from now one comments as follows will be correctly split by paragraph. Where first paragraph will resolve to summary and rest will become the description. ```rust /// This is test operation long multiline /// summary. That need to be correctly split. /// /// Additional info in long description /// /// With more info on separate lines /// containing text. /// /// Yeah ``` Follow up for #881 and #439 Fixes #942
Previously, the entire doc comment was used as the description in addition to the summary using the first line of the doc comment, resulting in duplicated text.
This commit changes the path macro doc comment extraction logic to split the first line from the rest of the body, and skip all lines until the first non-whitespace line. This results in the description no longer containing the summary.
In degenerate cases, such as a doc comment consisting of new lines, this will invoke a full linear search over all lines in the doc comment. This should be relatively acceptable as most reasonable forms of doc comments shouldn't have more than a couple of newlines between the first line and subsequent body.
For testing and verification, I've tested this in a private project using the
patch
override section inCargo.toml
, and tests needed to be changed to account for this change.This change was driven by my own preferences in documentation and in addition to this comment by @chrsteer: