Filter out topics that aren't available in the page's language #96

franklinsch · 2022-03-04T17:14:30Z

Bug/issue #, if applicable: rdar://89124291

Summary

Filters out topics that aren't available in the language of the symbol that curates them. Note that this was already working for automatically curated topics, but not manually curated topics.

This PR also includes making tutorial content available on all the languages in which the framework is available (last commit),

Also, since the original fix of this PR filters out topics that aren't available in the parent page's language, it was also causing DocC to filter out tutorial content from Objective-C pages. As such, the last commit of this PR resolves this issue by making tutorials multi-language as well, if the framework it's documenting is multi-language.

Dependencies

None.

Testing

Create symbol graph files with symbols that are only available in Swift and symbols only available in Objective-C, and curate them manually in a documentation extension file or in documentation comments.
Verify that the rendered documentation doesn't show the Objective-C–only symbols when browsing the Swift documentation, and vice-versa.

Checklist

Added tests
Ran the ./bin/test script and it succeeded
Updated documentation if necessary

TODO

Make Tutorials multi-language so that they can be curated in Objective-C pages. They're currently hardcoded to be Swift-only

franklinsch · 2022-03-07T12:48:41Z

@swift-ci please test

franklinsch · 2022-03-07T13:40:12Z

@swift-ci please test

franklinsch · 2022-03-08T17:49:37Z

@swift-ci please test

daniel-grumberg · 2022-03-08T21:26:37Z

Sources/SwiftDocC/Utility/DataStructures/GroupedSequence.swift

+    }
+
+    /// Creates an empty grouped sequence.
+    init(deriveKey: @escaping (Element) -> Key) {


Nit: typo derivedKey

I think deriveKey as it is currently works better because its type is a function, so it should be named like an action.

daniel-grumberg · 2022-03-08T21:42:27Z

Sources/SwiftDocC/Utility/DataStructures/GroupedSequence.swift

+/// // Prints "a"
+/// // Prints "aaa"
+/// ```
+struct GroupedSequence<Key: Hashable, Element>: Sequence, CustomStringConvertible {


Not needed for this PR but this already fulfills the requirements for Collection might be nice since we are making this a standalone data structure.

hm, yes, looks like we're close! But we need to implement a few more requirements, which I think we should implement when we get a use for them. This isn't intended to be a general-purpose API—it's just for DocC's purposes.

daniel-grumberg · 2022-03-08T22:01:00Z

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift


+            /// Returns whether the topic with the given identifier is available in one of the traits in `allowedTraits`.
+            func isAvailableInAllowedTrait(identifier: String) -> Bool {
+                guard let reference = contentCompiler.collectedTopicReferences[identifier] else {


Not sure I understand this logic. Could you explain why you return true if you couldn't find a topic reference for this identifier?

Is this because you are conservatively not filtering out content you don't have a definition for in this framework?

Ah yes, I should clarify that in a comment, thanks for pointing that out. It's expected for reference to never be nil as it is a precondition for contentCompiler to have populated collectionTopicReference with the reference at this point. Otherwise, it is a bug in contentCompiler.

In fact, we can actually add an assertionFailure so that we get a crash in debug builds, but not release.

Actually, after taking a closer look, I'm wrong on this. It's possible for the content compiler to generate a reference identifier without registering it in collectedTopicReferences, for example when the reference is a non-doc URL, e.g., https:// in which case it gets registered in linkReferences instead. I'll add a comment to clarify that.

daniel-grumberg · 2022-03-08T22:16:40Z

Tests/SwiftDocCTests/Utility/DataStructures/GroupedSequenceTests.swift

+        groupedSequence.append("aa")
+
+        XCTAssertEqual(groupedSequence[1], "a")
+        XCTAssertEqual(groupedSequence[2], "aa")


you probably want to add the following to this:

groupedSequence.append("b") XCTAssertEqual(groupedSequence[1], "b")

Yes, that's a great idea!

franklinsch · 2022-03-09T13:36:48Z

@swift-ci please test

ethan-kusters

This looks good to me! Just a few small things for your consideration.

Was tutorials the last thing we needed to support here or do we still need support for articles?

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

ethan-kusters · 2022-03-10T01:27:15Z

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

+                    semantic: technology
+                )
+                documentationCache[reference] = node
+                topicGraph.nodes[reference]?.reference = reference


Is this a change from before? We weren't including technology references in the topic graph?

We are at an earlier stage, in registerDocuments. What we're doing here is updating the reference now that we have enough information to know in which languages the page is available in. I'll add a comment to clarify that.

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

ethan-kusters · 2022-03-10T01:38:42Z

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

            }

+            /// Returns whether the topic with the given identifier is available in one of the traits in `allowedTraits`.
+            func isAvailableInAllowedTrait(identifier: String) -> Bool {


Suggested change

func isAvailableInAllowedTrait(identifier: String) -> Bool {

func topicIsAvailableInAllowedTraits(identifier topicIdentifier: String) -> Bool {

Fair. I can meet in the middle with isTopicAvailableInAllowedTraits. 🤝?

Deal. 🤝😃

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

franklinsch · 2022-03-10T15:12:05Z

Was tutorials the last thing we needed to support here or do we still need support for articles?

I believe tutorials was the last thing we needed to make multi-language. Article support was added by #81.

Filter out topics that are not available in the language of the symbol that curates them. rdar://89124291

In the same way articles do, infer the available languages of tutorials from the module's available language.

franklinsch · 2022-03-10T15:12:34Z

@swift-ci please test

franklinsch · 2022-03-10T15:14:37Z

@swift-ci please test macOS platform

franklinsch · 2022-03-10T15:15:22Z

@swift-ci please test Linux platform

ethan-kusters · 2022-03-10T17:19:34Z

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

+                documentationCache[technologyReference] = technologyNode
+
+                // Update the reference in the topic graph with the technology's available languages.
+                topicGraph.updateReference(


Nice! This looks great- much easier for me to follow.

franklinsch force-pushed the filter-topics-languages branch from 663153b to f9b5518 Compare March 7, 2022 12:43

franklinsch changed the title ~~[WIP] Filter out topics that aren't available in the page's language~~ Filter out topics that aren't available in the page's language Mar 7, 2022

franklinsch marked this pull request as ready for review March 7, 2022 12:43

franklinsch force-pushed the filter-topics-languages branch from f9b5518 to 4cd124c Compare March 7, 2022 13:39

franklinsch requested review from d-ronnqvist and ethan-kusters and removed request for ethan-kusters March 7, 2022 13:39

franklinsch requested review from QuietMisdreavus and daniel-grumberg March 7, 2022 13:40

franklinsch force-pushed the filter-topics-languages branch from 4cd124c to a2eb297 Compare March 8, 2022 17:49

daniel-grumberg reviewed Mar 8, 2022

View reviewed changes

franklinsch force-pushed the filter-topics-languages branch from a2eb297 to 906ac34 Compare March 9, 2022 13:19

franklinsch requested a review from daniel-grumberg March 9, 2022 13:19

ethan-kusters reviewed Mar 10, 2022

View reviewed changes

franklinsch force-pushed the filter-topics-languages branch from 906ac34 to 64e6daf Compare March 10, 2022 15:08

franklinsch requested a review from ethan-kusters March 10, 2022 15:12

franklinsch added 3 commits March 10, 2022 15:12

Move utility data structures tests

1805de5

Filter out topics in other languages

d694243

Filter out topics that are not available in the language of the symbol that curates them. rdar://89124291

Make tutorials language infer from module

a7857b3

In the same way articles do, infer the available languages of tutorials from the module's available language.

franklinsch force-pushed the filter-topics-languages branch from 64e6daf to a7857b3 Compare March 10, 2022 15:12

ethan-kusters approved these changes Mar 10, 2022

View reviewed changes

franklinsch merged commit 783cc96 into swiftlang:main Mar 11, 2022

franklinsch deleted the filter-topics-languages branch March 11, 2022 17:40

franklinsch mentioned this pull request Mar 11, 2022

Filter topics languages in articles #103

Merged

3 tasks

	func isAvailableInAllowedTrait(identifier: String) -> Bool {
	func topicIsAvailableInAllowedTraits(identifier topicIdentifier: String) -> Bool {

Filter out topics that aren't available in the page's language #96

Filter out topics that aren't available in the page's language #96

Uh oh!

Conversation

franklinsch commented Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependencies

Testing

Checklist

TODO

Uh oh!

franklinsch commented Mar 7, 2022

Uh oh!

franklinsch commented Mar 7, 2022

Uh oh!

franklinsch commented Mar 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franklinsch Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franklinsch Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-grumberg Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franklinsch commented Mar 9, 2022

Uh oh!

ethan-kusters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

franklinsch commented Mar 10, 2022

Uh oh!

franklinsch commented Mar 10, 2022

Uh oh!

franklinsch commented Mar 10, 2022

Uh oh!

franklinsch commented Mar 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

franklinsch commented Mar 4, 2022 •

edited

Loading

franklinsch Mar 9, 2022 •

edited

Loading

franklinsch Mar 9, 2022 •

edited

Loading

daniel-grumberg Mar 8, 2022 •

edited

Loading