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

add @Available directive for setting platform availability #440

Merged
merged 10 commits into from
Jan 6, 2023

Conversation

QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented Dec 15, 2022

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

Summary

This PR adds a new metadata directive: @Available. This can be used in articles and extension files to set platform availability for a documentation page. (When used in an extension, it overrides any information that came from the symbol itself.)

This directive was proposed in this forums thread: https://forums.swift.org/t/setting-a-documentation-article-s-platform-availability/61144

Note that while the thread proposed isBeta and isDeprecated arguments (and this PR parses them), they are currently not usable because of work that needs to happen in Swift-DocC-Render to support the behavior. The work in Swift-DocC is being tracked in #441.

Dependencies

None

Testing

Use the following Markdown as a test article:

# My Cool Article

@Metadata {
    @Available(macOS, introduced: "12.0")
}

This is a cool thing you can do.

Steps:

  1. Add an article with the above markdown as Sources/SwiftDocC/SwiftDocC.docc/SwiftDocC/AvailableArticle.md.
  2. bin/preview-docs
  3. Ensure that "My Cool Article" shows "macOS 12.0+" in its page header:

image

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

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

Sources/SwiftDocC/Semantics/Metadata/Availability.swift Outdated Show resolved Hide resolved
Sources/SwiftDocC/Semantics/Metadata/Availability.swift Outdated Show resolved Hide resolved
Sources/SwiftDocC/Semantics/Metadata/Availability.swift Outdated Show resolved Hide resolved
/// @Available(macOS, introduced: "12.0")
/// }
/// ```
public final class MetadataAvailability: Semantic, AutomaticDirectiveConvertible {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to not just call this Availability? If so – I personally would lean towards AvailabilityMetadata. MetadataAvailability makes me think this affects the availability of the metadata.

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 feel like the term "availability" is pretty overloaded already in the Swift-DocC codebase; there's already a SymbolGraph.Symbol.Availability in SymbolKit which could collide in the wrong situation, but SymbolKit also has AvailabilityItem docc itself adds AvailabilityRenderItem as well. AvailabilityMetadata seems like a fine enough rename; i could add that if you think the existing name is too confusing.

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 i want to keep the name, since my reasoning behind it is that it's "availability from the metadata", as opposed to "availability from the symbol" (SymbolGraph.Symbol.Availability). I would have nested the type declaration under Metadata, but the other Metadata directive types are also standalone so i decided to make it fit in with the others.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would have nested the type declaration under Metadata, but the other Metadata directive types are also standalone so i decided to make it fit in with the others.

I agree with your initial thought here. My preference would be to nest this as an extension under Metadata – that fits in well with how the Row/Column and TabNavigator/Tab directives are structured today so we have some precedent at least and we could do the same with other commonly-named Metadata directives moving forward. (Eventually I'd like to split out directives into their own library so we don't run into these kind of collisions.) What do you think? My thinking is that it's more important to maintain the one-to-one relationship of container-class-API-name to directive-name relationship we currently have for the vast majority of directives.

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 i want to keep the name, since my reasoning behind it is that it's "availability from the metadata", as opposed to "availability from the symbol" (SymbolGraph.Symbol.Availability).

My objection to this is that I think a reasonable read of both of these is "availability for the metadata" and "availability for the symbol".

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 nesting the type under Metadata is the right move. I'll make the change.

Sources/SwiftDocC/Semantics/Metadata/Metadata.swift Outdated Show resolved Hide resolved
Sources/SwiftDocC/Semantics/Metadata/Availability.swift Outdated Show resolved Hide resolved
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

2 similar comments
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus merged commit 9c28bc4 into apple:main Jan 6, 2023
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

3 participants