-
Notifications
You must be signed in to change notification settings - Fork 163
Update LinkDestinationSummary to support multi-language content #54
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
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.
| } | ||
| } | ||
|
|
||
| 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 |
ethan-kusters
left a comment
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
LinkDestinationSummaryand the LinkableEntities.json spec to support multi-language content by moving the properties that can vary into aContentVariantsubstructure.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-digestflag so that Swift-DocC will create the linkable-entities.json file.(For testing purposes it may be convenient to also set the
DOCC_JSON_PRETTYPRINTenvironment toYESto 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/testscript and it succeeded