Filter topics languages for automatically-curated See Alsos #104
Conversation
franklinsch
requested review from
QuietMisdreavus,
d-ronnqvist,
daniel-grumberg and
ethan-kusters
and removed request for
d-ronnqvist
|
@swift-ci please test |
franklinsch
force-pushed
the
filter-topics-languages-see-alsos
branch
from
9410ef5 to
8855506
Compare
|
@swift-ci please test |
|
|
||
| let parentReference = canonicalPath.last! | ||
|
|
||
| func filterReferences(_ references: [ResolvedTopicReference]) throws -> [ResolvedTopicReference] { |
There was a problem hiding this comment.
Nit: Is there a more specific name we can use to make this more readable at the call-site?
| func filterReferences(_ references: [ResolvedTopicReference]) throws -> [ResolvedTopicReference] { | |
| func referencesAvailableInCurrentTrait(in references: [ResolvedTopicReference]) throws -> [ResolvedTopicReference] { |
There was a problem hiding this comment.
It also filters out the current reference from the task group, though. We could also consider making this function only handle the trait-based filtering, but the logic to filter out the current reference would then be duplicated.
| }) | ||
| { | ||
| // Group match in render context, verify if there are any other references besides the current one. | ||
| guard linkingGroup.references.count > 1 else { return nil } |
There was a problem hiding this comment.
Nit since this is a little out-of-scope, but the existing logic seems a little hacky to me and would break if we ever stop including the current reference in the linking group.
| guard linkingGroup.references.count > 1 else { return nil } | |
| let references = try filterReferences(linkingGroup.references) | |
| guard !references.isEmpty else { return nil } |
There was a problem hiding this comment.
The automatic curation logic here finds a task group that contains this reference, removing this reference from the task group. The count > 1 is essentially to not emit the task group at all if the only reference it contains is this reference.
There was a problem hiding this comment.
Right I see that- I'm just suggesting we do the filtering ahead of time so we can gate on the already filtered group being empty or not.
let references = try filterReferences(linkingGroup.references)
guard !references.isEmpty else { return nil }
return (title: linkingGroup.title, references: references)|
|
||
| // Filter out nodes that aren't available in the given trait. | ||
| .filter { reference in | ||
| try context.entity(with: reference) |
There was a problem hiding this comment.
Should we be adopting this logic in the topics(for:withTrait:context:) function above? It seems cleaner than only relying on the kind being available for a given trait but maybe I'm missing something. (I'm not entirely sure what availableVariantTraits is pulled from.)
There was a problem hiding this comment.
We could, but we need to get the appropriate language-specific kind of the symbol anyway in that case. If we ported this logic to topics(for:withTrait:context:), we'd check the languages in DocumentationNode. availableVariantTraits but also need to verify that the symbol has kindVariants for each of these languages which seems redundant. Compilers are expected to produce the kind field for their symbols, so DocC shouldn't be in a case where a symbol is available in multiple languages, but the symbol only has 1 kind value.
DocumentationNode.availableVariantTraits is populated from this logic: https://github.com/apple/swift-docc/blob/main/Sources/SwiftDocC/Infrastructure/Symbol%20Graph/SymbolReference.swift#L174
There was a problem hiding this comment.
Compilers are expected to produce the kind field for their symbols, so DocC shouldn't be in a case where a symbol is available in multiple languages, but the symbol only has 1 kind value.
What if we have the same kind in multiple languages?
There was a problem hiding this comment.
The symbol would still have multiple kind entries, with the same value.
franklinsch
force-pushed
the
filter-topics-languages-see-alsos
branch
from
8855506 to
ffa93ca
Compare
|
@swift-ci please test |
For automatically-generated See Also sections, filter out topics that are not available in the parent's current language. rdar://90168171
franklinsch
force-pushed
the
filter-topics-languages-see-alsos
branch
from
ffa93ca to
f26b7ab
Compare
|
@swift-ci please test |
2 participants
Bug/issue #, if applicable: rdar://90168171
Summary
As a follow-up to #103, filters out topics in automatically-generated See Also sections that are not available in the parent's current language.
Dependencies
None.
Testing
Build documentation for a mixed-language framework and verify that automatically-generated See Also sections only contain topics that are available in the current language of the parent page.
Checklist
./bin/testscript and it succeeded