Skip to content

Update contributor documentation to reflect changes in input discovery #1060

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

Merged

Conversation

d-ronnqvist
Copy link
Contributor

@d-ronnqvist d-ronnqvist commented Oct 15, 2024

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

Summary

This updates the contributor documentation to reflect the changes in documentation input discovery and context creation from #1059, #1057, #1049, #1038, and #1031

It also makes some clarifications about what the documentation inputs are and how the user can provide them, the role of a documentation context, and the files that a documentation catalog can contain.

Dependencies

This builds on the changes from #1059 (Deprecate two-step documentation context creation) which build on the changes from #1057 (Pass documentation context its only bundle during initialization)

This diff shows the changes that are specific to this PR.

Testing

Preview the contributor documentation in SwiftDocC

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 d-ronnqvist added the documentation Improvements or additions to documentation label Oct 15, 2024
@d-ronnqvist d-ronnqvist force-pushed the input-discovery-contributor-docs branch 3 times, most recently from 7aea786 to 22962bf Compare October 25, 2024 08:44
Copy link
Contributor

@mayaepps mayaepps left a comment

Choose a reason for hiding this comment

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

Thanks for updating this!


When a documentation bundle is found in the workspace by a ``DocumentationWorkspaceDataProvider``, the following files are recognized and processed (others are ignored):
- Markup files, tutorial files, and assets (for example images)
- Symbol graph files, describing the symbols in a given module (types, functions, variables, etc.) and their relationships (inheritance, conformance, etc.)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think describing the extensions for each of these file types is useful. Can we add that back here?

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 didn't feel like the top-level page about the compiler pipeline was the right place to enumerate all the support file extensions in a catalog.

I'll see if I can find a better place for this information.

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 noticed that the top-level page also listed the possible content of a catalog so I updated that to mention the file extensions .

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

And finally to print all known paths in the context:

```swift
context.knownIdentifiers.forEach({ print($0) })
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this no longer true/possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still both true and possible. I just felt that it wasn't relevant.

guard let catalogURL = try inputProvider.findCatalog(startingPoint: startingPoint) else {
return
}
let bundle = try inputProvider.makeInputs(contentOf: catalogURL, options: BundleDiscoveryOptions())
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about briefly explaining BundleDiscoveryOptions? For example, that BundleDiscoveryOptions can also include Info.plist values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm... I think for the purposes of this article it's sufficient to see BundleDiscoveryOptions being used and showing that additional symbol graph locations are passed in the options.

If the developer goes to try and discover inputs themselves they would either try to create a new value and discover the first argument for both initializers is some form of Info.plist values, or they would look for an options value that's already passed to their scope and use that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhat related, with the new input discovery structure I find it a bit weird that the catalog is separate from the options. If BundleDiscoveryOptions didn't already exist I don't know if I would group the Info.plist fallbacks and the symbol graph URLs without also grouping the catalog URL.

It's also weird that it's named "options" but the information it holds is more user-provided inputs. allowArbitraryCatalogDirectories would make sense as an option but that is its own parameter.

At least now there's only really two places that use it (DocumentationContext/InputsProvider and DocumentationBundle/Info/init(from:bundleDiscoveryOptions:derivedDisplayName:) neither of which are public) so it would be easy to change in the future if we have new ideas about how to group this information.

Copy link
Contributor

@mayaepps mayaepps Oct 28, 2024

Choose a reason for hiding this comment

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

Makes sense. I agree -- from the name, it wasn't immediately clear (to me) what BundleDiscoveryOptions is. But you're right that it would hopefully become clearer when the developer goes to discover inputs.

@d-ronnqvist d-ronnqvist force-pushed the input-discovery-contributor-docs branch from b22d790 to b6a2079 Compare October 28, 2024 12:54
@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 5a690ce into swiftlang:main Nov 8, 2024
2 checks passed
@d-ronnqvist d-ronnqvist deleted the input-discovery-contributor-docs branch November 8, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants