-
Notifications
You must be signed in to change notification settings - Fork 121
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 @Small directive #380
Conversation
Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json
Outdated
Show resolved
Hide resolved
1307077
to
1202a1d
Compare
cc6975e
to
7c13a8e
Compare
|
@swift-ci please test |
| @@ -61,7 +61,11 @@ public enum RenderBlockContent: Equatable { | |||
| /// A table that contains a list of row data. | |||
| case table(Table) | |||
|
|
|||
| /// A row in a grid-based layout system that describes a collection of columns. | |||
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.
Mega nit, this probably belongs in another patch.
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 agreed- I just missed it in #378 so I thought it was worth cleaning up here. I can split it into a separate commit 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.
That would be nice
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.
Split out here: e56bd43
| "type": "object", | ||
| "required": [ | ||
| "type", | ||
| "inlineContent", |
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.
Nit trailing comma
| @@ -1057,5 +1057,15 @@ class RenderNodeTranslatorTests: XCTestCase { | |||
| XCTAssertEqual(row.columns.first?.content.count, 1) | |||
| XCTAssertEqual(row.columns.last?.size, 5) | |||
| XCTAssertEqual(row.columns.last?.content.count, 3) | |||
|
|
|||
| guard case let .small(small) = discussion.content.last else { | |||
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 not have a separate test from row and column one for this? I find that having smaller test cases helps pin out failures faster.
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.
Ah good catch- I meant to create a new one here. Should be fixed now. Thanks!
| XCTAssertEqual(renderBlockContent.count, 1) | ||
| XCTAssertEqual( | ||
| renderBlockContent, | ||
| renderBlockContent.first, |
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 this not be in a separate patch?
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.
Split into a separate commit here: e4a6fc4.
| } | ||
| } | ||
|
|
||
| func testMixesInlineAndBlockContent() throws { |
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.
Slight misnomer here, maybe something like testCanNotContainStructuredMarkup
|
Looks pretty good to me, small fixes and then LGTM |
The current tests only look at the `first` RenderBlockContentelement. This is a refactor to support testing `@Small` which can return multiple RenderBlockContent items.
7c13a8e
to
cad3718
Compare
Adds support for the `@Small` directive as described here: https://forums.swift.org/t/supporting-more-dynamic-content-in-swift-docc-reference-documentation/59527#small-18 A new `@Small` directive based on HTML’s small tag. This directive can be used to specify small print like legal, license, or copyright text that should be rendered in a smaller font size. `@Small` accepts no parameters and accepts inline DocC markup body content. Example: You can create a sloth using the ``init(name:color:power:)`` initializer, or create randomly generated sloth using a ``SlothGenerator``: let slothGenerator = MySlothGenerator(seed: randomSeed()) let habitat = Habitat(isHumid: false, isWarm: true) // ... @Small { Licensed under Apache License v2.0 with Runtime Library Exception } Dependencies: - swiftlang/swift-docc-render#405 Resolves rdar://97744845
cad3718
to
ec79942
Compare
|
@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.
LGTM
Bug/issue #, if applicable: rdar://97744845
Summary
Adds support for the
@Smalldirective as described here: https://forums.swift.org/t/supporting-more-dynamic-content-in-swift-docc-reference-documentation/59527#small-18A new
@Smalldirective based on HTML’s small tag. This directive can be used to specify small print like legal, license, or copyright text that should be rendered in a smaller font size.@Smallaccepts no parameters and accepts arbitrary DocC markup body content.Example:
Dependencies
@Smalldirective swift-docc-render#405Testing
Using the latest version of Swift-DocC-Render (support for
@Smalllanded on themainbranch recently), build documentation for a DocC catalog that includes the@Smalldirective. Confirm it renders as expected.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded