-
Notifications
You must be signed in to change notification settings - Fork 146
Support external links in the navigator #1247
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
Support external links in the navigator #1247
Conversation
Created an `ExternalRenderNode` structure that represents a pseudo render node out of an externally resolved value. This structure contains the minimal information needed to hold a value that can be passed to the Navigator Index. Fixes rdar://146123573. Co-authored-by: Sofía Rodríguez <[email protected]>
The structures `NavigatorExternalRenderNode` and `ExternalRenderNodeMetadataRepresentation` are meant to hold the information needed to add an external render node in the navigator index. Each of these contains the information of a external render node for a specific language variant, since things like the node title change depending on the selected variant. Fields which cannot be populated due to insufficient information have been documented. These can have varying effect on the resulting navigation node, particularly the navigator title. Fixes rdar://146123573. Co-authored-by: Sofía Rodríguez <[email protected]>
This method mimics the index method for render nodes [1] by indexing the main node representation, and the Objective-C representation. [1] https://github.com/swiftlang/swift-docc/blob/38ea39df14d0193f52900dbe54b7ae2be0abd856/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift\#L636 Fixes rdar://146123573. Co-authored-by: Sofía Rodríguez <[email protected]>
During the convert process create the external render nodes from the context external cache and consume them to get them added into the navigator index. Fixes rdar://146123573. Co-authored-by: Sofía Rodríguez <[email protected]>
The NavigationIndex code has some logic about what to do with fallouts (i.e. nodes which are not curated anywhere). It places them at the top level of the navigator, so that they are not lost. This works well for render nodes, but for external render nodes, we don't want them to persist at the top level if they are not referenced anywhere. We'd like them to be excluded if not referenced. This can happen due to the fact that we consume **all** successfully resolved external entities for indexing, which includes external references in other sections of the page such as the Overview section, the abstract and the See also section. Fixes rdar://146123573.
Now that the `isExternal` property is populated as part of the `NavigatorItem`s, we can now use that value to propagate whether the node `isExternal` in the final `RenderIndex`, something which we couldn't support before. External nodes are now marked as "external": true in `index.json` as expected. Fixes rdar://146123573.
/// A consumer for nodes generated from external references. | ||
/// | ||
/// Types that conform to this protocol manage what to do with external references, for example index them. | ||
package protocol ExternalNodeConsumer { |
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.
Created this package
-level protocol so that we don't have to make a breaking API change to public
protocol ConvertOutputConsumer
.
@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.
🔥🔥🔥🔥🔥
Bug/issue #, if applicable: rdar://146123573
Summary
Adds support for external links in the navigator. To test, used an example DocC catalog with one article and one external reference.
The goal was to have external links which are curated as part of the Topics section show as a navigator node in the final
index.json
file which powers the navigator. External links should appear under the page under which they were curated, and have a propertyexternal=true
which marks them as external nodes:swift-docc/Sources/SwiftDocC/Indexing/RenderIndexJSON/RenderIndex.swift
Lines 247 to 249 in c7660bb
Navigator comparison (using swift-docc-render):
There are some fields which we have insufficient information in
LinkResolver.ExternalEntity
to derive:For these cases, they have been documented in source.
The missing fields result in navigator titles differing between the navigator where the page is hosted, and the navigator where the reference is externally linked, particularly for symbols.
For a symbol, the original page & Topics section will show a simplified version of the declaration fragments as the page title, whereas here I have opted for simplicity of using only the shorter title rather than the full declaration fragments (which are the only declaration fragments available as part of the external entity, and used in the Topics section) in order to avoid too much verbosity.
In the future we can improve the quality of the title for external links in order to maintain consistency between the Topics section and navigation index.
Note: We could potentially derive symbol kind, platforms and metadata fragments in the future as the information is present in
OutofProcessReferenceResolver.ResolvedInformation
, but it would require some rework of the types to propagate the information upwards toLinkResolver.ExternalEntity
so I think that's best done in separate PRs and reviewed separately.Dependencies
N/A
Testing
Steps:
Example.docc.zip
/path/to/swift-docc/bin/test-data-external-resolver
and modifying the data as desiredDOCC_LINK_RESOLVER_EXECUTABLE=/path/to/swift-docc/bin/test-data-external-resolver swift run docc preview Example.docc
index.json
is showing the external reference as a node, and that the node has a propertyexternal=true
:Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded