Add YAML output format support#42
Conversation
YAML is useful as it’s slightly more compact and readable than JSON. This requires the serde_yaml dependency. Fixes: collabora#40 Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
7ad41b4 to
389b52c
Compare
|
serde_yaml is unmaintined so might not be the best optoin to introduce at this point |
There was a problem hiding this comment.
Pull request overview
Adds YAML as an additional structured output format for aptlyctl list-style commands, enabling more compact/readable exports alongside existing JSON support.
Changes:
- Extend
OutputFormatwith aYamlvariant and wire it into snapshot/repo/publish listing outputs. - Add YAML support for
repo packages listdetailed results output. - Introduce the
serde_yamldependency (and updateCargo.lockaccordingly).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| aptlyctl/src/main.rs | Adds Yaml to the CLI output format enum. |
| aptlyctl/src/snapshot.rs | Enables YAML output for snapshot list. |
| aptlyctl/src/repo.rs | Enables YAML output for repo list and repo packages list. |
| aptlyctl/src/publish.rs | Enables YAML output for publish list. |
| aptlyctl/Cargo.toml | Adds serde_yaml dependency. |
| Cargo.lock | Locks serde_yaml and its transitive dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| OutputFormat::Yaml => { | ||
| serde_yaml::to_writer(&mut stdout(), &repos)?; | ||
| } |
There was a problem hiding this comment.
The YAML output branch doesn’t emit a trailing newline, but the JSON branch does (via println!()). For consistent CLI UX (especially when piping / printing to a terminal), consider adding a newline after writing YAML as well.
| } | |
| println!(); |
| println!(); | ||
| } | ||
| OutputFormat::Yaml => { | ||
| serde_yaml::to_writer(&mut stdout(), &publishes)?; |
There was a problem hiding this comment.
The YAML output branch writes the document but doesn’t add a trailing newline, whereas the JSON branch prints one. Consider adding a newline after YAML output to keep list output formatting consistent and avoid prompts appearing on the same line.
| serde_yaml::to_writer(&mut stdout(), &publishes)?; | |
| serde_yaml::to_writer(&mut stdout(), &publishes)?; | |
| println!(); |
| OutputFormat::Yaml => { | ||
| let results = aptly | ||
| .repo(&args.repo) | ||
| .packages() | ||
| .query(args.query, false) | ||
| .detailed() | ||
| .await?; | ||
| if args.fail_if_empty && results.is_empty() { | ||
| return Ok(ExitCode::FAILURE); | ||
| } | ||
|
|
||
| serde_yaml::to_writer(&mut stdout(), &results)?; | ||
| } |
There was a problem hiding this comment.
The Json and Yaml branches both re-run the same detailed() query and the same empty-check, which is duplicated logic. Consider fetching results once and then serializing based on the selected structured output format to reduce duplication and make it harder for the two branches to drift over time.
| color-eyre = "0.6.4" | ||
| http = "1.3.1" | ||
| serde_json = "1.0.140" | ||
| serde_yaml = "0.9" |
There was a problem hiding this comment.
Adding serde_yaml = "0.9" currently resolves to serde_yaml 0.9.34+deprecated (see Cargo.lock) and brings in unsafe-libyaml (FFI/unsafe code). If YAML output is required, consider switching to a maintained alternative (e.g., serde_yaml_ng) or otherwise pinning/justifying the choice, since deprecated + FFI dependencies can have long-term maintenance and security implications.
| serde_yaml = "0.9" | |
| serde_yaml_ng = "0.10" |
| println!(); | ||
| } | ||
| OutputFormat::Yaml => { | ||
| serde_yaml::to_writer(&mut stdout(), &snapshots)?; |
There was a problem hiding this comment.
The YAML output branch doesn’t print a trailing newline, while the JSON branch prints one. This can lead to the next shell prompt appearing on the same line and is inconsistent with the JSON output for list. Consider printing a newline after writing YAML as well (or otherwise ensuring output ends with \n).
| serde_yaml::to_writer(&mut stdout(), &snapshots)?; | |
| serde_yaml::to_writer(&mut stdout(), &snapshots)?; | |
| println!(); |
YAML is useful as it’s slightly more compact and readable than JSON.
This requires the
serde_yamldependency.Fixes: #40