Use only canonical paths for symbol breadcrumbs #1081
Conversation
Like before, the shortest path is preferred regardless of language.
…mbol's "hierarchy"
| /// - reference: The symbol reference to find the canonical path to. | ||
| /// - sourceLanguage: The source language representation of the symbol to fin the canonical path for. | ||
| /// - Returns: The canonical path to the given symbol reference, or `nil` if the reference isn't a symbol or if the symbol doesn't have a representation in the given source language. | ||
| func breadcrumbs(of reference: ResolvedTopicReference, in sourceLanguage: SourceLanguage) -> [ResolvedTopicReference]? { |
There was a problem hiding this comment.
Maybe use a singular name for this since it only returns a single breadcrumb, not a list of breadcrumbs.
There was a problem hiding this comment.
I think I consider a path to be a list of breadcrumbs and a single breadcrumb to only be one path component.
I see that I used singular "breadcrumb" in the documentation comment. I'll update that documentation to say "breadcrumbs"
There was a problem hiding this comment.
I changed to plural "breadcrumbs" in the documentation comment in cc1e626
| // To match the caller's expectations we remove the starting point and then flip the paths. | ||
| .dropFirst().reversed() | ||
| .compactMap { | ||
| $0.identifier.flatMap { resolvedReferenceMap[$0] } |
There was a problem hiding this comment.
Should we assert on any nils? Or just remove them as you have done here? My guess is the PathHierarchy code is already well tested and has any asserts we would want.
There was a problem hiding this comment.
No, but I can add a comment that mentions why nil could be expected here.
The way that the ConvertService builds documentation for a single symbol may result in nodes in the hierarchy which doesn't correspond to any real symbols. The PathHierarchy internally refers to these as "sparse nodes". They only exist to construct a tree structure but links to them can't resolve. The ConvertService doesn't create any breadcrumbs, so handling nil here is possibly unnecessary.
There was a problem hiding this comment.
I added a code comment in 8647adf
| } | ||
|
|
||
| /// Returns the hierarchy under a given article. | ||
| /// - Parameter symbolReference: The reference to the article. | ||
| /// - Returns: The framework hierarchy that describes all paths where the article is curated. | ||
| mutating func visitArticle(_ symbolReference: ResolvedTopicReference) -> RenderHierarchy { | ||
| return visitSymbol(symbolReference) | ||
| mutating func visitArticle(_ symbolReference: ResolvedTopicReference) -> VariantCollection<RenderHierarchy?> { |
There was a problem hiding this comment.
Why is this function's parameter called symbolReference and not articleReference ?
There was a problem hiding this comment.
I didn't change it but I suspect that it was an old copy-and-paste mistake. I can fix it.
There was a problem hiding this comment.
Renamed in e63244e
| ], | ||
| expectedObjectiveCPaths: [ | ||
| "doc://GeometricalShapes/documentation/GeometricalShapes", | ||
| "doc://GeometricalShapes/documentation/GeometricalShapes/Circle", // named TLACircle in Objective-C |
There was a problem hiding this comment.
Why is the Objective-C breadcrumb the same as the Swift breadcrumb here?
There was a problem hiding this comment.
Because the center property is a member of the TLACircle structure (see the comment above the assertion)
There was a problem hiding this comment.
Since the symbol exist for both Swift and Objective-C it only has a single reference (which uses the Swift path)
|
@swift-ci please test |
|
@swift-ci please test |
Bug/issue #, if applicable: rdar://77922907
Summary
This changes how DocC computes "breadcrumbs" for symbols to only use the canonical path and to use different paths for each language representation.
Before these changes, DocC would use the same "breadcrumbs" for each language representation, always based on the shortest path to the symbol, including curation. This meant, among other things, that:
With these changes, symbol breadcrumbs ignore authored curation and only consider the canonical path for each language representation. For example:
Articles and tutorial content continue to base their breadcrumbs on authored curation.
Dependencies
None.
Testing
Build and preview documentation for the "GeometricalShapes" test catalog
Click through the different pages and inspect the breadcrumbs for both language representations.
centerandradiusare members in each language representation but thatintersects(_:),isEmpty, etc. are top-level functions in C/Objective-C.init(center:radius:)only exist in Swift and thatTLACircleMakeonly exist in C/Objective-C.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded