Add snippet support #61

bitjammer · 2021-12-16T21:52:54Z

Add snippet support

Add a @Snippet block directive that takes one argument: the path
to the snippet as if it were a symbol link.
Render a snippet as follows:
- The snippet explanation
- For each snippet chunk:
  - The name in bold if present
  - The source code as a code block

Snippets are transmitted to DocC via the Symbol Graph format.

Snippets do not have their own dedicated page as they won't have accompanying
long-form content. Snippets may be collected in a snippet "index" where one can
easily search or browse for them in one place, where they'll be grouped
appropriately.

This requires apple/swift-docc-symbolkit#10 (now merged)

This also requires apple/swift-docc-symbolkit#15.

bitjammer · 2021-12-16T21:53:05Z

@franklinsch How would you like this tested?

franklinsch · 2022-01-04T10:48:51Z

@bitjammer What do you think about the following:

Tests for Snippet.swift, to verify that we can parse the directive and emit the right diagnostics (we do the same for the other directives)
Integration-style test like the ones in SemaToRenderNodeTests, to verify the parsing -> render JSON emissions flow. We could add some snippets to one of the test bundles.

Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift

bitjammer · 2022-01-13T01:36:18Z

Tests added.

bitjammer · 2022-01-13T03:32:50Z

@swift-ci Please test

bitjammer · 2022-01-13T03:33:52Z

@swift-ci Please test Linux Platform

bitjammer · 2022-01-19T01:45:56Z

@swift-ci Please test

bitjammer · 2022-01-19T01:59:44Z

Not sure if this repo supports building with other PRs – it needs apple/swift-docc-symbolkit#15. Temporarily edited Package.{swift,resolved} for the time being.

bitjammer · 2022-01-19T01:59:49Z

@swift-ci Please test

bitjammer · 2022-01-19T02:19:36Z

replace_acceptable_years in check_source really needs another look at some point. I've updated check_source to work with 2022 to keep going on this PR.

bitjammer · 2022-01-19T02:22:41Z

@swift-ci Please test

bitjammer · 2022-01-19T21:16:41Z

@swift-ci Please test macOS Platform

bitjammer · 2022-01-19T21:31:38Z

Okay @franklinsch ready for review when you have a moment. Thanks!

bitjammer · 2022-01-19T21:31:48Z

@swift-ci Please test

d-ronnqvist · 2022-01-21T01:21:18Z

Tests/SwiftDocCTests/Converter/DocumentationContextConverterTests.swift

+            guard !context.onlyHasSnippetRelatedChildren(for: documentationNode.reference),
+                  documentationNode.kind != .snippet,
+                  documentationNode.kind != .snippetGroup else {
+                      continue


Since knownPages already filter out pages with only snippet related content, would it make sense to fail the test if there's an unexpected snippet page?

Snippets don’t get their own page but are in the topic graph (for linking), so the fake module page would be empty. That knownPages filter is to keep those empty module topics from getting registered as a top level page. So snippets are expected in this test.

d-ronnqvist · 2022-01-21T02:05:17Z

Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift

+                return []
+            }
+
+            let docCommentContent = snippetEntity.markup.children.flatMap { self.visit($0) }


Have I understood correctly that this comes from the snippet's doc comment? If so, does that mean that snippets support styled inline content (e.g. strong and emphasis)?

If that's supported, would it be worth adding some styling to the test data?

That's right, you can write styled markdown in those comments. I'm not convinced that it needs its own test case since it's just a call to visit and those methods are already tested independently, but let me know if you feel strongly about it.

d-ronnqvist · 2022-01-21T02:07:40Z

Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift

+                var elements = [RenderContent]()
+
+                if let chunkName = chunk.name {
+                    elements.append(RenderBlockContent.paragraph(inlineContent: [


This code path isn't taken in the tests. I think that it'd be good to add a name to the snippet in the test data to be able to assert that that content is processed as expected.

Sure thing.

As an aside, chunk names aren't yet authorable but I can add some more test cases once that's implemented on the SwiftPM side if that's all right?

Yes. That sounds good to me.

d-ronnqvist

This looks good to me. The Test Bundle displays the snipped like how I would expect.

I left two comments about potential cases that could also be tested but I don't feel that either of those is a required changes.

bitjammer · 2022-01-21T02:25:02Z

Updated the Package.resolved now that I've merged apple/swift-docc-symbolkit#15.

@swift-ci Please test

- Add a `@Snippet` block directive that takes one argument: the path to the snippet as if it were a symbol link. - Render a snippet as follows: - The snippet explanation - For each snippet chunk: - The name in bold if present - The source code as a code block Snippets do not have their own dedicated page as they won't have accompanying long-form content. Snippets may be collected in a snippet "index" where one can easily search or browse for them in one place, where they'll be grouped appropriately.

bitjammer · 2022-01-21T02:25:39Z

@swift-ci Please test

bitjammer requested a review from franklinsch December 16, 2021 21:52

Kyle-Ye mentioned this pull request Dec 21, 2021

Add snippet mixin for symbols apple/swift-docc-symbolkit#10

Merged

franklinsch reviewed Jan 4, 2022

View reviewed changes

Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift Outdated Show resolved Hide resolved

bitjammer force-pushed the acgarland/snippets branch 4 times, most recently from 550c0f0 to 4eaabc1 Compare January 13, 2022 01:34

bitjammer requested a review from franklinsch January 13, 2022 01:36

bitjammer force-pushed the acgarland/snippets branch from 4eaabc1 to de7a096 Compare January 13, 2022 03:32

bitjammer removed the request for review from franklinsch January 13, 2022 04:20

bitjammer force-pushed the acgarland/snippets branch from de7a096 to eb3aa49 Compare January 13, 2022 05:59

bitjammer changed the title ~~Add snippet support~~ [WIP] Add snippet support Jan 18, 2022

bitjammer mentioned this pull request Jan 18, 2022

Add .snippet and .snippetGroup kind apple/swift-docc-symbolkit#15

Merged

bitjammer force-pushed the acgarland/snippets branch from eb3aa49 to 2d238c5 Compare January 19, 2022 01:45

bitjammer force-pushed the acgarland/snippets branch from 2d238c5 to ece172e Compare January 19, 2022 01:58

bitjammer force-pushed the acgarland/snippets branch from ece172e to 9f7a87e Compare January 19, 2022 02:21

bitjammer requested a review from franklinsch January 19, 2022 21:31

bitjammer changed the title ~~[WIP] Add snippet support~~ Add snippet support Jan 20, 2022

d-ronnqvist reviewed Jan 21, 2022

View reviewed changes

d-ronnqvist approved these changes Jan 21, 2022

View reviewed changes

bitjammer force-pushed the acgarland/snippets branch from 8cb0fd7 to 72cf9b8 Compare January 21, 2022 02:24

bitjammer force-pushed the acgarland/snippets branch from 72cf9b8 to d395f71 Compare January 21, 2022 02:25

bitjammer merged commit 041bc6c into apple:main Jan 21, 2022

bitjammer deleted the acgarland/snippets branch January 21, 2022 03:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add snippet support #61

Add snippet support #61

bitjammer commented Dec 16, 2021 •

edited

bitjammer commented Dec 16, 2021

franklinsch commented Jan 4, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022 •

edited

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

d-ronnqvist Jan 21, 2022

bitjammer Jan 21, 2022

d-ronnqvist Jan 21, 2022

bitjammer Jan 21, 2022

d-ronnqvist Jan 21, 2022

bitjammer Jan 21, 2022

bitjammer Jan 21, 2022

d-ronnqvist Jan 21, 2022

d-ronnqvist left a comment

bitjammer commented Jan 21, 2022

bitjammer commented Jan 21, 2022

Add snippet support #61

Add snippet support #61

Conversation

bitjammer commented Dec 16, 2021 • edited

bitjammer commented Dec 16, 2021

franklinsch commented Jan 4, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 13, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022 • edited

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

bitjammer commented Jan 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-ronnqvist left a comment

Choose a reason for hiding this comment

bitjammer commented Jan 21, 2022

bitjammer commented Jan 21, 2022

bitjammer commented Dec 16, 2021 •

edited

bitjammer commented Jan 19, 2022 •

edited