-
Notifications
You must be signed in to change notification settings - Fork 224
Add support for Doxygen abstract tag #227
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
Add support for Doxygen abstract tag #227
Conversation
@swift-ci please test |
@swift-ci please test |
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.
Looks good to me 👍
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.
This looks great, thanks!
d0ad98f
to
3f9b8b5
Compare
…s with only an abstract retains its documentation
3f9b8b5
to
319fb59
Compare
…s with abstract headerdoc tag retains documentation
319fb59
to
2f24da8
Compare
@swift-ci please test. |
@swift-ci please test |
@@ -887,6 +892,8 @@ struct ParseContainerStack { | |||
kind = .discussion | |||
case "note": | |||
kind = .note | |||
case "abstract": |
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.
Should we also support the "brief" name here? (assuming that this is where the tag name is mapped to a kind)
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.
yes this is where the mapping is, should that be a separate pr ? open to add here as well.
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.
If all it would take is one more string literal on this line and possibly a test that verifies that it works, then it might be worth doing in the same PR. If it's any more than that it seems to me like it would be better to add in a follow-up PR. What do you think @QuietMisdreavus?
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 will wait till tomorrow to hear back on this. @d-ronnqvist @QuietMisdreavus shall I go ahead with merging this and have a separate pr for brief? Brief would need a mapping as well as @brief to be processed same as @abstract. Please let me know whatever you prefer, I would like to incorporate feedback soon.
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 go ahead and add support for @brief
here, the same as the case "return", "returns", "result"
line below this one. All you should need is to add the "brief"
check here and copy/paste a test with the new command name.
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 doing it now.
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.
Done please see latest commit. @QuietMisdreavus
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.
Awesome, looks great!
@swift-ci Please test |
Bug/issue #, if applicable: Related to rdar://147920933
Summary
This PR adds support for parsing the Doxygen @abstract tags. See also: swiftlang/swift-docc#1215
Dependencies
None; the PR to swift-docc depends on this PR.
Testing
Create a documentation comment containing @abstract
This is an overview.
@abstract This is a description of abstract tag
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded