Emit symbol URL to link to declaration in repo #256

franklinsch · 2022-05-15T11:35:45Z

Bug/issue #, if applicable: #186, rdar://88537303

Summary

Adds command-line arguments to configure a repo URL so that documentation websites can display a link to the symbol's declaration in the repo.

Example invocation:

swift package generate-documentation \
  --source-service github \
  --source-service-base-url https://github.com/<org>/<repo>/blob/main \
  --checkout-path <path to local checkout>

Dependencies

swiftlang/swift-docc-render#366

Testing

Set the DOCC_EXEC environment variable to a local build of DocC from this branch and run with the flags described above. Verify that documentation pages for Swift symbols contains a .metadata.source field that links to the symbol's declaration in the git service.

Checklist

Added tests
Ran the ./bin/test script and it succeeded
Updated documentation if necessary

TODO

Add documentation
Add tests

franklinsch · 2022-07-01T12:41:26Z

@swift-ci please test

franklinsch · 2022-07-01T12:57:39Z

@swift-ci please test

franklinsch · 2022-07-01T13:06:29Z

@swift-ci please test

franklinsch · 2022-07-01T13:10:11Z

@swift-ci please test

franklinsch · 2022-07-21T08:43:02Z

@ethan-kusters Did you have a chance to take a look at this? The DocC Render PR is moving forward swiftlang/swift-docc-render#366 (review)

ethan-kusters · 2022-07-27T03:44:23Z

Sources/SwiftDocC/Converter/DocumentationContextConverter.swift

    /// Whether the documentation converter should include access level information for symbols.
    let shouldEmitSymbolAccessLevels: Bool

+    /// The source repository where the documentation's sources are hosted.


Suggested change

/// The source repository where the documentation's sources are hosted.

/// The online source control repository where the documentation's source is hosted.

Good idea, I clarified this.

ethan-kusters · 2022-07-27T03:44:53Z

Sources/SwiftDocC/Model/Rendering/RenderNode/RenderMetadata.swift

    /// The variants for the source file URI of a page.
    public var sourceFileURIVariants: VariantCollection<String?> = .init(defaultValue: nil)

+    /// The source location of the topic's source code.


Suggested change

/// The source location of the topic's source code.

/// The source location of the topic's declaration.

I renamed this property and updated the documentation comment:

/// The remote location where the source declaration of the topic can be viewed. public var remoteSource: RemoteSource? {

ethan-kusters · 2022-07-27T03:46:59Z

Sources/SwiftDocC/Model/Rendering/RenderNode/RenderMetadata.swift

    }
+
+    /// Describes the location of the topic's source code, hosted by a source service.
+    public struct Source: Codable, Equatable {


Maybe this should be Location or Declaration? We use Location in SymbolKit it looks like: https://github.com/apple/swift-docc-symbolkit/blob/main/openapi.yaml#L290.

I think Source is a good term for describing the entire collection of a project's source code but I'm not sure it's right for describing a specific symbol's declaration.

I'm leaning towards being ok with "source" either referring to the source repository of the code in the 'semantic analysis' part of DocC and it referring to a specific symbol's remote source file when rendering each topic individually. I've renamed this to remoteSource to avoid confusion with the source of the symbol in a local file.

(fyi @dobromir-hristov, if we go with this name change, we'll need to update the DocC Render PR to update the property name in the metadata object)

No problem, just remind me in the PR with the final name :)

ethan-kusters · 2022-07-27T03:47:20Z

Sources/SwiftDocC/Model/Rendering/RenderNode/RenderMetadata.swift

+
+    /// Describes the location of the topic's source code, hosted by a source service.
+    public struct Source: Codable, Equatable {
+        /// The file name of the topic's source code.


Suggested change

/// The file name of the topic's source code.

/// The name of the file where the topic's declared.

ethan-kusters · 2022-07-27T03:52:39Z

Sources/SwiftDocC/SourceRepository/SourceRepository.swift

+
+    /// Creates a source code repository.
+    /// - Parameters:
+    ///   - checkoutPath: The path at which the repository is checked out locally.


Should we be more specific here? This needs to be the location where the repository was checked out when the given symbol graph was produced, right? Not just any local checkout.

...SwiftDocCUtilities/ArgumentParsing/Options/Source Repository/SourceRepositoryArguments.swift

ethan-kusters · 2022-07-27T04:01:52Z

Looks great overall! Really nicely structured. Just left a few nit-picky comments.

What are your thoughts on Info.plist support? I think it makes sense to land this initially without it (especially with the complications of needing to specify the current local checkout path and the possibility of local source getting out-of-sync with remote source), but I'm wondering if you have thoughts on if we should support this in the future.

It would also be nice if when running docc preview I got links to my local source for free... I guess it's unlikely that there's any kind of OS support for opening a file to a specific line number in the current default editor though.

ethan-kusters · 2022-07-27T04:03:30Z

Sources/SwiftDocC/Model/Rendering/RenderNode/RenderMetadata.swift

    public var sourceFileURIVariants: VariantCollection<String?> = .init(defaultValue: nil)

+    /// The source location of the topic's source code.
+    public var source: Source? {


I left a comment below wondering if we should call this source, but regardless of what general name we land on here – should we specify that this is a remote location? That way it's clear how this is different from the existing sourceFileURI.

Suggested change

public var source: Source? {

public var remoteSource: Source? {

Suggested change

public var source: Source? {

public var remoteLocation: Location? {

Suggested change

public var source: Source? {

public var remoteSourceLocation: SourceLocation? {

dobromir-hristov · 2022-07-27T05:37:49Z

docc-render PR is approved. So let me know when you guys are ready and want to merge it :)

franklinsch · 2022-07-27T13:39:36Z

What are your thoughts on Info.plist support? I think it makes sense to land this initially without it (especially with the complications of needing to specify the current local checkout path and the possibility of local source getting out-of-sync with remote source), but I'm wondering if you have thoughts on if we should support this in the future.

I don't think Info.plist is the right place to specify this data, especially the local checkout path as you mentioned. Instead, I see build systems/IDEs integrate with this in a way that provides sensible values to these arguments, if the user opts into this.

It would also be nice if when running docc preview I got links to my local source for free... I guess it's unlikely that there's any kind of OS support for opening a file to a specific line number in the current default editor though.

This would be nice as well, but opt-in as well I'd say. If your goal with docc preview is to preview a 1:1 mapping of what will be hosted on your documentation website, you may not want to see source links that wouldn't appear in your website.

franklinsch · 2022-08-04T13:44:39Z

Based on feedback above, I've gone ahead and made --checkout-path non-optional, i.e., a value needs to be provided by the user. This is because other DocC behavior assumes your current working directory is a DocC catalog (so that you can simply run docc preview from within a catalog), and DocC catalogs are not usually the root of git repositories.

However, it would be nice if the DocC SwiftPM plugin would automatically fill in --checkout-path to the root path of the local checkout, since SwiftPM already has smarts about git repositories. I've filed swiftlang/swift-docc-plugin#24 to track this enhancement.

franklinsch · 2022-08-04T13:44:47Z

@swift-ci please test

franklinsch · 2022-08-04T13:49:30Z

@swift-ci please test

ethan-kusters

Looks great- thank you!! Only blocking feedback is the OpenAPI spec validation. Very excited to have this in. 🥳

ethan-kusters · 2022-08-05T00:48:35Z

Sources/SwiftDocC/SourceRepository/SourceRepository.swift

+        SourceRepository(
+            checkoutPath: checkoutPath,
+            sourceServiceBaseURL: sourceServiceBaseURL,
+            formatLineNumber: { line in "lines-\(line)" }


It looks like lines- or line- will work here but line- seems a little nicer to me since we'll never be linking to a block.

Suggested change

formatLineNumber: { line in "lines-\(line)" }

formatLineNumber: { line in "line-\(line)" }

I couldn't find an official format reference from BitBucket, but lines- is what their website uses when you click on a file number, e.g., https://bitbucket.org/kannemadugupriyanka/sb1-test-fork-test-test/src/8f1c1b45b4622388b241cf281b595b4e02cef258/Test%20Test/.project#lines-4, so given that it seems safer to use lines-.

Sounds good. I did come across this from BitBucket as a spec for what it's worth: https://support.atlassian.com/bitbucket-cloud/docs/hyperlink-to-source-code-in-bitbucket/.

But it lists something entirely different from what the UI does: #<filename>-<linenumber>.

https://bitbucket.org/kannemadugupriyanka/sb1-test-fork-test-test/src/8f1c1b45b4622388b241cf281b595b4e02cef258/Test%20Test/.project#.project-4

And then the <filename> part is technically optional actually so:

https://bitbucket.org/kannemadugupriyanka/sb1-test-fork-test-test/src/8f1c1b45b4622388b241cf281b595b4e02cef258/Test%20Test/.project#-4

works as well.

Any way 😃 Just matching the behavior of the UI seems fine enough.

Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json

ethan-kusters · 2022-08-05T00:59:12Z

...SwiftDocCUtilities/ArgumentParsing/Options/Source Repository/SourceRepositoryArguments.swift

+                """
+        )
+    )
+    public var sourceService: String?


Super nit-pick but ArgParser I think actually has built-in support for this kind of thing: https://apple.github.io/swift-argument-parser/documentation/argumentparser/declaringarguments/#Parsing-custom-types.

enum ReleaseMode: String, ExpressibleByArgument { case debug, release } struct Example: ParsableCommand { @Option var mode: ReleaseMode mutating func run() throws { print(mode) } }

I had that initially, but I wanted to allow any casing for these values, e.g., both--source-service GitHub and --source-service github, so I implemented the validation logic manually.

rdar://88537303

franklinsch · 2022-08-08T13:28:11Z

@swift-ci please test

Added documentation to configure DocC's new functionality that emits links to symbols' declaration in the project's source repository, which was implemented in swiftlang#256.

Added documentation to configure DocC's new functionality that emits links to symbols' declaration in the project's source repository, which was implemented in #256.

franklinsch marked this pull request as draft May 15, 2022 11:36

franklinsch force-pushed the emit-source-url branch from e41b21d to 7f2be93 Compare May 15, 2022 12:21

Kyle-Ye linked an issue Jun 5, 2022 that may be closed by this pull request

[SR-15828] DocC should have an option to add links from docs to source #186

Closed

franklinsch force-pushed the emit-source-url branch 3 times, most recently from c8d7958 to 250f73c Compare June 12, 2022 09:22

dobromir-hristov mentioned this pull request Jun 23, 2022

Add view in source link to Documentation pages swiftlang/swift-docc-render#366

Merged

3 tasks

franklinsch force-pushed the emit-source-url branch 2 times, most recently from 3d304d2 to b7121f5 Compare July 1, 2022 12:40

franklinsch marked this pull request as ready for review July 1, 2022 12:41

franklinsch force-pushed the emit-source-url branch from b7121f5 to 6c429b9 Compare July 1, 2022 12:57

franklinsch requested a review from ethan-kusters July 1, 2022 12:57

franklinsch force-pushed the emit-source-url branch 2 times, most recently from 21c5978 to 2c432e3 Compare July 1, 2022 13:06

franklinsch force-pushed the emit-source-url branch from 2c432e3 to f75f8ba Compare July 1, 2022 13:09

franklinsch force-pushed the emit-source-url branch from f75f8ba to 44f6cc7 Compare July 19, 2022 12:45

ethan-kusters reviewed Jul 27, 2022

View reviewed changes

franklinsch force-pushed the emit-source-url branch from 44f6cc7 to 43aa251 Compare July 27, 2022 13:36

franklinsch requested a review from ethan-kusters July 27, 2022 13:39

franklinsch force-pushed the emit-source-url branch 2 times, most recently from 4349e0c to cdf218f Compare August 4, 2022 13:32

franklinsch mentioned this pull request Aug 4, 2022

Provide sensible defaults when using --source-service argument swiftlang/swift-docc-plugin#24

Open

franklinsch force-pushed the emit-source-url branch from cdf218f to f84cd8a Compare August 4, 2022 13:47

ethan-kusters requested changes Aug 5, 2022

View reviewed changes

Emit symbol URL to link to declaration in repo

7217cc1

rdar://88537303

franklinsch force-pushed the emit-source-url branch from f84cd8a to 7217cc1 Compare August 8, 2022 13:22

franklinsch requested a review from ethan-kusters August 8, 2022 13:27

ethan-kusters approved these changes Aug 8, 2022

View reviewed changes

franklinsch merged commit 082f8b0 into swiftlang:main Aug 8, 2022

franklinsch deleted the emit-source-url branch August 8, 2022 15:06

franklinsch mentioned this pull request Aug 12, 2022

Add documentation for links to source declarations #360

Merged

2 tasks

Kyle-Ye mentioned this pull request Dec 28, 2023

source link line number mismatch #781

Closed

2 tasks

	/// The source repository where the documentation's sources are hosted.
	/// The online source control repository where the documentation's source is hosted.

	/// The source location of the topic's source code.
	/// The source location of the topic's declaration.

	/// The file name of the topic's source code.
	/// The name of the file where the topic's declared.

	public var source: Source? {
	public var remoteSource: Source? {

	public var source: Source? {
	public var remoteLocation: Location? {

	public var source: Source? {
	public var remoteSourceLocation: SourceLocation? {

	formatLineNumber: { line in "lines-\(line)" }
	formatLineNumber: { line in "line-\(line)" }

Emit symbol URL to link to declaration in repo #256

Emit symbol URL to link to declaration in repo #256

Uh oh!

Conversation

franklinsch commented May 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependencies

Testing

Checklist

TODO

Uh oh!

franklinsch commented Jul 1, 2022

Uh oh!

franklinsch commented Jul 1, 2022

Uh oh!

franklinsch commented Jul 1, 2022

Uh oh!

franklinsch commented Jul 1, 2022

Uh oh!

franklinsch commented Jul 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ethan-kusters commented Jul 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dobromir-hristov commented Jul 27, 2022

Uh oh!

franklinsch commented Jul 27, 2022

Uh oh!

franklinsch commented Aug 4, 2022

Uh oh!

franklinsch commented Aug 4, 2022

Uh oh!

franklinsch commented Aug 4, 2022

Uh oh!

ethan-kusters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franklinsch commented Aug 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

franklinsch commented May 15, 2022 •

edited

Loading