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

No pages warning #664

Merged
merged 12 commits into from
Aug 7, 2023

Conversation

sofiaromorales
Copy link
Contributor

Bug/issue #, if applicable: #553

Summary

This pr is intended to provide a solution (or at least a better UX) for the cases when a user tries to convert documentation via CLI that results in no pages. 

Documentation with no pages occurs when a user creates article-only documentation and doesn't set a technology root inside the catalog. When this happens, the resulting documentation will show a blank page with an An unknown error occurred. message. This behavior could be frustrating to the users that are just starting with DocC and are not aware of the TechnologyRoot directive.

To improve the user experience, we are emitting a warning both for docc convert and docc preview letting the user know that the resulting documentation doesn't contain any pages and pointing them to the technologyroot doc.

This is the implementation of one of the solutions proposed by @d-ronnqvist in #553, and a topic in the Swift forums will be created to discuss with the rest of the community if this is the most convenient approach or if we should implement a more disruptive solution.

This pr also includes small changes to fix rdar://112462434.

Dependencies

N/A

Testing

Preview and/or a .docc catalog with no technology root. Check the CLI warnings and assert that the following message is printed on screen:

warning: The generated archive does not contain any pages.
For article-only documentation add a Technology Root using the `TechnologyRoot` directive within a `Metadata` directive (https://www.swift.org/documentation/docc/technologyroot).

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 (N/A)

Sofía Rodríguez added 2 commits July 19, 2023 16:30
* Emit warning when converting/previewing article-only catalogs with no technology root
* [rdar://112462434] fix: Emit diagnostics originated in the Convert Action `perform` process
@sofiaromorales sofiaromorales changed the title Issue 553/no pages warning No pages warning Jul 21, 2023
@sofiaromorales sofiaromorales marked this pull request as ready for review July 24, 2023 10:28
@sofiaromorales sofiaromorales marked this pull request as draft July 24, 2023 13:40
@sofiaromorales sofiaromorales marked this pull request as ready for review July 24, 2023 15:06
Copy link
Contributor

@d-ronnqvist d-ronnqvist left a comment

Choose a reason for hiding this comment

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

Since this error message is user-facing I feel that it'd be good to iterate on it to make it more actionable (depending on context, the developer may not be presented the explanation).

@sofiaromorales sofiaromorales marked this pull request as draft July 27, 2023 18:09
@sofiaromorales sofiaromorales marked this pull request as ready for review July 28, 2023 14:38
@d-ronnqvist
Copy link
Contributor

@swift-ci please test

Copy link
Contributor

@d-ronnqvist d-ronnqvist 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. Thank you

@sofiaromorales
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit b98a915 into swiftlang:main Aug 7, 2023
2 checks passed
@sofiaromorales
Copy link
Contributor Author

Thanks @d-ronnqvist

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

4 participants