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.

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

Add successfully resolved external references to reference index #582

Merged

Conversation

d-ronnqvist
Copy link
Contributor

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

Summary

This change tracks successfully resolved external references the same as successfully resolved local references, enabling them to be looked up in the reference index without going through the link resolution code path again.

Dependencies

None.

Testing

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

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

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@@ -206,11 +206,17 @@ struct RenderContentCompiler: MarkupVisitor {
}

mutating func resolveTopicReference(_ destination: String) -> ResolvedTopicReference? {
if let cached = context.referenceIndex[destination] {
if let cached = context.referenceIndex[destination] {
if let node = context.topicGraph.nodeWithReference(cached), !context.topicGraph.isLinkable(node.reference) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the change here needed? Was it just missing before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that it was needed because the code below does it but now that I think about it I also think that the link resolution would have already done this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, only MarkupReferenceResolver filters references that are linkable so I guess it's possible for the context to have resolved and saved a reference that isn't linkable.

So in order for the cached and non-cached code paths to behave the same, the cached code path should also check that the reference is linkable.

Co-authored-by: Ethan Kusters <ekusters@apple.com>
@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit d9ec4ae into swiftlang:main May 9, 2023
@d-ronnqvist d-ronnqvist deleted the external-references-in-index branch May 9, 2023 22:55
d-ronnqvist added a commit to d-ronnqvist/swift-docc that referenced this pull request May 9, 2023
…ftlang#582)

* Add successfully resolved external references to reference index

rdar://108974747

* Undo accidental whitespace change

Co-authored-by: Ethan Kusters <ekusters@apple.com>
d-ronnqvist added a commit that referenced this pull request May 10, 2023
… (#584)

* Add successfully resolved external references to reference index

rdar://108974747

* Undo accidental whitespace change

Co-authored-by: Ethan Kusters <ekusters@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants