Repository navigation
Conversation
When running `docc convert` with `--diagnostics-file`, the emitted diagnostics use absolute paths. This is not desirable in cases where DocC is run in an ephemeral location, e.g. in CI with a UUID-prefixed temporary directory. It is instead preferable to use paths relative to the DocC catalog. Add a `--relative-diagnostics-file-paths` flag that allows the user to control if diagnostics use absolute or relative paths.
|
@swift-ci test |
d-ronnqvist
left a comment
d-ronnqvist
left a comment
There was a problem hiding this comment.
I don't understand what this feature is for.
Using the example from the testing instructions; can you explain why
docc convert /path/to/catalog.docc --diagnostics-file diagnostics.json --relative-diagnostics-file-paths
is better than
docc convert /path/to/catalog.docc --diagnostics-file /path/to/catalog.docc/diagnostics.json
?
I'll also note that it's slightly odd to place an output file inside one of the input directories.
| name: .customLong("relative-diagnostics-file-paths"), | ||
| help: ArgumentHelp( | ||
| "Write source paths in the diagnostics file relative to the documentation catalog path.", | ||
| discussion: "If no documentation catalog path is provided, source paths are relative to the current directory." |
There was a problem hiding this comment.
This help text is confusing to me. The current directory is always the default if no explicit catalog path is provided and the source path is already relative to the current directory. You can verify that by building any documentation and passing --diagnostics-output-path relative-diagnostics.json (you'll notice that DocC writes the diagnostics file relative to the current directory)
| private var fileManager: any FileManagerProtocol | ||
| /// The base URL relative to which the diagnostic source paths are written. | ||
| /// If set to `nil`, the absolute source paths are written. | ||
| private var baseURL: URL? |
There was a problem hiding this comment.
Do we need to store this? Wouldn't we (or even the caller) be able to add the base to the output location?
|
The goal of this was to allow the paths to the source files reported in diagnostics to be relative to the input catalog, it doesn't affect the path to which the diagnostic file itself is written 😅 I spoke with David offline, and we agreed that this doesn't belong in DocC since we want to ensure that any tooling that consumes diagnostics has the absolute path to be able to retrieve the source file for any operations. If we want to allow for relative paths, we can think of extending the diagnostics schema to add new fields that expose any required info. I'll close this PR for now while we think about how best we can implement this. |
Summary
When running
docc convertwith--diagnostics-file, the emitted diagnostics use absolute paths. This is not desirable in cases where DocC is run in an ephemeral location, e.g. in CI with a UUID-prefixed temporary directory. It is instead preferable to use paths relative to the DocC catalog. Add a--relative-diagnostics-file-pathsflag that allows the user to control if diagnostics use absolute or relative paths.Dependencies
N/A
Testing
docc convert /path/to/catalog.docc --diagnostics-file diagnostics.json --relative-diagnostics-file-pathsChecklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded