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

Support snippet symbolgraph #95

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

heckj
Copy link
Member

@heckj heckj commented Feb 16, 2025

Summary

Snippet extension graphs being considered primary is the source of non-deterministic results in previewing documentation that include snippets (swiftlang/swift-docc#1084). For the logic within swift-dock's SymbolGraphLoader, they should be considered extension graphs. Since the snippet symbol graph extractor doesn't generate an @ in the file name of the graph, the logic is amended to look at the isVirtual metadata for the module, which is set to true for snippets.

I considered using the name generated by the snippet extractor, or doing some heuristic on the name the generator metadata, but after staring at the data for a while, the SymbolGraph.module.isVirtual seemed a more exact and lasting means of determining if a given symbol graph is "the main one" or not.

(reset of #89, previously reverted, when smoke tests didn't pass)

Dependencies

Depends on swiftlang/swift-docc#1160 for the CI/smoke tests to pass (in particular, this PR was merged earlier, and reverted, due to tests in swift-docc repeatedly failing with these updates. The PR in swift-docc adds structure to a test fixture so that better represents snippets as extending another symbolgraph, which resolves these test issues.

Testing

Added unit tests to verify loading functionality

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

heckj and others added 4 commits February 4, 2025 10:52
- expanded documentatin on mergeSymbolGraph to describe the forceLoading
  parameter.
- updated the logic to use the lack of `@` and the `isVirtual` metadata
  to determine if a symbolgraph is the primary graph for that module.
@heckj
Copy link
Member Author

heckj commented Feb 16, 2025

Prior (identical) PR was merged in #89 and reverted in #94 when the swift-docc tests failed. The PR swiftlang/swift-docc#1160 resolves those failing tests.

I've verified (manually) this PR works as intended when the upstream PR in place.

@QuietMisdreavus
Copy link
Contributor

@swift-ci Please test

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.

GraphCollector has limited logic to determine a "main" symbol graph
2 participants