-
Notifications
You must be signed in to change notification settings - Fork 127
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
Support a relative path for the options to "link to source" for the --checkout-path CLI option #490 #929
base: main
Are you sure you want to change the base?
Support a relative path for the options to "link to source" for the --checkout-path CLI option #490 #929
Changes from all commits
be84a97
f08135e
923e328
153cd70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,33 +9,40 @@ | |
*/ | ||
|
||
import Foundation | ||
import ArgumentParser | ||
Anthony-Eid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// A remote repository that hosts source code. | ||
public struct SourceRepository { | ||
/// The path at which the repository is cloned locally. | ||
public var checkoutPath: String | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this was defined as a |
||
|
||
/// The base URL where the service hosts the repository's contents. | ||
public var sourceServiceBaseURL: URL | ||
|
||
/// A function that formats a line number to be included in a URL. | ||
public var formatLineNumber: (Int) -> String | ||
|
||
/// Creates a source code repository. | ||
/// - Parameters: | ||
/// - checkoutPath: The path at which the repository is checked out locally and from which its symbol graphs were generated. | ||
/// - sourceServiceBaseURL: The base URL where the service hosts the repository's contents. | ||
/// - formatLineNumber: A function that formats a line number to be included in a URL. | ||
public init( | ||
public init ( | ||
checkoutPath: String, | ||
sourceServiceBaseURL: URL, | ||
formatLineNumber: @escaping (Int) -> String | ||
) { | ||
self.checkoutPath = checkoutPath | ||
|
||
// Get the absolute path of a file without the file:// prefix because this function used to only | ||
// expect absolute paths from a user and didn't convert checkoutPath to a URL and back. | ||
let absoluteCheckoutPath = URL(fileURLWithPath: checkoutPath).absoluteString | ||
let startIndex = absoluteCheckoutPath.index(absoluteCheckoutPath.startIndex, offsetBy: 7) | ||
self.checkoutPath = String(absoluteCheckoutPath[startIndex...]) | ||
Comment on lines
+36
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've never seen this pattern before and I can't find an input where it behaves differently than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was hesitant to use Also, I've updated the code to use |
||
|
||
self.sourceServiceBaseURL = sourceServiceBaseURL | ||
self.formatLineNumber = formatLineNumber | ||
} | ||
|
||
/// Formats a local source file URL to a URL hosted by the remote source code service. | ||
/// - Parameters: | ||
/// - sourceFileURL: The location of the source file on disk. | ||
|
@@ -45,7 +52,7 @@ public struct SourceRepository { | |
guard sourceFileURL.path.hasPrefix(checkoutPath) else { | ||
return nil | ||
} | ||
|
||
let path = sourceFileURL.path.dropFirst(checkoutPath.count).removingLeadingSlash | ||
return sourceServiceBaseURL | ||
.appendingPathComponent(path) | ||
|
@@ -65,7 +72,7 @@ public extension SourceRepository { | |
formatLineNumber: { line in "L\(line)" } | ||
) | ||
} | ||
|
||
/// Creates a source repository hosted by the GitLab service. | ||
/// - Parameters: | ||
/// - checkoutPath: The path of the local checkout. | ||
|
@@ -77,7 +84,7 @@ public extension SourceRepository { | |
formatLineNumber: { line in "L\(line)" } | ||
) | ||
} | ||
|
||
/// Creates a source repository hosted by the BitBucket service. | ||
/// - Parameters: | ||
/// - checkoutPath: The path of the local checkout. | ||
|
@@ -89,7 +96,7 @@ public extension SourceRepository { | |
formatLineNumber: { line in "lines-\(line)" } | ||
) | ||
} | ||
|
||
/// Creates a source repository hosted by the device's filesystem. | ||
/// | ||
/// Use this source repository to format `doc-source-file://` links to files on the | ||
|
@@ -98,7 +105,7 @@ public extension SourceRepository { | |
/// This source repository uses a custom scheme to offer more control local source file navigation. | ||
static func localFilesystem() -> SourceRepository { | ||
SourceRepository( | ||
checkoutPath: "", | ||
checkoutPath: "/", | ||
Anthony-Eid marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// 2 slashes to specify an empty authority/host component and 1 slash to specify a base path at the root. | ||
sourceServiceBaseURL: URL(string: "doc-source-file:///")!, | ||
formatLineNumber: { line in "L\(line)" } | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -30,7 +30,7 @@ or use Xcode's _Build Documentation_ command. | |||||
|
||||||
Alternatively, use the `docc` command-line tool directly, for example: | ||||||
|
||||||
```shell | ||||||
```shell | ||||||
docc convert MyNewPackage.docc \ | ||||||
--fallback-display-name MyNewPackage \ | ||||||
--fallback-bundle-identifier com.example.MyNewPackage \ | ||||||
|
@@ -61,28 +61,25 @@ service configuration flags like so: | |||||
docc convert […] \ | ||||||
--source-service github \ | ||||||
--source-service-base-url https://github.com/<org>/<repo>/blob/<branch> \ | ||||||
--checkout-path <absolute path to local checkout> | ||||||
--checkout-path <absolute or relative path to local checkout> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't find it necessary to explicitly mention that both absolute and relative paths are supported
Suggested change
|
||||||
``` | ||||||
|
||||||
**GitLab** | ||||||
```bash | ||||||
docc convert […] \ | ||||||
--source-service gitlab \ | ||||||
--source-service-base-url https://gitlab.com/<org>/<repo>/-/tree/<branch> \ | ||||||
--checkout-path <absolute path to local checkout> | ||||||
--checkout-path <absolute or relative path to local checkout> | ||||||
``` | ||||||
|
||||||
**BitBucket** | ||||||
```bash | ||||||
docc convert […] \ | ||||||
--source-service bitbucket \ | ||||||
--source-service-base-url https://bitbucket.org/<org>/<repo>/src/<branch> \ | ||||||
--checkout-path <absolute path to local checkout> | ||||||
--checkout-path <absolute or relative path to local checkout> | ||||||
``` | ||||||
|
||||||
> Note: The option `--checkout-path` expects an absolute path to where the package | ||||||
is checked out, not a relative path. | ||||||
|
||||||
These arguments can also be provided to `swift package generate-documentation` | ||||||
if you're using the SwiftPM DocC Plugin or via the `OTHER_DOCC_FLAGS` build | ||||||
setting when building in Xcode. | ||||||
|
@@ -115,22 +112,22 @@ https://www.example.com/documentation/MyNewPackage/MyNewProtocol | |||||
|
||||||
#### Host a Documentation Archive with a File Server | ||||||
|
||||||
You can host documentation archives you create with DocC from the Swift 5.7 | ||||||
toolchain and later using a regular file server. By default, the server hosts | ||||||
the documentation at the root of the website, like the "MyNewPackage" example | ||||||
above. To host the documentation at a specific subpath, pass a custom hosting | ||||||
You can host documentation archives you create with DocC from the Swift 5.7 | ||||||
toolchain and later using a regular file server. By default, the server hosts | ||||||
the documentation at the root of the website, like the "MyNewPackage" example | ||||||
above. To host the documentation at a specific subpath, pass a custom hosting | ||||||
base path for the `--hosting-base-path` option when you build the documentation | ||||||
archive. | ||||||
archive. | ||||||
|
||||||
```shell | ||||||
```shell | ||||||
docc convert MyNewPackage.docc \ | ||||||
--additional-symbol-graph-dir .build \ | ||||||
--output-dir MyNewPackage.doccarchive \ | ||||||
--hosting-base-path MyProject/Base/Path | ||||||
--hosting-base-path MyProject/Base/Path | ||||||
``` | ||||||
|
||||||
DocC adds the provided hosting base path before the path of each documentation | ||||||
page. For example, the URL to view for `MyNewProtocol` protocol in the | ||||||
page. For example, the URL to view for `MyNewProtocol` protocol in the | ||||||
`MyNewPackage` documentation might resemble the following: | ||||||
|
||||||
``` | ||||||
|
@@ -147,9 +144,9 @@ the `DOCC_HOSTING_BASE_PATH` build setting when building documentation in Xcode. | |||||
|
||||||
#### Host a Documentation Archive Using Custom Routing | ||||||
|
||||||
A file server is the recommended solution to host your documentation. But, if | ||||||
A file server is the recommended solution to host your documentation. But, if | ||||||
you need more control over how the server hosts your content, you can configure | ||||||
the request routing of your web server so it responds to documentation requests | ||||||
the request routing of your web server so it responds to documentation requests | ||||||
with the data and assets within the documentation archive. | ||||||
|
||||||
> Note: The following sections use Apache as an example. Other web server | ||||||
|
@@ -186,7 +183,7 @@ RewriteRule .* MyNewPackage.doccarchive/$0 [L] | |||||
``` | ||||||
|
||||||
With these rules in place, the web server provides access to the contents of | ||||||
the documentation archive. | ||||||
the documentation archive. | ||||||
|
||||||
After configuring your web server to host a documentation archive, keep it up | ||||||
to date by using a continuous integration workflow that builds the | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,26 +20,35 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
withExtension: "docc", | ||
subdirectory: "Test Bundles" | ||
)! | ||
|
||
private let testTemplateURL = Bundle.module.url( | ||
forResource: "Test Template", | ||
withExtension: nil, | ||
subdirectory: "Test Resources" | ||
)! | ||
|
||
func testSourceRepositoryAllArgumentsSpecified() throws { | ||
let tempDir = try createTemporaryDirectory(pathComponents: "addFileToCreateValidDirectory") | ||
|
||
// removing file:// prefix from checkout path because the directory is not | ||
// recognized as a valid directory with it | ||
let checkoutPath = tempDir.absoluteString | ||
let startIndex = checkoutPath.index(checkoutPath.startIndex, offsetBy: 7) | ||
let absoluteCheckoutPath = String(checkoutPath[startIndex...]) | ||
|
||
|
||
for sourceService in ["github", "gitlab", "bitbucket"] { | ||
try assertSourceRepositoryArguments( | ||
checkoutPath: "checkout path", | ||
checkoutPath: absoluteCheckoutPath, | ||
Comment on lines
-33
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you pass the original value ( |
||
sourceService: sourceService, | ||
sourceServiceBaseURL: "https://example.com/path/to/base" | ||
) { action in | ||
XCTAssertEqual(action.sourceRepository?.checkoutPath, "checkout path") | ||
XCTAssertEqual(action.sourceRepository?.checkoutPath, "\(absoluteCheckoutPath)/") | ||
XCTAssertEqual(action.sourceRepository?.sourceServiceBaseURL.absoluteString, "https://example.com/path/to/base") | ||
} | ||
} | ||
} | ||
|
||
func testDoesNotSetSourceRepositoryIfBothCheckoutPathAndsourceServiceBaseURLArgumentsAreMissing() throws { | ||
try assertSourceRepositoryArguments( | ||
checkoutPath: nil, | ||
|
@@ -50,6 +59,28 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenCheckoutPathIsInvalid() throws { | ||
let tempDir = try createTemporaryDirectory(named: "tmp").appendingPathComponent("InvalidDirectory", isDirectory: false) | ||
let absoluteCheckoutPath = tempDir.absoluteString | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, this test if verifying that using a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That test is checking that an error is thrown when a path is used as an argument, but the directories of the path don't exist. Specifically, it's testing this guard I added in SourceRepositoryArguments.swift guard FileManager.default.directoryExists(atPath: checkoutPath) else {
throw ValidationError("Checkout path directory '\(checkoutPath)' doesn't exist for --checkout-path argument.")
} To the end of the |
||
|
||
for sourceService in ["github", "gitlab", "bitbucket"] { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
checkoutPath: absoluteCheckoutPath, | ||
sourceService: sourceService, | ||
sourceServiceBaseURL: "https://example.com/path/to/base" | ||
) | ||
) { error in | ||
XCTAssertEqual( | ||
(error as? ValidationError)?.message, | ||
""" | ||
Checkout path directory '\(absoluteCheckoutPath)' doesn't exist for --checkout-path argument. | ||
""" | ||
) | ||
} | ||
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenSourceServiceIsSpecifiedButNotSourceServiceBaseURL() throws { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
|
@@ -67,7 +98,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
) | ||
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenSourceServiceBaseURLIsSpecifiedButNotSourceService() throws { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
|
@@ -85,7 +116,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
) | ||
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenSourceServiceBaseURLIsInvalid() throws { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
|
@@ -100,7 +131,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
) | ||
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenCheckoutPathIsNotSpecified() throws { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
|
@@ -118,7 +149,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
) | ||
} | ||
} | ||
|
||
func testThrowsValidationErrorWhenSourceServiceIsInvalid() throws { | ||
XCTAssertThrowsError( | ||
try assertSourceRepositoryArguments( | ||
|
@@ -133,15 +164,15 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
) | ||
} | ||
} | ||
|
||
private func assertSourceRepositoryArguments( | ||
checkoutPath: String?, | ||
sourceService: String?, | ||
sourceServiceBaseURL: String?, | ||
assertion: ((ConvertAction) throws -> Void)? = nil | ||
) throws { | ||
SetEnvironmentVariable(TemplateOption.environmentVariableKey, testTemplateURL.path) | ||
|
||
var arguments: [String] = [testBundleURL.path] | ||
if let checkoutPath { | ||
arguments.append(contentsOf: ["--checkout-path", checkoutPath]) | ||
|
@@ -152,9 +183,9 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase { | |
if let sourceServiceBaseURL { | ||
arguments.append(contentsOf: ["--source-service-base-url", sourceServiceBaseURL]) | ||
} | ||
|
||
let convertOptions = try Docc.Convert.parse(arguments) | ||
|
||
let result = try ConvertAction(fromConvertCommand: convertOptions) | ||
try assertion?(result) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a significant amount of whitespace-only changes in all these files which adds noise to the PR. Please remove all those.