From ccd4163f47eac12e5615234411555a0e547c3637 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 14 Jun 2025 18:47:39 +0100 Subject: [PATCH 1/4] Add global repo config via OrthoConfig --- Cargo.lock | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 11 +++ README.md | 15 +++- src/main.rs | 120 ++++++++++++++++++++++++++----- 4 files changed, 327 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d384f791..73b861bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,15 @@ version = "1.0.98" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" +[[package]] +name = "atomic" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d818003e740b63afc82337e3160717f4f63078720a810b7b903e70a5d1d2994" +dependencies = [ + "bytemuck", +] + [[package]] name = "autocfg" version = "1.4.0" @@ -127,6 +136,12 @@ version = "3.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "793db76d6187cd04dff33004d8e6c9cc4e05cd330500379d2394209271b4aeee" +[[package]] +name = "bytemuck" +version = "1.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c76a5792e44e4abe34d3abf15636779261d45a7450612059293d1d2cfc63422" + [[package]] name = "bytes" version = "1.10.1" @@ -158,6 +173,18 @@ dependencies = [ "clap_derive", ] +[[package]] +name = "clap-dispatch" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a558b9547b590c876e46e301da15d3b0e93b0384fd50d2c7870f7ea86760df5" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "clap_builder" version = "4.5.39" @@ -418,6 +445,22 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "figment" +version = "0.10.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cb01cd46b0cf372153850f4c6c272d9cbea2da513e07538405148f95bd789f3" +dependencies = [ + "atomic", + "parking_lot", + "pear", + "serde", + "tempfile", + "toml", + "uncased", + "version_check", +] + [[package]] name = "fnv" version = "1.0.7" @@ -787,6 +830,12 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "inlinable_string" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8fae54786f62fb2918dcfae3d568594e50eb9b5c25bf04371af6fe7516452fb" + [[package]] name = "ipnet" version = "2.11.0" @@ -1011,6 +1060,32 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "ortho_config" +version = "0.2.0" +source = "git+https://github.com/leynos/ortho-config?tag=v0.2.0#79860f4fb0f6c1293e0fce024f75acee17254f0e" +dependencies = [ + "clap", + "clap-dispatch", + "figment", + "ortho_config_macros", + "serde", + "thiserror 1.0.69", + "toml", + "uncased", + "xdg", +] + +[[package]] +name = "ortho_config_macros" +version = "0.2.0" +source = "git+https://github.com/leynos/ortho-config?tag=v0.2.0#79860f4fb0f6c1293e0fce024f75acee17254f0e" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "parking_lot" version = "0.12.4" @@ -1034,6 +1109,29 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "pear" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bdeeaa00ce488657faba8ebf44ab9361f9365a97bd39ffb8a60663f57ff4b467" +dependencies = [ + "inlinable_string", + "pear_codegen", + "yansi", +] + +[[package]] +name = "pear_codegen" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bab5b985dc082b345f812b7df84e1bef27e7207b39e448439ba8bd69c93f147" +dependencies = [ + "proc-macro2", + "proc-macro2-diagnostics", + "quote", + "syn", +] + [[package]] name = "percent-encoding" version = "2.3.1" @@ -1076,6 +1174,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proc-macro2-diagnostics" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "version_check", + "yansi", +] + [[package]] name = "quote" version = "1.0.40" @@ -1344,6 +1455,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "serde_urlencoded" version = "0.7.1" @@ -1648,6 +1768,47 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_write", + "winnow", +] + +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "tower-service" version = "0.3.3" @@ -1679,6 +1840,15 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "uncased" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1b88fcfe09e89d3866a5c11019378088af2d24c3fbd4f0543f96b479ec90697" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-ident" version = "1.0.18" @@ -1732,6 +1902,12 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "vk" version = "0.1.0" @@ -1739,6 +1915,8 @@ dependencies = [ "anyhow", "clap", "diffy", + "figment", + "ortho_config", "regex", "reqwest", "serde", @@ -1748,7 +1926,10 @@ dependencies = [ "termimad", "thiserror 1.0.69", "tokio", + "toml", + "uncased", "url", + "xdg", ] [[package]] @@ -2032,6 +2213,15 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winnow" +version = "0.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74c7b26e3480b707944fc872477815d29a8e429d2f93a1ce000f5fa84a15cbcd" +dependencies = [ + "memchr", +] + [[package]] name = "winreg" version = "0.50.0" @@ -2057,6 +2247,18 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea2f10b9bb0928dfb1b42b65e1f9e36f7f54dbdf08457afefb38afcdec4fa2bb" +[[package]] +name = "xdg" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fb433233f2df9344722454bc7e96465c9d03bff9d77c248f9e7523fe79585b5" + +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" + [[package]] name = "yoke" version = "0.8.0" diff --git a/Cargo.toml b/Cargo.toml index 6de3f9a9..f86fbc01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,17 @@ url = "2.5.0" regex = "1.10.3" diffy = "0.4.2" anyhow = "1.0" +ortho_config = { git = "https://github.com/leynos/ortho-config", tag = "v0.2.0" } +figment = { version = "0.10", default-features = false, features = ["env", "toml"] } +xdg = "3" +uncased = "0.9" +toml = "0.8" + +[features] +default = ["toml"] +toml = [] +json5 = [] +yaml = [] [dev-dependencies] tempfile = "3.20.0" diff --git a/README.md b/README.md index e166128f..8b6bda99 100644 --- a/README.md +++ b/README.md @@ -7,14 +7,23 @@ This tool is intended for use by AI coding agents such as Aider, OpenAI Codex or ## Usage ```bash -vk +vk pr ``` +`vk` now uses [OrthoConfig](https://github.com/leynos/ortho-config) v0.2.0 for +configuration. A global `--repo` option (or `VK_REPO` environment variable) +sets the default repository when passing only a pull request number. The two +available subcommands are: + +* `pr` - show unresolved pull request comments (existing behaviour) +* `issue` - read a GitHub issue (**to do**) + If you pass just a pull request number, `vk` tries to work out which repository you meant. It first examines `.git/FETCH_HEAD` for a GitHub remote URL and, if found, extracts the `owner/repo` from it. As Codex does not put the upstream URL in `.git/config`, we must obtain this from `FETCH_HEAD` for now. Failing that, it falls back to the -`VK_REPO` environment variable which should be set to `owner/repo` (with or -without a `.git` suffix). If neither source is available, `vk` will refuse to +`VK_REPO` environment variable or `--repo` flag (both handled by +OrthoConfig) which should be set to `owner/repo` (with or without a `.git` +suffix). If neither source is available, `vk` will refuse to run with only a number. `vk` uses the GitHub GraphQL API. Set `GITHUB_TOKEN` to authenticate. If it's not diff --git a/src/main.rs b/src/main.rs index 44ccaffd..f891031f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,9 @@ -use clap::Parser; +#![allow(non_snake_case)] +use clap::{Parser, Subcommand}; +use ortho_config::{OrthoConfig, load_and_merge_subcommand_for}; use regex::Regex; use reqwest::header::{ACCEPT, AUTHORIZATION, HeaderMap, USER_AGENT}; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_json::json; use std::sync::LazyLock; use std::{env, fs, path::Path}; @@ -9,13 +11,55 @@ use termimad::MadSkin; use thiserror::Error; use url::Url; +#[derive(Subcommand, Deserialize, Serialize, Clone, Debug)] +enum Commands { + /// Show unresolved pull request comments + Pr(PrArgs), + /// Read a GitHub issue (todo) + Issue(IssueArgs), +} + +impl Default for Commands { + fn default() -> Self { + Commands::Pr(PrArgs::default()) + } +} + #[derive(Parser)] -#[command(name = "vk", about = "View Komments - show unresolved PR comments")] -struct Args { +#[command( + name = "vk", + about = "View Komments - show unresolved PR comments", + subcommand_required = true, + arg_required_else_help = true +)] +struct Cli { + #[command(subcommand)] + command: crate::Commands, + #[command(flatten)] + global: GlobalArgsCli, +} + +#[derive(Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +#[ortho_config(prefix = "VK")] +struct GlobalArgs { + /// Repository used when passing only a pull request number + repo: Option, +} + +#[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +#[ortho_config(prefix = "VK")] +struct PrArgs { /// Pull request URL or number reference: String, } +#[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +#[ortho_config(prefix = "VK")] +struct IssueArgs { + /// Issue URL or number + reference: String, +} + #[derive(Debug, Clone)] struct RepoInfo { owner: String, @@ -34,6 +78,10 @@ enum VkError { BadResponse, #[error("API errors: {0}")] ApiErrors(String), + #[error("configuration error: {0}")] + Config(#[from] ortho_config::OrthoError), + #[error("{0} not implemented yet")] + Unimplemented(&'static str), } static GITHUB_RE: LazyLock = @@ -413,8 +461,9 @@ fn build_headers(token: &str) -> HeaderMap { headers } -async fn run(args: Args) -> Result<(), VkError> { - let (repo, number) = parse_reference(&args.reference)?; +#[allow(clippy::result_large_err)] +async fn run_pr(args: PrArgs, repo: Option<&str>) -> Result<(), VkError> { + let (repo, number) = parse_reference(&args.reference, repo)?; let token = env::var("GITHUB_TOKEN").unwrap_or_default(); if token.is_empty() { eprintln!("warning: GITHUB_TOKEN not set, using anonymous API access"); @@ -442,11 +491,27 @@ async fn run(args: Args) -> Result<(), VkError> { } #[tokio::main] +#[allow(clippy::result_large_err)] async fn main() -> Result<(), VkError> { - run(Args::parse()).await + let cli = Cli::parse(); + let mut global = GlobalArgs::load_from_iter(std::iter::empty::())?; + if let Some(repo) = cli.global.repo { + global.repo = Some(repo); + } + match cli.command { + Commands::Pr(pr_cli) => { + let args = load_and_merge_subcommand_for::(&pr_cli)?; + run_pr(args, global.repo.as_deref()).await + } + Commands::Issue(issue_cli) => { + let _args = load_and_merge_subcommand_for::(&issue_cli)?; + Err(VkError::Unimplemented("issue command")) + } + } } -fn parse_reference(input: &str) -> Result<(RepoInfo, u64), VkError> { +#[allow(clippy::result_large_err)] +fn parse_reference(input: &str, default_repo: Option<&str>) -> Result<(RepoInfo, u64), VkError> { if let Ok(url) = Url::parse(input) { if url.host_str() == Some("github.com") { let segments_iter = url.path_segments().ok_or(VkError::InvalidRef)?; @@ -464,7 +529,7 @@ fn parse_reference(input: &str) -> Result<(RepoInfo, u64), VkError> { Err(VkError::InvalidRef) } else if let Ok(number) = input.parse::() { let repo = repo_from_fetch_head() - .or_else(repo_from_env) + .or_else(|| default_repo.and_then(repo_from_str)) .ok_or(VkError::RepoNotFound)?; Ok((repo, number)) } else { @@ -489,9 +554,8 @@ fn repo_from_fetch_head() -> Option { None } -fn repo_from_env() -> Option { - let repo = env::var("VK_REPO").ok()?; - if let Some(caps) = GITHUB_RE.captures(&repo) { +fn repo_from_str(repo: &str) -> Option { + if let Some(caps) = GITHUB_RE.captures(repo) { let owner = caps.name("owner")?.as_str().to_string(); let name = caps.name("repo")?.as_str().to_string(); Some(RepoInfo { owner, name }) @@ -536,7 +600,8 @@ mod tests { #[test] fn parse_url() { - let (repo, number) = parse_reference("https://github.com/owner/repo/pull/42").unwrap(); + let (repo, number) = + parse_reference("https://github.com/owner/repo/pull/42", None).unwrap(); assert_eq!(repo.owner, "owner"); assert_eq!(repo.name, "repo"); assert_eq!(number, 42); @@ -544,7 +609,8 @@ mod tests { #[test] fn parse_url_git_suffix() { - let (repo, number) = parse_reference("https://github.com/owner/repo.git/pull/7").unwrap(); + let (repo, number) = + parse_reference("https://github.com/owner/repo.git/pull/7", None).unwrap(); assert_eq!(repo.owner, "owner"); assert_eq!(repo.name, "repo"); assert_eq!(number, 7); @@ -569,12 +635,16 @@ mod tests { } #[test] - fn repo_from_env_git_suffix() { - set_var("VK_REPO", "a/b.git"); - let repo = repo_from_env().unwrap(); + fn repo_from_str_git_suffix() { + let repo = repo_from_str("a/b.git").unwrap(); assert_eq!(repo.owner, "a"); assert_eq!(repo.name, "b"); - remove_var("VK_REPO"); + } + + #[test] + fn cli_loads_repo_from_flag() { + let cli = Cli::try_parse_from(["vk", "--repo", "foo/bar", "pr", "1"]).unwrap(); + assert_eq!(cli.global.repo.as_deref(), Some("foo/bar")); } use serial_test::serial; @@ -692,4 +762,18 @@ mod tests { let out = format_comment_diff(&comment).unwrap(); assert_eq!(out.lines().count(), 20); } + + #[test] + fn cli_requires_subcommand() { + assert!(Cli::try_parse_from(["vk"]).is_err()); + } + + #[test] + fn pr_subcommand_parses() { + let cli = Cli::try_parse_from(["vk", "pr", "123"]).unwrap(); + match cli.command { + Commands::Pr(args) => assert_eq!(args.reference, "123"), + _ => panic!("wrong variant"), + } + } } From 1924a2bede46238326d573e003d5d3a22cdc65a1 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 14 Jun 2025 20:29:26 +0100 Subject: [PATCH 2/4] Fix PR feedback and refine CLI --- Cargo.lock | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 16 +++--- README.md | 15 ++--- src/main.rs | 28 ++++----- 4 files changed, 191 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73b861bb..59ca933c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -130,6 +130,15 @@ version = "2.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + [[package]] name = "bumpalo" version = "3.18.1" @@ -255,6 +264,15 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" +[[package]] +name = "cpufeatures" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" +dependencies = [ + "libc", +] + [[package]] name = "crokey" version = "1.2.0" @@ -364,6 +382,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "crypto-common" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" +dependencies = [ + "generic-array", + "typenum", +] + [[package]] name = "derive_more" version = "2.0.1" @@ -394,6 +422,16 @@ dependencies = [ "nu-ansi-term", ] +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer", + "crypto-common", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -455,12 +493,24 @@ dependencies = [ "parking_lot", "pear", "serde", + "serde_yaml", "tempfile", "toml", "uncased", "version_check", ] +[[package]] +name = "figment-json5" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26f6982da09e166efe7dc3c5cf1fe01ef85419733eb188c0df0b571eda9e8a81" +dependencies = [ + "figment", + "json5", + "serde", +] + [[package]] name = "fnv" version = "1.0.7" @@ -568,6 +618,16 @@ dependencies = [ "slab", ] +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", +] + [[package]] name = "getrandom" version = "0.2.16" @@ -864,6 +924,17 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "json5" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96b0db21af676c1ce64250b5f40f3ce2cf27e4e47cb91ed91eb6fe9350b430c1" +dependencies = [ + "pest", + "pest_derive", + "serde", +] + [[package]] name = "lazy-regex" version = "3.4.1" @@ -1068,8 +1139,10 @@ dependencies = [ "clap", "clap-dispatch", "figment", + "figment-json5", "ortho_config_macros", "serde", + "serde_yaml", "thiserror 1.0.69", "toml", "uncased", @@ -1138,6 +1211,51 @@ version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" +[[package]] +name = "pest" +version = "2.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "198db74531d58c70a361c42201efde7e2591e976d518caf7662a47dc5720e7b6" +dependencies = [ + "memchr", + "thiserror 2.0.12", + "ucd-trie", +] + +[[package]] +name = "pest_derive" +version = "2.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d725d9cfd79e87dccc9341a2ef39d1b6f6353d68c4b33c177febbe1a402c97c5" +dependencies = [ + "pest", + "pest_generator", +] + +[[package]] +name = "pest_generator" +version = "2.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db7d01726be8ab66ab32f9df467ae8b1148906685bbe75c82d1e65d7f5b3f841" +dependencies = [ + "pest", + "pest_meta", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "pest_meta" +version = "2.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f9f832470494906d1fca5329f8ab5791cc60beb230c74815dff541cbd2b5ca0" +dependencies = [ + "once_cell", + "pest", + "sha2", +] + [[package]] name = "pin-project-lite" version = "0.2.16" @@ -1476,6 +1594,19 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_yaml" +version = "0.9.34+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" +dependencies = [ + "indexmap", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "serial_test" version = "3.2.0" @@ -1501,6 +1632,17 @@ dependencies = [ "syn", ] +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "shlex" version = "1.3.0" @@ -1840,6 +1982,18 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "typenum" +version = "1.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1dccffe3ce07af9386bfd29e80c0ab1a8205a2fc34e4bcd40364df902cfa8f3f" + +[[package]] +name = "ucd-trie" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971" + [[package]] name = "uncased" version = "0.9.10" @@ -1867,6 +2021,12 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" +[[package]] +name = "unsafe-libyaml" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" + [[package]] name = "untrusted" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index f86fbc01..41f515b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,17 +16,17 @@ url = "2.5.0" regex = "1.10.3" diffy = "0.4.2" anyhow = "1.0" -ortho_config = { git = "https://github.com/leynos/ortho-config", tag = "v0.2.0" } -figment = { version = "0.10", default-features = false, features = ["env", "toml"] } -xdg = "3" -uncased = "0.9" -toml = "0.8" +ortho_config = { git = "https://github.com/leynos/ortho-config", tag = "v0.2.0", default-features = false } +figment = { version = "0.10", default-features = false, features = ["env", "toml"], optional = true } +xdg = { version = "3", optional = true } +uncased = { version = "0.9", optional = true } +toml = { version = "0.8", optional = true } [features] default = ["toml"] -toml = [] -json5 = [] -yaml = [] +toml = ["dep:figment", "dep:xdg", "dep:uncased", "dep:toml", "ortho_config/toml"] +json5 = ["toml", "ortho_config/json5"] +yaml = ["toml", "ortho_config/yaml"] [dev-dependencies] tempfile = "3.20.0" diff --git a/README.md b/README.md index 8b6bda99..6709e548 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,14 @@ available subcommands are: * `pr` - show unresolved pull request comments (existing behaviour) * `issue` - read a GitHub issue (**to do**) -If you pass just a pull request number, `vk` tries to work out which repository -you meant. It first examines `.git/FETCH_HEAD` for a GitHub remote URL and, if -found, extracts the `owner/repo` from it. As Codex does not put the upstream URL in `.git/config`, we must obtain this from `FETCH_HEAD` for now. Failing that, it falls back to the -`VK_REPO` environment variable or `--repo` flag (both handled by -OrthoConfig) which should be set to `owner/repo` (with or without a `.git` -suffix). If neither source is available, `vk` will refuse to -run with only a number. +If you pass just a pull request number, `vk` tries to work out which +repository you meant. It first examines `.git/FETCH_HEAD` for a GitHub remote +URL and, if found, extracts the `owner/repo` from it. As Codex does not put the +upstream URL in `.git/config`, we must obtain this from `FETCH_HEAD` for now. +Failing that, it falls back to the `VK_REPO` environment variable or the +`--repo` flag (both handled by OrthoConfig). Set either one to `owner/repo` +(with or without a `.git` suffix). If neither source is available, `vk` will +refuse to run with only a number. `vk` uses the GitHub GraphQL API. Set `GITHUB_TOKEN` to authenticate. If it's not set you'll get a warning and anonymous requests may be rate limited. diff --git a/src/main.rs b/src/main.rs index f891031f..24223eaa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,13 +36,14 @@ struct Cli { #[command(subcommand)] command: crate::Commands, #[command(flatten)] - global: GlobalArgsCli, + global: GlobalArgs, } -#[derive(Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +#[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] #[ortho_config(prefix = "VK")] struct GlobalArgs { /// Repository used when passing only a pull request number + #[arg(long)] repo: Option, } @@ -50,14 +51,16 @@ struct GlobalArgs { #[ortho_config(prefix = "VK")] struct PrArgs { /// Pull request URL or number - reference: String, + #[arg(required = true)] + reference: Option, } #[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] #[ortho_config(prefix = "VK")] struct IssueArgs { /// Issue URL or number - reference: String, + #[arg(required = true)] + reference: Option, } #[derive(Debug, Clone)] @@ -463,7 +466,8 @@ fn build_headers(token: &str) -> HeaderMap { #[allow(clippy::result_large_err)] async fn run_pr(args: PrArgs, repo: Option<&str>) -> Result<(), VkError> { - let (repo, number) = parse_reference(&args.reference, repo)?; + let reference = args.reference.as_deref().ok_or(VkError::InvalidRef)?; + let (repo, number) = parse_reference(reference, repo)?; let token = env::var("GITHUB_TOKEN").unwrap_or_default(); if token.is_empty() { eprintln!("warning: GITHUB_TOKEN not set, using anonymous API access"); @@ -494,7 +498,7 @@ async fn run_pr(args: PrArgs, repo: Option<&str>) -> Result<(), VkError> { #[allow(clippy::result_large_err)] async fn main() -> Result<(), VkError> { let cli = Cli::parse(); - let mut global = GlobalArgs::load_from_iter(std::iter::empty::())?; + let mut global = GlobalArgs::load()?; if let Some(repo) = cli.global.repo { global.repo = Some(repo); } @@ -503,10 +507,7 @@ async fn main() -> Result<(), VkError> { let args = load_and_merge_subcommand_for::(&pr_cli)?; run_pr(args, global.repo.as_deref()).await } - Commands::Issue(issue_cli) => { - let _args = load_and_merge_subcommand_for::(&issue_cli)?; - Err(VkError::Unimplemented("issue command")) - } + Commands::Issue(_issue_cli) => Err(VkError::Unimplemented("issue command")), } } @@ -528,8 +529,9 @@ fn parse_reference(input: &str, default_repo: Option<&str>) -> Result<(RepoInfo, } Err(VkError::InvalidRef) } else if let Ok(number) = input.parse::() { - let repo = repo_from_fetch_head() - .or_else(|| default_repo.and_then(repo_from_str)) + let repo = default_repo + .and_then(repo_from_str) + .or_else(repo_from_fetch_head) .ok_or(VkError::RepoNotFound)?; Ok((repo, number)) } else { @@ -772,7 +774,7 @@ mod tests { fn pr_subcommand_parses() { let cli = Cli::try_parse_from(["vk", "pr", "123"]).unwrap(); match cli.command { - Commands::Pr(args) => assert_eq!(args.reference, "123"), + Commands::Pr(args) => assert_eq!(args.reference.as_deref(), Some("123")), _ => panic!("wrong variant"), } } From dd1d04b4dd0985d8ce3fc36a9940c2f9443a6827 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 14 Jun 2025 21:11:32 +0100 Subject: [PATCH 3/4] address review comments --- README.md | 16 ++++++++-------- src/main.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 6709e548..529137f2 100644 --- a/README.md +++ b/README.md @@ -11,9 +11,10 @@ vk pr ``` `vk` now uses [OrthoConfig](https://github.com/leynos/ortho-config) v0.2.0 for -configuration. A global `--repo` option (or `VK_REPO` environment variable) -sets the default repository when passing only a pull request number. The two -available subcommands are: +configuration. A global `--repo` option or the `VK_REPO` environment variable +sets the default repository when passing only a pull request number. + +The CLI provides two subcommands: * `pr` - show unresolved pull request comments (existing behaviour) * `issue` - read a GitHub issue (**to do**) @@ -22,10 +23,9 @@ If you pass just a pull request number, `vk` tries to work out which repository you meant. It first examines `.git/FETCH_HEAD` for a GitHub remote URL and, if found, extracts the `owner/repo` from it. As Codex does not put the upstream URL in `.git/config`, we must obtain this from `FETCH_HEAD` for now. -Failing that, it falls back to the `VK_REPO` environment variable or the -`--repo` flag (both handled by OrthoConfig). Set either one to `owner/repo` -(with or without a `.git` suffix). If neither source is available, `vk` will -refuse to run with only a number. +Failing that, it falls back to the configured repository (`--repo` or +`VK_REPO`). Set this value to `owner/repo` with or without a `.git` suffix. If +neither source is available, `vk` will refuse to run with only a number. `vk` uses the GitHub GraphQL API. Set `GITHUB_TOKEN` to authenticate. If it's not set you'll get a warning and anonymous requests may be rate limited. @@ -37,7 +37,7 @@ detailed guide to creating one. ## Example ``` -$ vk https://github.com/leynos/mxd/pull/31 +$ vk pr https://github.com/leynos/mxd/pull/31 ``` ## Troubleshooting diff --git a/src/main.rs b/src/main.rs index 24223eaa..abb6a4df 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,7 +52,7 @@ struct GlobalArgs { struct PrArgs { /// Pull request URL or number #[arg(required = true)] - reference: Option, + reference: String, } #[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] @@ -60,7 +60,7 @@ struct PrArgs { struct IssueArgs { /// Issue URL or number #[arg(required = true)] - reference: Option, + reference: String, } #[derive(Debug, Clone)] @@ -466,7 +466,7 @@ fn build_headers(token: &str) -> HeaderMap { #[allow(clippy::result_large_err)] async fn run_pr(args: PrArgs, repo: Option<&str>) -> Result<(), VkError> { - let reference = args.reference.as_deref().ok_or(VkError::InvalidRef)?; + let reference = &args.reference; let (repo, number) = parse_reference(reference, repo)?; let token = env::var("GITHUB_TOKEN").unwrap_or_default(); if token.is_empty() { @@ -774,7 +774,7 @@ mod tests { fn pr_subcommand_parses() { let cli = Cli::try_parse_from(["vk", "pr", "123"]).unwrap(); match cli.command { - Commands::Pr(args) => assert_eq!(args.reference.as_deref(), Some("123")), + Commands::Pr(args) => assert_eq!(args.reference, "123"), _ => panic!("wrong variant"), } } From e654e5c4e71f48fef0f1ff26279a4d07e5562ac6 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 14 Jun 2025 22:33:35 +0100 Subject: [PATCH 4/4] Apply review fixes --- README.md | 2 +- src/main.rs | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 529137f2..6166aa97 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ The CLI provides two subcommands: If you pass just a pull request number, `vk` tries to work out which repository you meant. It first examines `.git/FETCH_HEAD` for a GitHub remote -URL and, if found, extracts the `owner/repo` from it. As Codex does not put the +URL and, if found, extracts the `owner/repo` from it. As the Codex agent does not put the upstream URL in `.git/config`, we must obtain this from `FETCH_HEAD` for now. Failing that, it falls back to the configured repository (`--repo` or `VK_REPO`). Set this value to `owner/repo` with or without a `.git` suffix. If diff --git a/src/main.rs b/src/main.rs index abb6a4df..1a61dc69 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,12 +19,6 @@ enum Commands { Issue(IssueArgs), } -impl Default for Commands { - fn default() -> Self { - Commands::Pr(PrArgs::default()) - } -} - #[derive(Parser)] #[command( name = "vk", @@ -39,6 +33,7 @@ struct Cli { global: GlobalArgs, } +#[allow(non_snake_case)] #[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] #[ortho_config(prefix = "VK")] struct GlobalArgs { @@ -47,7 +42,16 @@ struct GlobalArgs { repo: Option, } -#[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +impl GlobalArgs { + fn merge(&mut self, other: GlobalArgs) { + if let Some(repo) = other.repo { + self.repo = Some(repo); + } + } +} + +#[allow(non_snake_case)] +#[derive(Parser, Deserialize, Serialize, Debug, OrthoConfig, Clone)] #[ortho_config(prefix = "VK")] struct PrArgs { /// Pull request URL or number @@ -55,7 +59,17 @@ struct PrArgs { reference: String, } -#[derive(Parser, Deserialize, Serialize, Default, Debug, OrthoConfig, Clone)] +impl Default for PrArgs { + #[allow(clippy::derivable_impls)] + fn default() -> Self { + Self { + reference: String::new(), + } + } +} + +#[allow(non_snake_case)] +#[derive(Parser, Deserialize, Serialize, Debug, OrthoConfig, Clone)] #[ortho_config(prefix = "VK")] struct IssueArgs { /// Issue URL or number @@ -63,6 +77,15 @@ struct IssueArgs { reference: String, } +impl Default for IssueArgs { + #[allow(clippy::derivable_impls)] + fn default() -> Self { + Self { + reference: String::new(), + } + } +} + #[derive(Debug, Clone)] struct RepoInfo { owner: String, @@ -499,9 +522,7 @@ async fn run_pr(args: PrArgs, repo: Option<&str>) -> Result<(), VkError> { async fn main() -> Result<(), VkError> { let cli = Cli::parse(); let mut global = GlobalArgs::load()?; - if let Some(repo) = cli.global.repo { - global.repo = Some(repo); - } + global.merge(cli.global); match cli.command { Commands::Pr(pr_cli) => { let args = load_and_merge_subcommand_for::(&pr_cli)?;