Provide ConvertService with mapping of USRs to minimal access level required for extended documentation to be available #555
Conversation
|
@swift-ci please test |
| /// DocC sets the ``RenderMetadata/hasExpandedDocumentationForSymbols`` property to `true` | ||
| /// for these symbols if their access level meets the minimum requirement, so that renderers can display a | ||
| /// "View More" link that navigates the user to the full version of the documentation page. | ||
| public var symbolIdentifiersWithExpandedDocumentation: [String: AccessControlLevel]? |
There was a problem hiding this comment.
Should we consider passing a more generically named struct here that itself contains AccessControlLevel? I think it's likely we'll want to pass additional requirements in the future.
| public var symbolIdentifiersWithExpandedDocumentation: [String: AccessControlLevel]? | |
| public var symbolIdentifiersWithExpandedDocumentation: [String: ExpandedDocumentationRequirement]? |
struct ExpandedDocumentationRequirement: Codable {
let accessLevel: AccessControlLevel?
}| @@ -250,13 +257,22 @@ public struct DocumentationConverter: DocumentationConverterProtocol { | |||
| // Copy images, sample files, and other static assets. | |||
| try outputConsumer.consume(assetsInBundle: bundle) | |||
|
|
|||
| let symbolIdentifiersMeetingRequirementsForExpandedDocumentation: [String]? = symbolIdentifiersWithExpandedDocumentation?.compactMap { (identifier, accessControlLevelRequirement) -> String? in | |||
There was a problem hiding this comment.
I believe we need to consider whether the symbol is underscored and if it's SPI here as well.
We could leave it a TODO to pass in those requirements later-on and hard-code for them now but it might be nice to just pass them in.
Eventually I'll imagine us needing to support the recently added @_documentation(visibility: ...) modifier as well.
There was a problem hiding this comment.
That''s a good idea, I will do that and the suggestion of making it easier to add different kinds of requirements here.
| } | ||
|
|
||
| return lhs < rhs | ||
| return AccessControlLevel(from: lhs) < AccessControlLevel(from: rhs) |
There was a problem hiding this comment.
Oh interesting! I was a little concerned about introducing the idea of comparable access levels to Swift-DocC in this PR but since it's already here and used I like how you've made it more generic and accessible. We should look at adding package here if/when it's approved.
There was a problem hiding this comment.
Yeah, I decided to add the type so that we wouldn't have to traffic strings or reexport SymbolKit. It would be good to have a set of vocabulary types internal to DocC
daniel-grumberg
force-pushed
the
convert-service-has-expanded-docs
branch
3 times, most recently
from
06cb3d9 to
d9e5a61
Compare
| @@ -288,4 +301,14 @@ extension ConvertRequest { | |||
| self.character = character | |||
| } | |||
| } | |||
|
|
|||
| public struct ExpandedDocumentationRequirements: Codable { | |||
There was a problem hiding this comment.
Nit: Would be good to document these.
| @@ -11,18 +11,13 @@ | |||
| import SymbolKit | |||
|
|
|||
| extension SymbolGraph.Symbol.AccessControl: Comparable { | |||
| private var level: Int? { | |||
| private var level: UInt? { | |||
There was a problem hiding this comment.
Should this change be in this PR? Swift generally advises against using UInt as a way to enforce positive values: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/thebasics/#UInt.
There was a problem hiding this comment.
No it shouldn't be in here.
|
|
||
| public struct ExpandedDocumentationRequirements: Codable { | ||
| public let accessControlLevels: [String] | ||
| public let canBeUnderscored: Bool |
There was a problem hiding this comment.
It would be good to support checking SPI here as well but we can leave that to a future PR.
daniel-grumberg
force-pushed
the
convert-service-has-expanded-docs
branch
from
d9e5a61 to
85341fa
Compare
|
@swift-ci please test |
…equired for extended documentation to be available rdar://105460209
daniel-grumberg
force-pushed
the
convert-service-has-expanded-docs
branch
from
85341fa to
9b3bd0c
Compare
|
@swift-ci please test |
daniel-grumberg
changed the title
daniel-grumberg
changed the title
Provide ConvertService with mapping of USRs to minimal access level required for extended documentation to be available
rdar://105460209
Bug/issue #, if applicable:
Summary
Provide a way for the convert service to add information in the render node to say if there is available additional documentation for a given symbol. This information is provided by the client on a per symbol access control level basis.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded