-
Notifications
You must be signed in to change notification settings - Fork 120
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
Update LinkDestinationSummary to support multi-language content #54
Update LinkDestinationSummary to support multi-language content #54
Conversation
rdar://83716043
rdar://83716043
Most of this is ready to review but the |
I opened https://bugs.swift.org/browse/SR-15574 as a follow-up change to update the implementation to write content variants for all source languages. |
@swift-ci please test |
throw DecodingError.dataCorruptedError(forKey: .contentVariants, in: container, debugDescription: "Missing required content. ContentVariations is empty.") | ||
} | ||
self.contentVariants = contentVariants | ||
} catch DecodingError.keyNotFound(_, let originalErrorContext) { |
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.
Should we do a version check instead (== "0.1.0"
) for the legacy decoding rather than falling back to it if the variants key is missing, or do you think it's not worth it?
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.
That would require that we encode the version information in the JSON, which we would probably do once for the entire file instead of once for per element.
@@ -271,6 +302,71 @@ class ExternalLinkableTests: XCTestCase { | |||
} | |||
} | |||
|
|||
func testDecodingLegacyData() throws { |
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.
Nice!
Now, the data continues to be encoded directly in the summary and each variant only sets the information that's different in that source language.
…onnqvist/swift-docc into multi-languge-linkable-entities-spec
I made some changes to the spec and the implementation to reduce the amount of duplicate encoded data when some content is the same in multiple languages. This also makes the single source language content almost the same as previously (the only difference is that Since the changes are so small we could decide to redundantly encode the |
@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.
This looks great to me! I left a few documentation nits but happy to approve now.
/// The traits of the variant. | ||
public let traits: [RenderNode.Variant.Trait] | ||
|
||
/// A wrapper for variant values that are either set (the variant has a custom value) not set (the variant have the same value as the summarized element) |
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.
/// A wrapper for variant values that are either set (the variant has a custom value) not set (the variant have the same value as the summarized element) | |
/// A wrapper for variant values that can either be specified, meaning the variant has a custom value, or not, meaning the variant has the same value as the summarized element. |
/// This alias is used to make the property declarations more explicit while at the same time offering the convenient syntax of optionals. | ||
public typealias VariantValue = Optional | ||
|
||
/// The kind of the variant or `nil` if the kind is the same as the summarized element. |
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.
Should we use .none here maybe? To differentiate between the wrapper and the inner value a little more.
/// The kind of the variant or `nil` if the kind is the same as the summarized element. | |
/// The kind of the variant or `.none` if the kind is the same as the summarized element. |
|
||
/// The abstract of the variant or `nil` if the abstract is the same as the summarized element. | ||
/// | ||
/// - Note: If the summarized element has an abstract but the variant doesn't, this property will be `Optional.some(nil)`. |
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.
Nit: Note's render kind of strangely if they're not embedded in other content. It might be better to just make this a regular discussion.
/// - Note: If the summarized element has an abstract but the variant doesn't, this property will be `Optional.some(nil)`. | |
/// If the summarized element has an abstract but the variant doesn't, this property will be `Optional.some(nil)`. |
/// The kind of the variant or `nil` if the kind is the same as the summarized element. | ||
public let kind: VariantValue<DocumentationNode.Kind> | ||
|
||
/// The language of the variant or `nil` if the kind is the same as the summarized element. |
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.
/// The language of the variant or `nil` if the kind is the same as the summarized element. | |
/// The source language of the variant or `nil` if the kind is the same as the summarized element. |
@swift-ci please test |
…tlang#54) * Move content into a source language variant structure rdar://83716043 * Update LinkDestinationSummary to match spec changes rdar://83716043 * Move `usr` out of variant substructure since it doesn't vary * Update the spec to reduce the amount of duplicate data. Now, the data continues to be encoded directly in the summary and each variant only sets the information that's different in that source language. * Remove extra closing back-tick in documentation comments * Use an alias to make optionality of variant properties more explicit * Remove trait from main summary element to avoid using it as a language replacement * Small documentation comment refinements
Bug/issue #, if applicable: rdar://83716043
Summary
This updates
LinkDestinationSummary
and the LinkableEntities.json spec to support multi-language content by moving the properties that can vary into aContentVariant
substructure.The implementation supports decoding data in the "0.1.0" format but the encoded data will always be in the "0.2.0" format.
Dependencies
As a code change this doesn't depend on #47 but the changes in this PR are not useful without the base support for multi-language documentation catalogs.
Testing
Convert a multi-language documentation catalog and include the
--emit-digest
flag so that Swift-DocC will create the linkable-entities.json file.(For testing purposes it may be convenient to also set the
DOCC_JSON_PRETTYPRINT
environment toYES
to get output that's more easily inspectable.)The content of the linkable-entities.json file should include the variant content of each interface language for each "entity".
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded