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

Generate documentation content based on the symbol graph module names #52

Merged
merged 2 commits into from
Dec 9, 2021

Conversation

d-ronnqvist
Copy link
Contributor

Bug/issue #, if applicable: SR-15408 rdar://84810134

Summary

This updates the GeneratedDataProvider to generate top level documentation extension files based on the module names from the available symbol graph files instead of the fallback display name. This means that:

  • the documentation extension file will match one of the module symbols
  • if there are multiple modules there will be multiple generated documentation extension files

This PR also removes a small amount of ambiguity regarding the provider's use of BundleDiscoveryOptions by removing the options argument to the initializer. Previously, this value wasn't used when providing the bundles but it was used when providing their raw content.

Dependencies

n/a

Testing

Call docc convert or docc preview with a fallback display name that doesn't match the name of any module in any symbol graph file.

For example, with the example symbol graph file from SR-15408:

docc preview \
 --fallback-display-name "Tiny Framework"\
 --fallback-bundle-identifier "test.tiny-framework" \
 --additional-symbol-graph-files /path/to/Tiny_Framework.symbols.json

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

We don't currently have a way to customize how the title of a module is displayed. I could add a new directive within the @Metadata directive that allows the developer to customize the display name if you prefer to make both changes at the same time @ethan-kusters

For example:

# ``Tiny_Framework``

@Metadata {
  @DisplayName("Tiny Framework")
}

@ethan-kusters
Copy link
Contributor

We don't currently have a way to customize how the title of a module is displayed. I could add a new directive within the @Metadata directive that allows the developer to customize the display name if you prefer to make both changes at the same time @ethan-kusters

For example:

# ``Tiny_Framework``

@Metadata {
  @DisplayName("Tiny Framework")
}

@d-ronnqvist I really like that as an enhancement but I think we should probably split that out into a separate bug/PR since I would expect to have a forums discussion around the naming of the directive. Honestly, I'd be interested in creating metadata directives for some of our other Info.plist keys as well since I find it a more friendly format to work with.

We still support the display name flag though, correct?

So in the given example:

docc preview \
 --fallback-display-name "Tiny Framework"\
 --fallback-bundle-identifier "test.tiny-framework" \
 --additional-symbol-graph-files /path/to/Tiny_Framework.symbols.json

Does this render the framework title as "Tiny Framework" or "Tiny_Framework"?

@d-ronnqvist
Copy link
Contributor Author

Does this render the framework title as "Tiny Framework" or "Tiny_Framework"?

It renders as Tiny_Framework since that's the name of the symbol that the page represents. That's why I suggested adding a directive so that the generated content (and developers) can customize the display name of a symbol's page.

@ethan-kusters
Copy link
Contributor

Does this render the framework title as "Tiny Framework" or "Tiny_Framework"?

It renders as Tiny_Framework since that's the name of the symbol that the page represents. That's why I suggested adding a directive so that the generated content (and developers) can customize the display name of a symbol's page.

Ah got it. I think it will be somewhat unexpected for users that DocC doesn't actually respect the provided display name when building documentation for a given symbol graph. But since this is the existing behavior, I think we can address that in a separate PR.

Thank you for clarifying!

Copy link
Contributor

@ethan-kusters ethan-kusters left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks David.

I've logged https://bugs.swift.org/browse/SR-15572 to track adding support for customizing a module's display name in the future.

@d-ronnqvist
Copy link
Contributor Author

@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.

None yet

2 participants