Skip to content

Add support for abstract Headerdoc tag #1215

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

binamaniar
Copy link
Contributor

@binamaniar binamaniar commented May 9, 2025

Bug/issue #, if applicable: rdar://147920933

Summary

This note adds support for Doxygen @abstract tag. Abstract tags are turned into normal paragraphs.

Dependencies

Swift-markdown dependency swiftlang/swift-markdown#227

Testing

Create a documentation comment containing @abstract

This is an overview.

@abstract this is the content for abstract tag

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • [ yes] Added tests
  • [yes ] Ran the ./bin/test script and it succeeded
  • [ na] Updated documentation if necessary

@binamaniar binamaniar marked this pull request as draft May 9, 2025 01:59
@binamaniar binamaniar changed the title [WIP]rdar://147920933 (Support HeaderDoc tag) [WIP]Support HeaderDoc tag abstract May 9, 2025
@binamaniar binamaniar changed the title [WIP]Support HeaderDoc tag abstract Add support for abstract Headerdoc tag May 9, 2025
@binamaniar
Copy link
Contributor Author

@swift-ci please test

@binamaniar binamaniar marked this pull request as ready for review May 9, 2025 06:39
Copy link
Contributor

@patshaughnessy patshaughnessy left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Nit: A couple of the code files you modify here still have 2024 in their copyright message; pls update the year to 2025. (RenderContentCompiler.swift and DoxygenTests.swift)

@@ -5,17 +5,17 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-argument-parser.git",
"state" : {
"revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b",
"version" : "1.4.0"
"revision" : "41982a3656a71c768319979febd796c6fd111d5c",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we avoid updating all the dependencies? Maybe we should just update the one new version of swift-markdown below. Or do we typically run swift package update and update everything when possible?

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually with a paired PR like this, we should only run swift package update swift-markdown to only pull in Swift-Markdown specifically, and leave the other dependencies alone.

Copy link
Contributor

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

This looks good, though i would prefer if you could reset the Package.resolved and only run swift package resolve or swift package update swift-markdown when you update the branch back, so it only updates Swift-Markdown and swift-cmark.

My other comment is more of a curiosity. I think this PR can land as-is just so the content is properly rendered somehow, even if it's not as an abstract per se.

@@ -5,17 +5,17 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-argument-parser.git",
"state" : {
"revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b",
"version" : "1.4.0"
"revision" : "41982a3656a71c768319979febd796c6fd111d5c",
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually with a paired PR like this, we should only run swift package update swift-markdown to only pull in Swift-Markdown specifically, and leave the other dependencies alone.

XCTAssertEqual(overviewSection.content, [
.heading(.init(level: 2, text: "Overview", anchor: "overview")),

.paragraph(.init(inlineContent: [.text("This is description with abstract.")])),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we try to actually parse @abstract as an abstract? I feel like it would stay true to the use of the command.

On the other hand, we probably shouldn't add too much Doxygen/HeaderDoc support, since we really want to push people to using Markdown... 😅 I'm fine either way on this, though i'm curious what the workgroup (or just the rest of our team) thinks.

…s with abstract headerdoc tag retains documentation
@binamaniar binamaniar force-pushed the 147920933_abstract_headerdoc_tag branch from 13d5c18 to b28f17e Compare May 13, 2025 04:50
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.

3 participants