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

Fix rare crash when tutorial file has same as documentation bundle identifier #517

Merged
merged 4 commits into from
Mar 29, 2023

Conversation

d-ronnqvist
Copy link
Contributor

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

Summary

This fixes a rare crash when a tutorial file name is the same as the documentation bundle identifier and that tutorial is curated together with a symbol from the module with the same name.

In other words; in the documentation for the "Something" module, where no bundle identifier is specified, the bundle identifier is inferred from the bundle name which is inferred from the module name. Here, the module name, bundle name, and bundle identifier are all "Something". If this documentation also contains a tutorial overview in a file named "Something.tutorial", curating this tutorial in the same topic section as a symbol from the "Something" module would crash.

Dependencies

None.

Testing

Build the documentation for SlothCreator. The page for the Sloth symbol should have a tutorial in its generated See Also section.

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

@@ -67,9 +67,9 @@ final class PathHierarchyBasedLinkResolver {

/// Map the resolved identifiers to resolved topic references for a given bundle's article, tutorial, and technology root pages.
func addMappingForRoots(bundle: DocumentationBundle) {
resolvedReferenceMap[pathHierarchy.tutorialContainer.identifier] = bundle.tutorialsRootReference
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The root cause for this crash is that I had forgotten which is the "tutorial" page and which is the "technology" page so the tutorial link resolved the reference for the technology page and vice versa resulting in an assertion during rendering of the See Also section.

Copy link
Contributor

Choose a reason for hiding this comment

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

should we rename these so that they are easier to distinguish?

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 think that's a good idea. There are still many places that use the "technology" terminology in the code and I think we should update them all to match the user-facing terminology.

I opened this issue about updating the general "technology" terminology in the code.

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 3b2524b into swiftlang:main Mar 29, 2023
d-ronnqvist added a commit to d-ronnqvist/swift-docc that referenced this pull request Mar 29, 2023
…entifier (swiftlang#517)

* Fix crash when tutorial named like module/bundle is curated w/ symbol

rdar://106958656

* Fix test that was always passing and update misleading comments in test
d-ronnqvist added a commit that referenced this pull request Mar 29, 2023
…entifier (#517) (#524)

* Fix crash when tutorial named like module/bundle is curated w/ symbol

rdar://106958656

* Fix test that was always passing and update misleading comments in test
@d-ronnqvist d-ronnqvist deleted the tutorial-see-also-crash branch February 19, 2024 08:20
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