Skip to content

Avoid full URL parsing to trim a known prefix from a reference string #1323

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

Merged
merged 1 commit into from
Oct 21, 2025
Merged

Avoid full URL parsing to trim a known prefix from a reference string #1323

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
5 changes: 4 additions & 1 deletion Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,10 @@ extension OutOfProcessReferenceResolver {
return .success( makeReference(for: cachedSummary) )
}

let linkString = unresolvedReference.topicURL.url.withoutHostAndPortAndScheme().standardized.absoluteString
let linkString = String(
unresolvedReferenceString.dropFirst(6) // "doc://"
.drop(while: { $0 != "/" }) // the known identifier (host component)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we know the bundle ID, should we drop that instead of dropping until the first /?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either way works. I don't have a strong opinion on how to implement. IMO the current implementation is fine, isn't externally visible, and is covered by tests so we can always change it in the future if we feel a need.

)
let response: ResponseV2 = try longRunningProcess.sendAndWait(request: RequestV2.link(linkString))

switch response {
Expand Down
7 changes: 6 additions & 1 deletion Tests/SwiftDocCTests/OutOfProcessReferenceResolverV2Tests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,9 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
read # Wait for 3rd request
echo $REPLY >> \(savedRequestsFile.path) # Save the raw request
echo '{"failure":"ignored error message"}' # Respond
read # Wait for 4th request
echo $REPLY >> \(savedRequestsFile.path) # Save the raw request
echo '{"failure":"ignored error message"}' # Respond
""".write(to: executableLocation, atomically: true, encoding: .utf8)

// `0o0700` is `-rwx------` (read, write, & execute only for owner)
Expand All @@ -544,10 +547,11 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
TextFile(name: "Something.md", utf8Content: """
# My root page

This page contains an 3 external links hat will fail to resolve:
This page contains an 4 external links that will fail to resolve:
- <doc://\(externalBundleID.rawValue)/some-link>
- <doc://\(externalBundleID.rawValue)/path/to/some-link>
- <doc://\(externalBundleID.rawValue)/path/to/some-link#some-fragment>
- <doc://\(externalBundleID.rawValue)//not-parsable-as-url>
""")
])
let inputDirectory = Folder(name: "path", content: [Folder(name: "to", content: [catalog])])
Expand All @@ -568,6 +572,7 @@ class OutOfProcessReferenceResolverV2Tests: XCTestCase {
XCTAssertEqual(readRequests, """
{"link":"/some-link"}
{"link":"/path/to/some-link"}
{"link":"//not-parsable-as-url"}
{"link":"/path/to/some-link#some-fragment"}
""")
}
Expand Down