-
Notifications
You must be signed in to change notification settings - Fork 146
Populate symbol kind in external render nodes #1251
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
base: main
Are you sure you want to change the base?
Populate symbol kind in external render nodes #1251
Conversation
There is a mapping from `SymbolGraph.Symbol.KindIdentifier` to `DocumentationNode.Kind`, but not the other way around; this commit adds one. For documentation kinds which both map to the same symbol kind, this has been expressed as: - both documentation kinds are converted to the same symbol kind in `symbolKind(for:)`. - a specific documentation kind is chosen as the default mapping in `kind(forKind:)` For example, for `DocumentationNode.Kind. typeConstant` [1]: - both `symbolKind(for: .typeConstant)` and `symbolKind(for: .typeProperty)` return `.typeProperty` - `kind(forKind: .typeProperty)` returns `.typeProperty` This function will be used to map and external entity's documentation kind to a symbol kind, so that that information can be populated as part of the navigator. ## Verification: This has been verified by testing the round trip conversion of all symbol kinds, and all documentation kinds which are symbols. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Model/Kind.swift#L151
The documentation kind is captured as part of the `OutOfProcessReferenceResolver. ResolvedInformation` [1], but it is never propagated upwards to `LinkResolver.ExternalEntity`. We need access to the documentation kind in order to resolve the symbol kind as part of computing the navigator title. [2] [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Infrastructure/External%20Data/OutOfProcessReferenceResolver.swift#L564-L565 [2]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/RenderNode%2BNavigatorIndex.swift#L163
Derives the symbol kind from the documentation kind of the external render node. Also updates how the symbol kind is propagated to the navigator metadata, by using `.renderingIdentifier` over `.identifier`, to match the behaviour of local nodes [1]. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift#L1300
@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.
I'm unsure about the usage of renderIdentifier
.
@_spi(ExternalLinks) | ||
public init(topicRenderReference: TopicRenderReference, renderReferenceDependencies: RenderReferenceDependencies, sourceLanguages: Set<SourceLanguage>) { | ||
public init(topicRenderReference: TopicRenderReference, renderReferenceDependencies: RenderReferenceDependencies, sourceLanguages: Set<SourceLanguage>, documentationKind: DocumentationNode.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.
This is a public API, shouldn't we deprecate it and create a new initializer?
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 didn't do this because it's experimental API and comes with a warning:
swift-docc/Sources/SwiftDocC/Infrastructure/Link Resolution/LinkResolver.swift
Lines 44 to 45 in 65aaf92
@_spi(ExternalLinks) // This isn't stable API yet. | |
public struct ExternalEntity { |
WDYT? Happy to do as you suggested.
@@ -116,7 +117,7 @@ struct NavigatorExternalRenderNode: NavigatorIndexableRenderNodeRepresentation { | |||
navigatorTitle: renderNode.navigatorTitleVariants.value(for: traits), | |||
externalID: renderNode.externalIdentifier.identifier, | |||
role: renderNode.role, | |||
symbolKind: renderNode.symbolKind?.identifier, | |||
symbolKind: renderNode.symbolKind?.renderingIdentifier, |
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.
From
var renderingIdentifier: String { |
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.
renderingIdentifier
is what we use for deriving the navigator symbol kind for local items -- for consistency I think we should use the same.
swift-docc/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift
Lines 1299 to 1301 in 65aaf92
node.metadata.symbolKindVariants = VariantCollection<String?>(from: symbol.kindVariants) { _, kindVariants in | |
kindVariants.identifier.renderingIdentifier | |
} ?? .init(defaultValue: nil) |
Bug/issue #, if applicable: rdar://156487928
Summary
Fixes the navigator page type for external render nodes such that it takes into account the kind of symbol of the node.
In order to properly determine the navigator page type for a symbol, we need to know its symbol kind:
swift-docc/Sources/SwiftDocC/Indexing/Navigator/RenderNode+NavigatorIndex.swift
Lines 148 to 168 in 65aaf92
However, the symbol kind was not available for external nodes and all items were showing as their generic role, e.g. "article" or "symbol".
This PR adds logic to derive an external render node's symbol kind:
OutOfProcessReferenceResolver.ResolvedInformation
's documentation kind toLinkResolver.ExternalEntity
, the type which is used to deal with external entities.Navigator comparison (using swift-docc-render):
The navigator icons on the right match the symbol kinds of the external pages.
The navigator
index.json
shows the correct page type as well:Dependencies
N/A.
Testing
Previewed a custom bundle locally which contained different types of external links (module, class, function and article).
index.json
looked as expectedSteps:
DOCC_LINK_RESOLVER_EXECUTABLE=Example.docc/bin/test-data-external-resolver swift run docc preview Example.docc
Example.docc/.docc-build/index/index.json
looks as expected.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded