Skip to content
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.

Sign up for GitHub

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

Make NavigatorIndex.Builder ignore language variants when requested #1077

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Make NavigatorIndex.Builder ignore language variants when requested #1077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,8 @@ extension NavigatorIndex {
A `Builder` is a utility class to build a navigator index.

The builder generates an index for content navigation, but also maps important information to filter content based on availability, symbol type, platform and some others.
- Note: The builder is not thread safe and therefore, calling `index(renderNode:)` requires external synchronization in case the process is performed on different threads.

- Note: The builder is not thread safe and therefore, calling `index(renderNode:)` requires external synchronization in case the process is performed on different threads.
*/
open class Builder {

Expand Down Expand Up @@ -613,12 +613,13 @@ extension NavigatorIndex {

/// Index a single render `RenderNode`.
/// - Parameter renderNode: The render node to be indexed.
public func index(renderNode: RenderNode) throws {
/// - Parameter ignoringLanguage: Whether language variants should be ignored when indexing this render node.
public func index(renderNode: RenderNode, ignoringLanguage: Bool = false) throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Introducing this parameter remains source stable because it has a default value, so exiting calls to .index(renderNode: renderNode) .

However, if we later wanted to change the name of this parameter, that would be source breaking.

Considering that, are we happy with ignoringLanguage or do we want to include the word "variants" somewhere in this parameter name?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since both the parameter documentation and the code comment above where this value is checked mentions variants, I'm pretty sure that future us will understand what it's for regardless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think? I don't have a strong opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also fine either way.

// Always index the main render node representation
let language = try index(renderNode, traits: nil)

// Additionally, for Swift want to also index the Objective-C variant, if there is any.
guard language == .swift else {
guard !ignoringLanguage && language == .swift else {
return
}

Expand Down
146 changes: 89 additions & 57 deletions Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,14 @@ Root
}

func testNavigatorIndexGenerationVariantsPayload() throws {
try testNavigatorIndexGenerationVariantsPayload(ignoringLanguage: false)
}

func testNavigatorIndexGenerationVariantsPayloadIgnoringLanguage() throws {
try testNavigatorIndexGenerationVariantsPayload(ignoringLanguage: true)
}

private func testNavigatorIndexGenerationVariantsPayload(ignoringLanguage: Bool) throws {
let jsonFile = Bundle.module.url(forResource: "Variant-render-node", withExtension: "json", subdirectory: "Test Resources")!
let jsonData = try Data(contentsOf: jsonFile)

Expand All @@ -711,88 +719,112 @@ Root
builder.setup()

let renderNode = try XCTUnwrap(RenderJSONDecoder.makeDecoder().decode(RenderNode.self, from: jsonData))
try builder.index(renderNode: renderNode)
try builder.index(renderNode: renderNode, ignoringLanguage: ignoringLanguage)

builder.finalize()

let navigatorIndex = builder.navigatorIndex!

assertUniqueIDs(node: navigatorIndex.navigatorTree.root)
assertEqualDumps(navigatorIndex.navigatorTree.root.dumpTree(), """
var expectedDump = """
[Root]

"""

if !ignoringLanguage {
expectedDump += """
┣╸Objective-C
┃ ┗╸My Article in Objective-C
┃ ┣╸Task Group 1
┃ ┣╸Task Group 2
┃ ┗╸Task Group 3

"""
}

expectedDump += """
┗╸Swift
┗╸My Article
┣╸Task Group 1
┣╸Task Group 2
┗╸Task Group 3
""")

try XCTAssertEqual(
RenderIndex.fromURL(targetURL.appendingPathComponent("index.json")),
RenderIndex.fromString(#"""
{
"interfaceLanguages": {
"occ": [
"""

assertEqualDumps(navigatorIndex.navigatorTree.root.dumpTree(), expectedDump)

var expectedRenderIndexString = """
{
"interfaceLanguages": {
"""

if !ignoringLanguage {
expectedRenderIndexString += #"""
"occ": [
{
"children": [
{
"title": "Task Group 1",
"type": "groupMarker"
},
{
"title": "Task Group 2",
"type": "groupMarker"
},
{
"children": [
{
"title": "Task Group 1",
"type": "groupMarker"
},
{
"title": "Task Group 2",
"type": "groupMarker"
},
{
"title": "Task Group 3",
"type": "groupMarker"
}
],
"path": "\/documentation\/mykit\/my-article",
"title": "My Article in Objective-C",
"type": "article"
"title": "Task Group 3",
"type": "groupMarker"
}
],
"swift": [
"path": "\/documentation\/mykit\/my-article",
"title": "My Article in Objective-C",
"type": "article"
}
],
"""#
}

expectedRenderIndexString += #"""
"swift": [
{
"children": [
{
"children": [
{
"title": "Task Group 1",
"type": "groupMarker"
},
{
"title": "Task Group 2",
"type": "groupMarker"
},
{
"title": "Task Group 3",
"type": "groupMarker"
}
],
"path": "\/documentation\/mykit\/my-article",
"title": "My Article",
"type": "article"
"title": "Task Group 1",
"type": "groupMarker"
},
{
"title": "Task Group 2",
"type": "groupMarker"
},
{
"title": "Task Group 3",
"type": "groupMarker"
}
]
},
"includedArchiveIdentifiers": [
"org.swift.docc.example"
],
"schemaVersion": {
"major": 0,
"minor": 1,
"patch": 2
],
"path": "\/documentation\/mykit\/my-article",
"title": "My Article",
"type": "article"
}
}
]
"""#
)

expectedRenderIndexString += #"""
},
"includedArchiveIdentifiers": [
"org.swift.docc.example"
],
"schemaVersion": {
"major": 0,
"minor": 1,
"patch": 2
}
}
"""#

try XCTAssertEqual(
RenderIndex.fromURL(targetURL.appendingPathComponent("index.json")),
RenderIndex.fromString(expectedRenderIndexString)
)

try FileManager.default.removeItem(at: targetURL)
}

Expand Down