Skip to content

Populate abbreviated declaration fragments in external render nodes #1255

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ public class OutOfProcessReferenceResolver: ExternalDocumentationSource, GlobalE
url: resolvedInformation.url.path,
kind: kind,
role: role,
fragments: resolvedInformation.declarationFragments?.declarationFragments.map { DeclarationRenderSection.Token(fragment: $0, identifier: nil) },
fragments: resolvedInformation.subHeadingDeclarationFragments?.declarationFragments
.map { DeclarationRenderSection.Token(fragment: $0, identifier: nil) },
isBeta: resolvedInformation.isBeta,
isDeprecated: (resolvedInformation.platforms ?? []).contains(where: { $0.deprecated != nil }),
images: resolvedInformation.topicImages ?? []
Expand All @@ -182,7 +183,7 @@ public class OutOfProcessReferenceResolver: ExternalDocumentationSource, GlobalE
.init(traits: variant.traits, patch: [.replace(value: [.text(abstract)])])
)
}
if let declarationFragments = variant.declarationFragments {
if let declarationFragments = variant.subHeadingDeclarationFragments {
renderReference.fragmentsVariants.variants.append(
.init(traits: variant.traits, patch: [.replace(value: declarationFragments?.declarationFragments.map { DeclarationRenderSection.Token(fragment: $0, identifier: nil) })])
)
Expand Down Expand Up @@ -577,7 +578,11 @@ extension OutOfProcessReferenceResolver {
public let platforms: [PlatformAvailability]?
/// Information about the resolved declaration fragments, if any.
public let declarationFragments: DeclarationFragments?

/// Information about the resolved abbreviated declaration fragments, if any.
///
/// They are used for displaying in contexts where the full declaration fragments would be too verbose, like in the Topics section or the navigation index.
public let subHeadingDeclarationFragments: DeclarationFragments?

Comment on lines +581 to +585
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the subheading declaration fragments it's used instead of the fragments in the TopicRenderReference why do we want to keep declarationFragments here instead of only keeping the subheading one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't need the full declaration fragments for anything then we should repurpose the declarationFragments property instead.

I consider OutOfProcessResolver.ResolvedInformation to almost be a deprecated type at this point (see #802).

It could also be good to check if implementing #802 would fix this issue. If so, it might be worth using this use-case as a driver to implement that and get rid of this code duplication that loses certain data richness.

// We use the real types here because they're Codable and don't have public member-wise initializers.

/// Platform availability for a resolved symbol reference.
Expand Down Expand Up @@ -620,6 +625,7 @@ extension OutOfProcessReferenceResolver {
/// - availableLanguages: The languages where the resolved node is available.
/// - platforms: The platforms and their versions where the resolved node is available, if any.
/// - declarationFragments: The resolved declaration fragments, if any.
/// - subHeadingDeclarationFragments: The abbreviated resolved declaration fragments, if any.
/// - topicImages: Images that are used to represent the summarized element.
/// - references: References used in the content of the summarized element.
/// - variants: The variants of content for this resolver information.
Expand All @@ -632,6 +638,7 @@ extension OutOfProcessReferenceResolver {
availableLanguages: Set<SourceLanguage>,
platforms: [PlatformAvailability]? = nil,
declarationFragments: DeclarationFragments? = nil,
subHeadingDeclarationFragments: DeclarationFragments? = nil,
topicImages: [TopicImage]? = nil,
references: [any RenderReference]? = nil,
variants: [Variant]? = nil
Expand All @@ -644,6 +651,7 @@ extension OutOfProcessReferenceResolver {
self.availableLanguages = availableLanguages
self.platforms = platforms
self.declarationFragments = declarationFragments
self.subHeadingDeclarationFragments = subHeadingDeclarationFragments
self.topicImages = topicImages
self.references = references
self.variants = variants
Expand Down Expand Up @@ -675,7 +683,12 @@ extension OutOfProcessReferenceResolver {
///
/// If the resolver information has a declaration but the variant doesn't, this property will be `Optional.some(nil)`.
public let declarationFragments: VariantValue<DeclarationFragments?>

/// The abbreviated declaration fragments of the variant or `nil` if the declaration is the same as the resolved information.
///
/// They are used for displaying in contexts where the full declaration fragments would be too verbose, like in the Topics section or the navigation index.
/// If the resolver information has a declaration but the variant doesn't, this property will be `Optional.some(nil)`.
public let subHeadingDeclarationFragments: VariantValue<DeclarationFragments?>

/// Creates a new resolved information variant with the values that are different from the resolved information values.
///
/// - Parameters:
Expand All @@ -686,14 +699,16 @@ extension OutOfProcessReferenceResolver {
/// - abstract: The resolved (plain text) abstract.
/// - language: The resolved language.
/// - declarationFragments: The resolved declaration fragments, if any.
/// - subHeadingDeclarationFragments: The resolved abbreviated declaration fragments, if any.
public init(
traits: [RenderNode.Variant.Trait],
kind: VariantValue<DocumentationNode.Kind> = nil,
url: VariantValue<URL> = nil,
title: VariantValue<String> = nil,
abstract: VariantValue<String> = nil,
language: VariantValue<SourceLanguage> = nil,
declarationFragments: VariantValue<DeclarationFragments?> = nil
declarationFragments: VariantValue<DeclarationFragments?> = nil,
subHeadingDeclarationFragments: VariantValue<DeclarationFragments?> = nil
) {
self.traits = traits
self.kind = kind
Expand All @@ -702,6 +717,7 @@ extension OutOfProcessReferenceResolver {
self.abstract = abstract
self.language = language
self.declarationFragments = declarationFragments
self.subHeadingDeclarationFragments = subHeadingDeclarationFragments
}
}
}
Expand All @@ -717,6 +733,7 @@ extension OutOfProcessReferenceResolver.ResolvedInformation {
case availableLanguages
case platforms
case declarationFragments
case subHeadingDeclarationFragments
case topicImages
case references
case variants
Expand All @@ -733,6 +750,8 @@ extension OutOfProcessReferenceResolver.ResolvedInformation {
availableLanguages = try container.decode(Set<SourceLanguage>.self, forKey: .availableLanguages)
platforms = try container.decodeIfPresent([OutOfProcessReferenceResolver.ResolvedInformation.PlatformAvailability].self, forKey: .platforms)
declarationFragments = try container.decodeIfPresent(OutOfProcessReferenceResolver.ResolvedInformation.DeclarationFragments.self, forKey: .declarationFragments)
subHeadingDeclarationFragments = try container
.decodeIfPresent(OutOfProcessReferenceResolver.ResolvedInformation.DeclarationFragments.self, forKey: .subHeadingDeclarationFragments)
topicImages = try container.decodeIfPresent([TopicImage].self, forKey: .topicImages)
references = try container.decodeIfPresent([CodableRenderReference].self, forKey: .references).map { decodedReferences in
decodedReferences.map(\.reference)
Expand All @@ -752,6 +771,7 @@ extension OutOfProcessReferenceResolver.ResolvedInformation {
try container.encode(self.availableLanguages, forKey: .availableLanguages)
try container.encodeIfPresent(self.platforms, forKey: .platforms)
try container.encodeIfPresent(self.declarationFragments, forKey: .declarationFragments)
try container.encodeIfPresent(self.subHeadingDeclarationFragments, forKey: .subHeadingDeclarationFragments)
try container.encodeIfPresent(self.topicImages, forKey: .topicImages)
try container.encodeIfPresent(references?.map { CodableRenderReference($0) }, forKey: .references)
try container.encodeIfPresent(self.variants, forKey: .variants)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
declarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "declaration fragment", preciseIdentifier: nil)
]),
subHeadingDeclarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "subheading declaration fragment", preciseIdentifier: nil)
]),
topicImages: nil,
references: nil,
variants: [
Expand All @@ -82,7 +85,10 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
language: .init(name: "Language Name 2", id: "com.test.another-language.id"),
declarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "variant declaration fragment", preciseIdentifier: nil)
])
]),
subHeadingDeclarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "variant subheading declaration fragment", preciseIdentifier: nil)
]),
)
]
)
Expand Down Expand Up @@ -120,7 +126,7 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
XCTAssertEqual(availableSourceLanguages[1], expectedLanguages[1])
XCTAssertEqual(availableSourceLanguages[2], expectedLanguages[2])

XCTAssertEqual(entity.topicRenderReference.fragments, [.init(text: "declaration fragment", kind: .text, preciseIdentifier: nil)])
XCTAssertEqual(entity.topicRenderReference.fragments, [.init(text: "subheading declaration fragment", kind: .text, preciseIdentifier: nil)])

let variantTraits = [RenderNode.Variant.Trait.interfaceLanguage("com.test.another-language.id")]
XCTAssertEqual(entity.topicRenderReference.titleVariants.value(for: variantTraits), "Resolved Variant Title")
Expand All @@ -129,7 +135,7 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
let fragmentVariant = try XCTUnwrap(entity.topicRenderReference.fragmentsVariants.variants.first(where: { $0.traits == variantTraits }))
XCTAssertEqual(fragmentVariant.patch.map(\.operation), [.replace])
if case .replace(let variantFragment) = fragmentVariant.patch.first {
XCTAssertEqual(variantFragment, [.init(text: "variant declaration fragment", kind: .text, preciseIdentifier: nil)])
XCTAssertEqual(variantFragment, [.init(text: "variant subheading declaration fragment", kind: .text, preciseIdentifier: nil)])
} else {
XCTFail("Unexpected fragments variant patch")
}
Expand Down Expand Up @@ -226,6 +232,9 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
declarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "declaration fragment", preciseIdentifier: nil)
]),
subHeadingDeclarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "subheading declaration fragment", preciseIdentifier: nil)
]),
topicImages: [
TopicImage(
type: .card,
Expand Down Expand Up @@ -260,6 +269,9 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
language: .init(name: "Language Name 2", id: "com.test.another-language.id"),
declarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "variant declaration fragment", preciseIdentifier: nil)
]),
subHeadingDeclarationFragments: .init(declarationFragments: [
.init(kind: .text, spelling: "variant subheading declaration fragment", preciseIdentifier: nil)
])
)
]
Expand Down Expand Up @@ -288,7 +300,7 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
XCTAssertEqual(availableSourceLanguages[1], expectedLanguages[1])
XCTAssertEqual(availableSourceLanguages[2], expectedLanguages[2])

XCTAssertEqual(entity.topicRenderReference.fragments, [.init(text: "declaration fragment", kind: .text, preciseIdentifier: nil)])
XCTAssertEqual(entity.topicRenderReference.fragments, [.init(text: "subheading declaration fragment", kind: .text, preciseIdentifier: nil)])

let variantTraits = [RenderNode.Variant.Trait.interfaceLanguage("com.test.another-language.id")]
XCTAssertEqual(entity.topicRenderReference.titleVariants.value(for: variantTraits), "Resolved Variant Title")
Expand All @@ -297,7 +309,7 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
let fragmentVariant = try XCTUnwrap(entity.topicRenderReference.fragmentsVariants.variants.first(where: { $0.traits == variantTraits }))
XCTAssertEqual(fragmentVariant.patch.map(\.operation), [.replace])
if case .replace(let variantFragment) = fragmentVariant.patch.first {
XCTAssertEqual(variantFragment, [.init(text: "variant declaration fragment", kind: .text, preciseIdentifier: nil)])
XCTAssertEqual(variantFragment, [.init(text: "variant subheading declaration fragment", kind: .text, preciseIdentifier: nil)])
} else {
XCTFail("Unexpected fragments variant patch")
}
Expand Down