From 046850f1ffb7f1187353c8f5f5fffbbdb1f7854e Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 3 Mar 2026 17:01:29 -0500 Subject: [PATCH 1/5] Automated changes from cargo fix --edition --- src/cmd_user.rs | 18 ++++++++++++------ src/colors.rs | 36 ++++++++++++++++++++++++------------ src/config.rs | 2 +- src/context.rs | 35 ++++++++++++++++++++++++----------- 4 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/cmd_user.rs b/src/cmd_user.rs index f9dd78dd..cade143b 100644 --- a/src/cmd_user.rs +++ b/src/cmd_user.rs @@ -60,8 +60,10 @@ mod test { let test_host = std::env::var("ZOO_TEST_HOST").unwrap_or_default(); let test_token = std::env::var("ZOO_TEST_TOKEN").expect("ZOO_TEST_TOKEN is required"); - std::env::set_var("ZOO_HOST", test_host); - std::env::set_var("ZOO_API_TOKEN", test_token); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_HOST", test_host) }; + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_API_TOKEN", test_token) }; orig } @@ -69,15 +71,19 @@ mod test { async fn teardown(self) { // Put the original env var back. if let Ok(ref val) = self.orig_zoo_host { - std::env::set_var("ZOO_HOST", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_HOST", val) }; } else { - std::env::remove_var("ZOO_HOST"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_HOST") }; } if let Ok(ref val) = self.orig_zoo_token { - std::env::set_var("ZOO_API_TOKEN", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_API_TOKEN", val) }; } else { - std::env::remove_var("ZOO_API_TOKEN"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_API_TOKEN") }; } } } diff --git a/src/colors.rs b/src/colors.rs index d310df3a..2541da99 100644 --- a/src/colors.rs +++ b/src/colors.rs @@ -168,21 +168,27 @@ mod test { fn teardown(self) { // Put the original env var back. if let Ok(ref val) = self.orig_no_color_env { - std::env::set_var("NO_COLOR", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("NO_COLOR", val) }; } else { - std::env::remove_var("NO_COLOR"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("NO_COLOR") }; } if let Ok(ref val) = self.orig_clicolor_env { - std::env::set_var("CLICOLOR", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR", val) }; } else { - std::env::remove_var("CLICOLOR"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("CLICOLOR") }; } if let Ok(ref val) = self.orig_clicolor_force_env { - std::env::set_var("CLICOLOR_FORCE", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR_FORCE", val) }; } else { - std::env::remove_var("CLICOLOR_FORCE"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("CLICOLOR_FORCE") }; } } } @@ -238,9 +244,12 @@ mod test { ]; for t in tests { - std::env::set_var("NO_COLOR", t.no_color_env); - std::env::set_var("CLICOLOR", t.clicolor_env); - std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("NO_COLOR", t.no_color_env) }; + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR", t.clicolor_env) }; + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env) }; let got = env_color_disabled(); assert_eq!(got, t.want, "test {}", t.name); @@ -296,9 +305,12 @@ mod test { ]; for t in tests { - std::env::set_var("NO_COLOR", t.no_color_env); - std::env::set_var("CLICOLOR", t.clicolor_env); - std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("NO_COLOR", t.no_color_env) }; + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR", t.clicolor_env) }; + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env) }; let got = env_color_forced(); diff --git a/src/config.rs b/src/config.rs index fca02794..3638ecae 100644 --- a/src/config.rs +++ b/src/config.rs @@ -157,7 +157,7 @@ pub fn validate_value(target_key: &str, value: &str) -> Result<()> { // new_from_string initializes a Config from a toml string. #[cfg(test)] -pub fn new_from_string(s: &str) -> Result { +pub fn new_from_string(s: &str) -> Result> { let root = s.parse::()?; Ok(new_config(root)) } diff --git a/src/context.rs b/src/context.rs index 33f98307..0b51f31c 100644 --- a/src/context.rs +++ b/src/context.rs @@ -64,9 +64,14 @@ impl Context<'_> { // 3. PAGER if let Ok(zoo_pager) = std::env::var("ZOO_PAGER") { io.set_pager(zoo_pager); - } else if let Ok(pager) = config.get("", "pager") { - if !pager.is_empty() { - io.set_pager(pager); + } else { + match config.get("", "pager") { + Ok(pager) => { + if !pager.is_empty() { + io.set_pager(pager); + } + } + _ => {} } } @@ -1054,15 +1059,19 @@ mod test { fn teardown(self) { // Put the original env var back. if let Ok(ref val) = self.orig_zoo_pager_env { - std::env::set_var("ZOO_PAGER", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_PAGER", val) }; } else { - std::env::remove_var("ZOO_PAGER"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_PAGER") }; } if let Ok(ref val) = self.orig_zoo_force_tty_env { - std::env::set_var("ZOO_FORCE_TTY", val); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_FORCE_TTY", val) }; } else { - std::env::remove_var("ZOO_FORCE_TTY"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_FORCE_TTY") }; } } } @@ -1148,15 +1157,19 @@ mod test { } if !t.zoo_pager_env.is_empty() { - std::env::set_var("ZOO_PAGER", t.zoo_pager_env.clone()); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_PAGER", t.zoo_pager_env.clone()) }; } else { - std::env::remove_var("ZOO_PAGER"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_PAGER") }; } if !t.zoo_force_tty_env.is_empty() { - std::env::set_var("ZOO_FORCE_TTY", t.zoo_force_tty_env.clone()); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::set_var("ZOO_FORCE_TTY", t.zoo_force_tty_env.clone()) }; } else { - std::env::remove_var("ZOO_FORCE_TTY"); + // TODO: Audit that the environment access only happens in single-threaded code. + unsafe { std::env::remove_var("ZOO_FORCE_TTY") }; } let ctx = Context::new(&mut c); From a7527b7d3edc2426562d9e1437d4ecab3d8fad07 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 15:04:09 -0400 Subject: [PATCH 2/5] Remove environment variable mutation --- src/cmd_user.rs | 58 ++---------- src/colors.rs | 89 +++++------------- src/context.rs | 208 ++++++++++++++++++++++++++++++------------- tests/kcl_process.rs | 123 +++++++++++++++++++++++++ 4 files changed, 298 insertions(+), 180 deletions(-) create mode 100644 tests/kcl_process.rs diff --git a/src/cmd_user.rs b/src/cmd_user.rs index cade143b..90782278 100644 --- a/src/cmd_user.rs +++ b/src/cmd_user.rs @@ -32,7 +32,6 @@ impl crate::cmd::Command for CmdUser { #[cfg(test)] mod test { use pretty_assertions::assert_eq; - use test_context::{test_context, AsyncTestContext}; use crate::cmd::Command; @@ -44,54 +43,14 @@ mod test { want_err: String, } - struct TContext { - orig_zoo_host: Result, - orig_zoo_token: Result, - } - - impl AsyncTestContext for TContext { - async fn setup() -> TContext { - let orig = TContext { - orig_zoo_host: std::env::var("ZOO_HOST"), - orig_zoo_token: std::env::var("ZOO_API_TOKEN"), - }; - - // Set our test values. - let test_host = std::env::var("ZOO_TEST_HOST").unwrap_or_default(); - - let test_token = std::env::var("ZOO_TEST_TOKEN").expect("ZOO_TEST_TOKEN is required"); - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_HOST", test_host) }; - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_API_TOKEN", test_token) }; - - orig - } - - async fn teardown(self) { - // Put the original env var back. - if let Ok(ref val) = self.orig_zoo_host { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_HOST", val) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_HOST") }; - } - - if let Ok(ref val) = self.orig_zoo_token { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_API_TOKEN", val) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_API_TOKEN") }; - } - } - } - - #[test_context(TContext)] + /// Same-process unit test for paths that do not require dependencies to + /// read real process env. + /// + /// Tests that need `kcl_lib` or other dependencies to read `ZOO_API_TOKEN` + /// must use the child-process integration test harness in + /// [`tests/kcl_process.rs`](../../tests/kcl_process.rs). #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[serial_test::serial] - async fn test_cmd_user(_ctx: &mut TContext) { + async fn test_cmd_user() { let tests: Vec = vec![TestItem { name: "volume: input file does not exist".to_string(), cmd: crate::cmd_user::SubCommand::Edit(crate::cmd_user::CmdUserEdit { @@ -111,7 +70,6 @@ mod test { }]; let mut config = crate::config::new_blank_config().unwrap(); - let mut c = crate::config_from_env::EnvConfig::inherit_env(&mut config); for t in tests { let (mut io, stdout_path, stderr_path) = crate::iostreams::IoStreams::test(); @@ -123,7 +81,7 @@ mod test { io.set_color_enabled(false); io.set_never_prompt(true); let mut ctx = crate::context::Context { - config: &mut c, + config: &mut config, io, debug: false, override_host: None, diff --git a/src/colors.rs b/src/colors.rs index 2541da99..051414ea 100644 --- a/src/colors.rs +++ b/src/colors.rs @@ -1,10 +1,18 @@ use crate::config_file::get_env_var; pub fn env_color_disabled() -> bool { - !get_env_var("NO_COLOR").is_empty() || get_env_var("CLICOLOR") == "0" + env_color_disabled_with(get_env_var) } pub fn env_color_forced() -> bool { + env_color_forced_with(get_env_var) +} + +fn env_color_disabled_with(get_env_var: impl Fn(&str) -> String) -> bool { + !get_env_var("NO_COLOR").is_empty() || get_env_var("CLICOLOR") == "0" +} + +fn env_color_forced_with(get_env_var: impl Fn(&str) -> String) -> bool { !get_env_var("CLICOLOR_FORCE").is_empty() && get_env_var("CLICOLOR_FORCE") != "0" } @@ -146,53 +154,9 @@ impl ColorScheme { #[cfg(test)] mod test { use pretty_assertions::assert_eq; - use test_context::{test_context, TestContext}; use super::*; - struct Context { - orig_no_color_env: Result, - orig_clicolor_env: Result, - orig_clicolor_force_env: Result, - } - - impl TestContext for Context { - fn setup() -> Context { - Context { - orig_no_color_env: std::env::var("NO_COLOR"), - orig_clicolor_env: std::env::var("CLICOLOR"), - orig_clicolor_force_env: std::env::var("CLICOLOR_FORCE"), - } - } - - fn teardown(self) { - // Put the original env var back. - if let Ok(ref val) = self.orig_no_color_env { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("NO_COLOR", val) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("NO_COLOR") }; - } - - if let Ok(ref val) = self.orig_clicolor_env { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR", val) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("CLICOLOR") }; - } - - if let Ok(ref val) = self.orig_clicolor_force_env { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR_FORCE", val) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("CLICOLOR_FORCE") }; - } - } - } - pub struct TestItem { name: String, no_color_env: String, @@ -201,10 +165,8 @@ mod test { want: bool, } - #[test_context(Context)] #[test] - #[serial_test::serial] - fn test_env_color_disabled(_ctx: &mut Context) { + fn test_env_color_disabled() { let tests = vec![ TestItem { name: "pristine env".to_string(), @@ -244,21 +206,18 @@ mod test { ]; for t in tests { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("NO_COLOR", t.no_color_env) }; - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR", t.clicolor_env) }; - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env) }; - - let got = env_color_disabled(); + let got = env_color_disabled_with(|key| match key { + "NO_COLOR" => t.no_color_env.clone(), + "CLICOLOR" => t.clicolor_env.clone(), + "CLICOLOR_FORCE" => t.clicolor_force_env.clone(), + _ => String::new(), + }); assert_eq!(got, t.want, "test {}", t.name); } } - #[test_context(Context)] #[test] - fn test_env_color_forced(_ctx: &mut Context) { + fn test_env_color_forced() { let tests = vec![ TestItem { name: "pristine env".to_string(), @@ -305,14 +264,12 @@ mod test { ]; for t in tests { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("NO_COLOR", t.no_color_env) }; - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR", t.clicolor_env) }; - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("CLICOLOR_FORCE", t.clicolor_force_env) }; - - let got = env_color_forced(); + let got = env_color_forced_with(|key| match key { + "NO_COLOR" => t.no_color_env.clone(), + "CLICOLOR" => t.clicolor_env.clone(), + "CLICOLOR_FORCE" => t.clicolor_force_env.clone(), + _ => String::new(), + }); assert_eq!(got, t.want, "test {}", t.name); } diff --git a/src/context.rs b/src/context.rs index 0b51f31c..0e01702f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -18,7 +18,7 @@ pub struct Context<'a> { pub(crate) override_host: Option, } -impl Context<'_> { +impl<'a> Context<'a> { fn resolve_api_host_and_baseurl(&self, hostname: &str) -> Result<(String, String)> { let host = if !hostname.is_empty() { hostname.to_string() @@ -47,10 +47,18 @@ impl Context<'_> { .connect_timeout(std::time::Duration::from_secs(60)) } - pub fn new(config: &mut (dyn Config + Send + Sync)) -> Context<'_> { + pub fn new(config: &'a mut (dyn Config + Send + Sync)) -> Context<'a> { // Let's get our IO streams. - let mut io = crate::iostreams::IoStreams::system(); + let io = crate::iostreams::IoStreams::system(); + + Context::new_with_io_and_env(config, io, |key| std::env::var(key)) + } + fn new_with_io_and_env( + config: &'a mut (dyn Config + Send + Sync), + mut io: crate::iostreams::IoStreams, + get_env_var: impl Fn(&str) -> std::result::Result, + ) -> Context<'a> { // Set the prompt. let prompt = config.get("", "prompt").unwrap(); if prompt == "disabled" { @@ -62,21 +70,18 @@ impl Context<'_> { // 1. ZOO_PAGER // 2. pager from config // 3. PAGER - if let Ok(zoo_pager) = std::env::var("ZOO_PAGER") { + if let Ok(zoo_pager) = get_env_var("ZOO_PAGER") { io.set_pager(zoo_pager); } else { - match config.get("", "pager") { - Ok(pager) => { - if !pager.is_empty() { - io.set_pager(pager); - } + if let Ok(pager) = config.get("", "pager") { + if !pager.is_empty() { + io.set_pager(pager); } - _ => {} } } // Check if we should force use the tty. - if let Ok(zoo_force_tty) = std::env::var("ZOO_FORCE_TTY") { + if let Ok(zoo_force_tty) = get_env_var("ZOO_FORCE_TTY") { if !zoo_force_tty.is_empty() { io.force_terminal(&zoo_force_tty); } @@ -1038,59 +1043,132 @@ fn format_copilot_error(detail: &str, use_color: bool) -> String { #[cfg(test)] mod test { + use std::{collections::HashMap, sync::Arc}; + use pretty_assertions::assert_eq; - use test_context::{test_context, TestContext}; use super::*; - struct TContext { - orig_zoo_pager_env: Result, - orig_zoo_force_tty_env: Result, + pub struct TestItem { + name: String, + zoo_pager_env: String, + zoo_force_tty_env: String, + pager: String, + prompt: String, + want_pager: String, + want_prompt: String, + want_terminal_width_override: i32, + } + + struct TestEnvConfig<'a> { + config: &'a mut (dyn crate::config::Config + 'a), + env: Arc>, } - impl TestContext for TContext { - fn setup() -> TContext { - TContext { - orig_zoo_pager_env: std::env::var("ZOO_PAGER"), - orig_zoo_force_tty_env: std::env::var("ZOO_FORCE_TTY"), - } + impl TestEnvConfig<'_> { + fn get_env_var(&self, key: &str) -> String { + self.env.get(key).cloned().unwrap_or_default() + } + } + + impl crate::config::Config for TestEnvConfig<'_> { + fn get(&self, hostname: &str, key: &str) -> Result { + let (val, _) = self.get_with_source(hostname, key)?; + Ok(val) } - fn teardown(self) { - // Put the original env var back. - if let Ok(ref val) = self.orig_zoo_pager_env { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_PAGER", val) }; + fn get_with_source(&self, hostname: &str, key: &str) -> Result<(String, String)> { + if key == "token" { + let token = self.get_env_var("ZOO_API_TOKEN"); + let token = if token.is_empty() { + self.get_env_var("ZOO_TOKEN") + } else { + token + }; + if !token.is_empty() { + return Ok((token, "ZOO_API_TOKEN".to_string())); + } } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_PAGER") }; + let var = format!("ZOO_{}", heck::AsShoutySnakeCase(key)); + let val = self.get_env_var(&var); + if !val.is_empty() { + return Ok((val, var)); + } } - if let Ok(ref val) = self.orig_zoo_force_tty_env { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_FORCE_TTY", val) }; + self.config.get_with_source(hostname, key) + } + + fn set(&mut self, hostname: &str, key: &str, value: Option<&str>) -> Result<()> { + self.config.set(hostname, key, value) + } + + fn unset_host(&mut self, key: &str) -> Result<()> { + self.config.unset_host(key) + } + + fn hosts(&self) -> Result> { + self.config.hosts() + } + + fn default_host(&self) -> Result { + let (host, _) = self.default_host_with_source()?; + Ok(host) + } + + fn default_host_with_source(&self) -> Result<(String, String)> { + if let Some(host) = self.env.get("ZOO_HOST") { + Ok((host.clone(), "ZOO_HOST".to_string())) } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_FORCE_TTY") }; + self.config.default_host_with_source() } } - } - pub struct TestItem { - name: String, - zoo_pager_env: String, - zoo_force_tty_env: String, - pager: String, - prompt: String, - want_pager: String, - want_prompt: String, - want_terminal_width_override: i32, + fn aliases(&mut self) -> Result> { + self.config.aliases() + } + + fn save_aliases(&mut self, aliases: &crate::config_map::ConfigMap) -> Result<()> { + self.config.save_aliases(aliases) + } + + fn expand_alias(&mut self, args: Vec) -> Result<(Vec, bool)> { + self.config.expand_alias(args) + } + + fn check_writable(&self, hostname: &str, key: &str) -> Result<()> { + if key == "token" { + let token = self.get_env_var("ZOO_API_TOKEN"); + let token = if token.is_empty() { + self.get_env_var("ZOO_TOKEN") + } else { + token + }; + if !token.is_empty() { + return Err( + crate::config_from_env::ReadOnlyEnvVarError::Variable("ZOO_API_TOKEN".to_string()).into(), + ); + } + } + + self.config.check_writable(hostname, key) + } + + fn write(&self) -> Result<()> { + self.config.write() + } + + fn config_to_string(&self) -> Result { + self.config.config_to_string() + } + + fn hosts_to_string(&self) -> Result { + self.config.hosts_to_string() + } } - #[test_context(TContext)] #[test] - #[serial_test::serial] - fn test_context(_ctx: &mut TContext) { + fn test_context() { let tests = vec![ TestItem { name: "ZOO_PAGER env".to_string(), @@ -1146,33 +1224,35 @@ mod test { for t in tests { let mut config = crate::config::new_blank_config().unwrap(); - let mut c = crate::config_from_env::EnvConfig::inherit_env(&mut config); + let mut env = HashMap::new(); - if !t.pager.is_empty() { - c.set("", "pager", Some(&t.pager)).unwrap(); + if !t.zoo_pager_env.is_empty() { + env.insert("ZOO_PAGER".to_string(), t.zoo_pager_env.clone()); } - if !t.prompt.is_empty() { - c.set("", "prompt", Some(&t.prompt)).unwrap(); + if !t.zoo_force_tty_env.is_empty() { + env.insert("ZOO_FORCE_TTY".to_string(), t.zoo_force_tty_env.clone()); } - if !t.zoo_pager_env.is_empty() { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_PAGER", t.zoo_pager_env.clone()) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_PAGER") }; + let env = Arc::new(env); + let context_env = Arc::clone(&env); + let mut c = TestEnvConfig { + config: &mut config, + env, + }; + + if !t.pager.is_empty() { + c.set("", "pager", Some(&t.pager)).unwrap(); } - if !t.zoo_force_tty_env.is_empty() { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::set_var("ZOO_FORCE_TTY", t.zoo_force_tty_env.clone()) }; - } else { - // TODO: Audit that the environment access only happens in single-threaded code. - unsafe { std::env::remove_var("ZOO_FORCE_TTY") }; + if !t.prompt.is_empty() { + c.set("", "prompt", Some(&t.prompt)).unwrap(); } - let ctx = Context::new(&mut c); + let (io, _stdout_path, _stderr_path) = crate::iostreams::IoStreams::test(); + let ctx = Context::new_with_io_and_env(&mut c, io, move |key| { + context_env.get(key).cloned().ok_or(std::env::VarError::NotPresent) + }); assert_eq!(ctx.io.get_pager(), t.want_pager, "test: {}", t.name); diff --git a/tests/kcl_process.rs b/tests/kcl_process.rs new file mode 100644 index 00000000..f7c4ea43 --- /dev/null +++ b/tests/kcl_process.rs @@ -0,0 +1,123 @@ +//! Child-process integration tests for KCL commands that need real process +//! environment variables. +//! +//! These tests execute the compiled `zoo` binary instead of calling `do_main` +//! in-process. This is intentional: some KCL paths depend on `kcl_lib`, and +//! `kcl_lib` reads authentication details such as `ZOO_API_TOKEN` directly from +//! the process environment. In Rust 2024, mutating the parent test process +//! environment with `std::env::set_var` or `std::env::remove_var` is not a safe +//! way to test that behavior on every platform. Passing environment variables to +//! a child process with `Command::env` gives each test its own real environment +//! without mutating global state in the test process. +//! +//! Use this harness for tests that must verify behavior of `kcl snapshot`, +//! `kcl export`, `kcl analyze`, or other commands where dependencies need to +//! observe real environment variables. Use ordinary same-process unit tests for +//! pure CLI/config behavior that can be tested with injected values or in-memory +//! config. + +use std::{ + path::{Path, PathBuf}, + process::{Command, Output}, +}; + +const ANALYZE_CUBE_KCL: &str = r#"@settings(kclVersion = 2.0) + +rectangleSketch = sketch(on = XY) { + line1 = line(start = [var 2.47mm, var 2.96mm], end = [var 3.47mm, var 2.96mm]) + line2 = line(start = [var 3.47mm, var 2.96mm], end = [var 3.47mm, var 3.96mm]) + line3 = line(start = [var 3.47mm, var 3.96mm], end = [var 2.47mm, var 3.96mm]) + line4 = line(start = [var 2.47mm, var 3.96mm], end = [var 2.47mm, var 2.96mm]) + coincident([line1.end, line2.start]) + coincident([line2.end, line3.start]) + coincident([line3.end, line4.start]) + coincident([line4.end, line1.start]) + parallel([line2, line4]) + parallel([line3, line1]) + perpendicular([line1, line2]) + horizontal(line3) + distance([line4.end, line2.start]) == 1mm + distance([line2.start, line3.start]) == 1mm +} +hidden001 = hide(rectangleSketch) +region001 = region(segments = [ + rectangleSketch.line4, + rectangleSketch.line1 +]) +cube1 = extrude(region001, length = 2mm) +"#; + +fn zoo_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_zoo")) +} + +fn apply_test_auth_env(cmd: &mut Command) { + match std::env::var("ZOO_TEST_TOKEN") { + Ok(token) => { + cmd.env("ZOO_API_TOKEN", token); + } + Err(_) => { + eprintln!("WARNING: ZOO_TEST_TOKEN is not set; kcl child-process test may fail"); + } + } + + if let Ok(host) = std::env::var("ZOO_TEST_HOST") { + if !host.is_empty() { + cmd.env("ZOO_HOST", host); + } + } +} + +fn run_zoo(args: &[&str], current_dir: &Path, config_dir: &Path) -> Output { + let mut cmd = Command::new(zoo_bin()); + cmd.args(args) + .current_dir(current_dir) + .env("ZOO_CONFIG_DIR", config_dir); + apply_test_auth_env(&mut cmd); + cmd.output() + .unwrap_or_else(|err| panic!("failed to run zoo with args {args:?}: {err}")) +} + +fn assert_success(output: &Output, args: &[&str]) { + if !output.status.success() { + panic!( + "zoo {args:?} failed\nstatus: {}\nstdout:\n{}\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } +} + +#[test] +fn kcl_analyze_child_process_returns_expected_json_sections() { + let project_dir = tempfile::tempdir().expect("create project temp dir"); + let config_dir = tempfile::tempdir().expect("create config temp dir"); + std::fs::write(project_dir.path().join("main.kcl"), ANALYZE_CUBE_KCL).expect("write main.kcl"); + + let args = ["kcl", "analyze", "main.kcl", "--format", "json"]; + let output = run_zoo(&args, project_dir.path(), config_dir.path()); + assert_success(&output, &args); + + let stdout = String::from_utf8_lossy(&output.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap_or_else(|err| { + panic!( + "zoo {args:?} did not write valid JSON\nerror: {err}\nstdout:\n{stdout}\nstderr:\n{}", + String::from_utf8_lossy(&output.stderr) + ) + }); + + for key in [ + "volume", + "mass", + "density", + "surface_area", + "center_of_mass", + "bounding_box", + ] { + assert!( + json.get(key).is_some(), + "zoo {args:?} JSON output missing `{key}`\nstdout:\n{stdout}" + ); + } +} From 17cb15bb646292dc82646e65673b1dcf909935e2 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 15:14:59 -0400 Subject: [PATCH 3/5] Change to Rust 2024 edition --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4cafa649..1bbb8d03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "zoo" version = "0.2.180" -edition = "2021" +edition = "2024" rust-version = "1.95" build = "build.rs" From 0f75622e155ef74f9be1654e18beba4825506854 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 15:17:56 -0400 Subject: [PATCH 4/5] Update rustfmt to 2024 --- rustfmt.toml | 2 +- src/cmd_alias.rs | 2 +- src/cmd_api.rs | 2 +- src/cmd_api_call.rs | 4 +++- src/cmd_auth.rs | 2 +- src/cmd_completion.rs | 2 +- src/cmd_config.rs | 4 ++-- src/cmd_file.rs | 18 +++++++++++------- src/cmd_kcl.rs | 14 ++++++++------ src/cmd_ml/cmd_text_to_cad.rs | 18 ++++++++++-------- src/config.rs | 10 ++++++---- src/config_file.rs | 2 +- src/config_from_file.rs | 2 +- src/config_map.rs | 2 +- src/context.rs | 6 +++--- src/iostreams.rs | 4 ++-- src/ml/copilot/run.rs | 6 +++--- src/ml/copilot/state.rs | 6 +----- src/ml/copilot/ui.rs | 2 +- src/tests.rs | 2 +- src/update.rs | 2 +- tests/win_ca_smoke.rs | 5 +++-- 22 files changed, 63 insertions(+), 54 deletions(-) diff --git a/rustfmt.toml b/rustfmt.toml index 75b77657..862b9f06 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -1,5 +1,5 @@ max_width = 120 -edition = "2018" +edition = "2024" format_code_in_doc_comments = true format_strings = false imports_granularity = "Crate" diff --git a/src/cmd_alias.rs b/src/cmd_alias.rs index c505661e..f65977cd 100644 --- a/src/cmd_alias.rs +++ b/src/cmd_alias.rs @@ -1,6 +1,6 @@ use std::io::Write; -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use clap::{Command, CommandFactory, Parser}; /// Create command shortcuts. diff --git a/src/cmd_api.rs b/src/cmd_api.rs index 1fbae1e1..4183098a 100644 --- a/src/cmd_api.rs +++ b/src/cmd_api.rs @@ -4,7 +4,7 @@ use std::{ io::{Read, Write}, }; -use anyhow::{anyhow, Result}; +use anyhow::{Result, anyhow}; use clap::Parser; use reqwest::header::CONTENT_TYPE; use serde::{Deserialize, Serialize}; diff --git a/src/cmd_api_call.rs b/src/cmd_api_call.rs index 18a23941..c1b7e60f 100644 --- a/src/cmd_api_call.rs +++ b/src/cmd_api_call.rs @@ -73,7 +73,9 @@ impl crate::cmd::Command for CmdApiCallStatus { let path = std::env::current_dir()?; for (name, output) in outputs { if output.is_empty() { - anyhow::bail!("no output was generated for the file conversion! (this is probably a bug in the API) you should report it to support@zoo.dev"); + anyhow::bail!( + "no output was generated for the file conversion! (this is probably a bug in the API) you should report it to support@zoo.dev" + ); } let path = path.join(name); std::fs::write(&path, &output.0)?; diff --git a/src/cmd_auth.rs b/src/cmd_auth.rs index 6e30f0cc..2cd0f219 100644 --- a/src/cmd_auth.rs +++ b/src/cmd_auth.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use anyhow::{anyhow, Result}; +use anyhow::{Result, anyhow}; use clap::Parser; use oauth2::TokenResponse; diff --git a/src/cmd_completion.rs b/src/cmd_completion.rs index 75c3ee31..3742cad4 100644 --- a/src/cmd_completion.rs +++ b/src/cmd_completion.rs @@ -1,6 +1,6 @@ use anyhow::Result; use clap::{Command, CommandFactory, Parser}; -use clap_complete::{generate, Shell}; +use clap_complete::{Shell, generate}; /// Generate shell completion scripts. /// diff --git a/src/cmd_config.rs b/src/cmd_config.rs index 2d3f6ada..d6967b22 100644 --- a/src/cmd_config.rs +++ b/src/cmd_config.rs @@ -1,7 +1,7 @@ -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use clap::Parser; -use crate::config::{ConfigOption, CONFIG_OPTIONS}; +use crate::config::{CONFIG_OPTIONS, ConfigOption}; // TODO: make this doc a function that parses from the config the options so it's not hardcoded /// Manage configuration for `zoo`. diff --git a/src/cmd_file.rs b/src/cmd_file.rs index e7cd0223..340437dd 100644 --- a/src/cmd_file.rs +++ b/src/cmd_file.rs @@ -7,8 +7,8 @@ use anyhow::Result; use base64::prelude::*; use clap::Parser; use kittycad_modeling_cmds::{ - self as kcmc, ok_response::OkModelingCmdResponse, shared::FileImportFormat, websocket::OkWebSocketResponseData, - ModelingCmd, + self as kcmc, ModelingCmd, ok_response::OkModelingCmdResponse, shared::FileImportFormat, + websocket::OkWebSocketResponseData, }; use crate::cmd_kcl::write_deterministic_export; @@ -145,7 +145,9 @@ impl crate::cmd::Command for CmdFileConvert { )?; } } else { - anyhow::bail!("no output was generated! (this is probably a bug in the API) you should report it to support@zoo.dev"); + anyhow::bail!( + "no output was generated! (this is probably a bug in the API) you should report it to support@zoo.dev" + ); } } @@ -233,10 +235,12 @@ impl crate::cmd::Command for CmdFileSnapshot { // In order for the program to know it's dealing with this type, an // attempt to parse as json is made, then we check for the buffers // property which describes what external files are needed. - let mut files: Vec = vec![kcmc::ImportFile::builder() - .path(filename.to_string()) - .data(input.clone()) - .build()]; + let mut files: Vec = vec![ + kcmc::ImportFile::builder() + .path(filename.to_string()) + .data(input.clone()) + .build(), + ]; if matches!(src_format, kcmc::format::InputFormat3d::Gltf(_)) { if let Ok(str) = std::str::from_utf8(&input) { diff --git a/src/cmd_kcl.rs b/src/cmd_kcl.rs index 70692231..9f34e2e4 100644 --- a/src/cmd_kcl.rs +++ b/src/cmd_kcl.rs @@ -3,7 +3,7 @@ use std::{net::SocketAddr, path::Path, str::FromStr}; use anyhow::Result; use clap::Parser; use image::{DynamicImage, ImageReader}; -use kcl_lib::{unit_conversion::ToKcmc, ToLspRange, TypedPath}; +use kcl_lib::{ToLspRange, TypedPath, unit_conversion::ToKcmc}; use kcmc::{format::OutputFormat3d as OutputFormat, ok_response::OkModelingCmdResponse}; use kittycad::types as kt; use kittycad_modeling_cmds::{self as kcmc, units::UnitLength, websocket::ModelingSessionData}; @@ -765,8 +765,8 @@ pub fn get_image_format_from_extension_kcmc(ext: &str) -> Result Ok(format), Err(_) => { anyhow::bail!( - "unknown source format for file extension: {ext}. Try setting the `--src-format` flag explicitly or use a valid format." - ) + "unknown source format for file extension: {ext}. Try setting the `--src-format` flag explicitly or use a valid format." + ) } } } @@ -777,8 +777,8 @@ pub fn get_image_format_from_extension_dot_rs(ext: &str) -> Result Ok(format), Err(_) => { anyhow::bail!( - "unknown source format for file extension: {ext}. Try setting the `--src-format` flag explicitly or use a valid format." - ) + "unknown source format for file extension: {ext}. Try setting the `--src-format` flag explicitly or use a valid format." + ) } } } @@ -1757,7 +1757,9 @@ fn print_trace_link(io: &mut IoStreams, session_data: &Option default true`. Options for hosts are: example.org, thing.com"); + assert_eq!( + e.to_string(), + "Multiple hosts in config file but none has been set as a default. Try setting a default with `zoo config set -H default true`. Options for hosts are: example.org, thing.com" + ); } c.set("example.org", "default", Some("true")).unwrap(); diff --git a/src/config_file.rs b/src/config_file.rs index 7981051b..e66f9321 100644 --- a/src/config_file.rs +++ b/src/config_file.rs @@ -4,7 +4,7 @@ use std::{ path::{Path, PathBuf}, }; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result, anyhow}; const ZOO_CONFIG_DIR: &str = "ZOO_CONFIG_DIR"; const XDG_CONFIG_HOME: &str = "XDG_CONFIG_HOME"; diff --git a/src/config_from_file.rs b/src/config_from_file.rs index d7378533..56c12ea0 100644 --- a/src/config_from_file.rs +++ b/src/config_from_file.rs @@ -1,4 +1,4 @@ -use anyhow::{anyhow, Result}; +use anyhow::{Result, anyhow}; use crate::config_alias::AliasConfig; diff --git a/src/config_map.rs b/src/config_map.rs index 260a9ba5..25712ae2 100644 --- a/src/config_map.rs +++ b/src/config_map.rs @@ -1,4 +1,4 @@ -use anyhow::{anyhow, Result}; +use anyhow::{Result, anyhow}; // ConfigMap implements a low-level get/set config that is backed by an in-memory tree of toml // nodes. It allows us to interact with a toml-based config programmatically, preserving any diff --git a/src/context.rs b/src/context.rs index 0e01702f..ca969dc4 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,12 +1,12 @@ use std::{io::Write, str::FromStr, time::Duration}; -use anyhow::{anyhow, Result}; +use anyhow::{Result, anyhow}; use futures::StreamExt; use kcl_lib::engine_connection::EngineManager; use kcmc::{each_cmd as mcmd, websocket::OkWebSocketResponseData}; use kittycad::types::{ApiCallStatus, AsyncApiCallOutput, TextToCad, TextToCadCreateBody, TextToCadMultiFileIteration}; -use kittycad_modeling_cmds::{self as kcmc, output::TakeSnapshot, websocket::ModelingSessionData, ModelingCmd}; -use tokio_tungstenite::{tungstenite::protocol::Role, WebSocketStream}; +use kittycad_modeling_cmds::{self as kcmc, ModelingCmd, output::TakeSnapshot, websocket::ModelingSessionData}; +use tokio_tungstenite::{WebSocketStream, tungstenite::protocol::Role}; use crate::{cmd_kcl, config::Config, config_file::get_env_var, kcl_error_fmt, types::FormatOutput}; diff --git a/src/iostreams.rs b/src/iostreams.rs index 146a702f..b4d4ad46 100644 --- a/src/iostreams.rs +++ b/src/iostreams.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, env, io::IsTerminal, process::Command}; -use anyhow::{anyhow, Result}; -use terminal_size::{terminal_size, Height, Width}; +use anyhow::{Result, anyhow}; +use terminal_size::{Height, Width, terminal_size}; use crate::config_file::get_env_var; diff --git a/src/ml/copilot/run.rs b/src/ml/copilot/run.rs index cb6e9876..d900ab1c 100644 --- a/src/ml/copilot/run.rs +++ b/src/ml/copilot/run.rs @@ -8,17 +8,17 @@ use crossterm::{ cursor::MoveTo, event::{Event, EventStream}, execute, queue, - terminal::{disable_raw_mode, enable_raw_mode, ClearType, EnterAlternateScreen, LeaveAlternateScreen}, + terminal::{ClearType, EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode}, }; use futures::{SinkExt, StreamExt}; use kcl_lib::TypedPath; use log::LevelFilter; -use ratatui::{backend::CrosstermBackend, Terminal}; +use ratatui::{Terminal, backend::CrosstermBackend}; use similar::TextDiff; use tokio::sync::mpsc; use tokio_tungstenite::{ - tungstenite::{protocol::Role, Message}, WebSocketStream, + tungstenite::{Message, protocol::Role}, }; use crate::ml::copilot::{ diff --git a/src/ml/copilot/state.rs b/src/ml/copilot/state.rs index 622909c2..31755dc6 100644 --- a/src/ml/copilot/state.rs +++ b/src/ml/copilot/state.rs @@ -436,11 +436,7 @@ fn autocomplete_slash(current: &str) -> Option { return Some(matches[0].to_string()); } let cp = common_prefix(&matches); - if cp.len() > current.len() { - Some(cp) - } else { - None - } + if cp.len() > current.len() { Some(cp) } else { None } } #[cfg(test)] diff --git a/src/ml/copilot/ui.rs b/src/ml/copilot/ui.rs index 11e2d468..f6291b99 100644 --- a/src/ml/copilot/ui.rs +++ b/src/ml/copilot/ui.rs @@ -292,7 +292,7 @@ pub fn draw(frame: &mut Frame, app: &App) { #[cfg(test)] mod tests { - use ratatui::{backend::TestBackend, Terminal}; + use ratatui::{Terminal, backend::TestBackend}; use super::*; diff --git a/src/tests.rs b/src/tests.rs index 24bfa9d5..aee0566a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use anyhow::Result; -use test_context::{test_context, AsyncTestContext}; +use test_context::{AsyncTestContext, test_context}; use crate::config::Config; diff --git a/src/update.rs b/src/update.rs index f1bdc6e4..bf8b05a3 100644 --- a/src/update.rs +++ b/src/update.rs @@ -5,7 +5,7 @@ use std::{ io::{IsTerminal, Write}, }; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result, anyhow}; use serde::{Deserialize, Serialize}; use crate::config_file::get_env_var; diff --git a/tests/win_ca_smoke.rs b/tests/win_ca_smoke.rs index 14d9da49..bdd9981c 100644 --- a/tests/win_ca_smoke.rs +++ b/tests/win_ca_smoke.rs @@ -1,6 +1,6 @@ use std::time::Duration; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result, anyhow}; use tokio::{process::Command, time::sleep}; use url::Url; @@ -78,7 +78,8 @@ async fn win_ca_cli_smoke() -> Result<()> { return Ok(()); } else { last_error = Some(format!( - "CLI succeeded but response missing expected pair {expected_key:?}={expected_value:?}. Full JSON: {json}")); + "CLI succeeded but response missing expected pair {expected_key:?}={expected_value:?}. Full JSON: {json}" + )); break; } } From 11b3bfe1f04a4d410b0d05e547915e185c7fb309 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 15:19:04 -0400 Subject: [PATCH 5/5] Clippy auto-fix to use if-let --- src/cmd_api_call.rs | 44 ++++++++++---------- src/cmd_auth.rs | 8 ++-- src/cmd_file.rs | 92 ++++++++++++++++++++--------------------- src/cmd_kcl.rs | 15 +++---- src/cmd_org.rs | 9 ++-- src/context.rs | 27 ++++++------ src/iostreams.rs | 8 ++-- src/main.rs | 8 ++-- src/ml/copilot/run.rs | 10 ++--- src/ml/copilot/state.rs | 42 +++++++++---------- tests/kcl_process.rs | 8 ++-- 11 files changed, 133 insertions(+), 138 deletions(-) diff --git a/src/cmd_api_call.rs b/src/cmd_api_call.rs index c1b7e60f..45442f22 100644 --- a/src/cmd_api_call.rs +++ b/src/cmd_api_call.rs @@ -67,32 +67,30 @@ impl crate::cmd::Command for CmdApiCallStatus { updated_at: _, user_id: _, } = &api_call + && *status == kittycad::types::ApiCallStatus::Completed + && let Some(outputs) = &outputs { - if *status == kittycad::types::ApiCallStatus::Completed { - if let Some(outputs) = &outputs { - let path = std::env::current_dir()?; - for (name, output) in outputs { - if output.is_empty() { - anyhow::bail!( - "no output was generated for the file conversion! (this is probably a bug in the API) you should report it to support@zoo.dev" - ); - } - let path = path.join(name); - std::fs::write(&path, &output.0)?; - } - - let paths = outputs - .keys() - .map(|k| path.join(k)) - .map(|p| p.to_string_lossy().to_string()) - .collect_vec(); - // Tell them where we saved the file. - writeln!(ctx.io.out, "Saved file conversion output(s) to: {}", paths.join(", "))?; - - // Return early. - return Ok(()); + let path = std::env::current_dir()?; + for (name, output) in outputs { + if output.is_empty() { + anyhow::bail!( + "no output was generated for the file conversion! (this is probably a bug in the API) you should report it to support@zoo.dev" + ); } + let path = path.join(name); + std::fs::write(&path, &output.0)?; } + + let paths = outputs + .keys() + .map(|k| path.join(k)) + .map(|p| p.to_string_lossy().to_string()) + .collect_vec(); + // Tell them where we saved the file. + writeln!(ctx.io.out, "Saved file conversion output(s) to: {}", paths.join(", "))?; + + // Return early. + return Ok(()); } // Print the output of the conversion. diff --git a/src/cmd_auth.rs b/src/cmd_auth.rs index 2cd0f219..63c609e5 100644 --- a/src/cmd_auth.rs +++ b/src/cmd_auth.rs @@ -437,10 +437,10 @@ impl crate::cmd::Command for CmdAuthStatus { let only_host = ctx.global_host().map(|s| s.to_string()); for hostname in &hostnames { - if let Some(h) = only_host.as_deref() { - if h != *hostname { - continue; - } + if let Some(h) = only_host.as_deref() + && h != *hostname + { + continue; } hostname_found = true; diff --git a/src/cmd_file.rs b/src/cmd_file.rs index 340437dd..c9f7a991 100644 --- a/src/cmd_file.rs +++ b/src/cmd_file.rs @@ -199,13 +199,14 @@ pub struct CmdFileSnapshot { impl crate::cmd::Command for CmdFileSnapshot { async fn run(&self, ctx: &mut crate::context::Context) -> Result<()> { // Make sure the parent directory is a directory and exists. - if let Some(parent) = self.output_file.parent() { - if !parent.is_dir() && !parent.to_str().unwrap_or("").is_empty() { - anyhow::bail!( - "directory `{}` does not exist or is not a directory", - parent.to_str().unwrap_or("") - ); - } + if let Some(parent) = self.output_file.parent() + && !parent.is_dir() + && !parent.to_str().unwrap_or("").is_empty() + { + anyhow::bail!( + "directory `{}` does not exist or is not a directory", + parent.to_str().unwrap_or("") + ); } // Parse the image format. @@ -242,46 +243,45 @@ impl crate::cmd::Command for CmdFileSnapshot { .build(), ]; - if matches!(src_format, kcmc::format::InputFormat3d::Gltf(_)) { - if let Ok(str) = std::str::from_utf8(&input) { - if let Ok(json) = serde_json::from_str::(str) { - // Use the path of the control file as the prefix path of - // the relative file name. - - for buffer in json.buffers { - if is_data_uri(&buffer.uri) { - // Using the whole data URI would create massive - // path properties. Use a hash instead. - let mut hasher = DefaultHasher::new(); - buffer.uri.hash(&mut hasher); - let hash_u64 = hasher.finish(); - - if let Some(buf_base64) = buffer.uri.split(',').nth(1) { - files.push( - kcmc::ImportFile::builder() - .path(hash_u64.to_string()) - .data(BASE64_STANDARD.decode(buf_base64)?) - .build(), - ); - } else { - anyhow::bail!("invalid data uri in gltf.buffers.uri property"); - } - } else { - let path_ = self - .input - .parent() - .unwrap_or(std::path::Path::new("")) - .join(std::path::Path::new(&buffer.uri)); - let path = path_.to_str().unwrap_or_default(); - let data = ctx.read_file(path)?; - files.push( - kcmc::ImportFile::builder() - .path(path_.file_name().unwrap_or_default().to_str().unwrap_or("").to_string()) - .data(data) - .build(), - ); - } + if matches!(src_format, kcmc::format::InputFormat3d::Gltf(_)) + && let Ok(str) = std::str::from_utf8(&input) + && let Ok(json) = serde_json::from_str::(str) + { + // Use the path of the control file as the prefix path of + // the relative file name. + + for buffer in json.buffers { + if is_data_uri(&buffer.uri) { + // Using the whole data URI would create massive + // path properties. Use a hash instead. + let mut hasher = DefaultHasher::new(); + buffer.uri.hash(&mut hasher); + let hash_u64 = hasher.finish(); + + if let Some(buf_base64) = buffer.uri.split(',').nth(1) { + files.push( + kcmc::ImportFile::builder() + .path(hash_u64.to_string()) + .data(BASE64_STANDARD.decode(buf_base64)?) + .build(), + ); + } else { + anyhow::bail!("invalid data uri in gltf.buffers.uri property"); } + } else { + let path_ = self + .input + .parent() + .unwrap_or(std::path::Path::new("")) + .join(std::path::Path::new(&buffer.uri)); + let path = path_.to_str().unwrap_or_default(); + let data = ctx.read_file(path)?; + files.push( + kcmc::ImportFile::builder() + .path(path_.file_name().unwrap_or_default().to_str().unwrap_or("").to_string()) + .data(data) + .build(), + ); } } } diff --git a/src/cmd_kcl.rs b/src/cmd_kcl.rs index 9f34e2e4..4bc8e5ce 100644 --- a/src/cmd_kcl.rs +++ b/src/cmd_kcl.rs @@ -351,13 +351,14 @@ pub struct CmdKclSnapshot { impl crate::cmd::Command for CmdKclSnapshot { async fn run(&self, ctx: &mut crate::context::Context) -> Result<()> { // Make sure the parent directory is a directory and exists. - if let Some(parent) = self.output_file.parent() { - if !parent.is_dir() && !parent.to_str().unwrap_or("").is_empty() { - anyhow::bail!( - "directory `{}` does not exist or is not a directory", - parent.to_str().unwrap_or("") - ); - } + if let Some(parent) = self.output_file.parent() + && !parent.is_dir() + && !parent.to_str().unwrap_or("").is_empty() + { + anyhow::bail!( + "directory `{}` does not exist or is not a directory", + parent.to_str().unwrap_or("") + ); } // Parse the image format. diff --git a/src/cmd_org.rs b/src/cmd_org.rs index 636c8d8e..8252b78c 100644 --- a/src/cmd_org.rs +++ b/src/cmd_org.rs @@ -1439,10 +1439,11 @@ fn is_supported_dataset_file(path: &Path) -> bool { return false; }; let mut lower = file_name.to_ascii_lowercase(); - if let Some((stem, suffix)) = lower.rsplit_once('.') { - if !suffix.is_empty() && suffix.chars().all(|c| c.is_ascii_digit()) { - lower = stem.to_string(); - } + if let Some((stem, suffix)) = lower.rsplit_once('.') + && !suffix.is_empty() + && suffix.chars().all(|c| c.is_ascii_digit()) + { + lower = stem.to_string(); } let Some((_, extension)) = lower.rsplit_once('.') else { return false; diff --git a/src/context.rs b/src/context.rs index ca969dc4..fe03b9d6 100644 --- a/src/context.rs +++ b/src/context.rs @@ -73,18 +73,18 @@ impl<'a> Context<'a> { if let Ok(zoo_pager) = get_env_var("ZOO_PAGER") { io.set_pager(zoo_pager); } else { - if let Ok(pager) = config.get("", "pager") { - if !pager.is_empty() { - io.set_pager(pager); - } + if let Ok(pager) = config.get("", "pager") + && !pager.is_empty() + { + io.set_pager(pager); } } // Check if we should force use the tty. - if let Ok(zoo_force_tty) = get_env_var("ZOO_FORCE_TTY") { - if !zoo_force_tty.is_empty() { - io.force_terminal(&zoo_force_tty); - } + if let Ok(zoo_force_tty) = get_env_var("ZOO_FORCE_TTY") + && !zoo_force_tty.is_empty() + { + io.force_terminal(&zoo_force_tty); } Context { @@ -669,12 +669,11 @@ impl<'a> Context<'a> { } } else { // Otherwise be sure we have a kcl file. - if path.to_str().unwrap_or("-") != "-" { - if let Some(ext) = path.extension() { - if ext != "kcl" { - return Err(anyhow::anyhow!("File must have a .kcl extension")); - } - } + if path.to_str().unwrap_or("-") != "-" + && let Some(ext) = path.extension() + && ext != "kcl" + { + return Err(anyhow::anyhow!("File must have a .kcl extension")); } } diff --git a/src/iostreams.rs b/src/iostreams.rs index b4d4ad46..3e24fe0e 100644 --- a/src/iostreams.rs +++ b/src/iostreams.rs @@ -215,10 +215,10 @@ impl IoStreams { return; } - if spec.ends_with('%') { - if let Ok(p) = spec.trim_end_matches('%').parse::() { - self.terminal_width_override = ((self.terminal_width_override as f64) * (p / 100.00)) as i32; - } + if spec.ends_with('%') + && let Ok(p) = spec.trim_end_matches('%').parse::() + { + self.terminal_width_override = ((self.terminal_width_override as f64) * (p / 100.00)) as i32; } } diff --git a/src/main.rs b/src/main.rs index b55d3f30..4db767af 100644 --- a/src/main.rs +++ b/src/main.rs @@ -323,10 +323,10 @@ async fn run_cmd(cmd: &impl crate::cmd::Command, ctx: &mut context::Context<'_>) writeln!(ctx.io.err_out, "zoo.dev api error: {body}")?; } else { writeln!(ctx.io.err_out, "{err}")?; - if let kittycad::types::error::Error::RequestError(inner) = &err { - if let Some(source) = inner.source() { - writeln!(ctx.io.err_out, " caused by: {source}")?; - } + if let kittycad::types::error::Error::RequestError(inner) = &err + && let Some(source) = inner.source() + { + writeln!(ctx.io.err_out, " caused by: {source}")?; } } } diff --git a/src/ml/copilot/run.rs b/src/ml/copilot/run.rs index d900ab1c..9d91a392 100644 --- a/src/ml/copilot/run.rs +++ b/src/ml/copilot/run.rs @@ -472,14 +472,13 @@ pub async fn run_copilot_tui( continue; } let files_ready = files_opt.is_some(); - if let Some(prompt) = app.try_submit(submit, files_ready) { - if let Some(files) = &files_opt { + if let Some(prompt) = app.try_submit(submit, files_ready) + && let Some(files) = &files_opt { let state::QueuedPrompt { content, forced_tools } = prompt; let (msg, _len) = build_user_message(content, files, &project_name, forced_tools); let _ = tx_out.send(WsSend::Client { msg }); } - } } KeyAction::Inserted | KeyAction::None => {} } @@ -509,14 +508,13 @@ pub async fn run_copilot_tui( ScanEvent::Done(map) => { files_opt = Some(map); app.scanning = false; - if let Some(files) = &files_opt { - if let Some(next) = app.on_scan_done() { + if let Some(files) = &files_opt + && let Some(next) = app.on_scan_done() { let state::QueuedPrompt { content, forced_tools } = next; let (msg, _len) = build_user_message(content, files, &project_name, forced_tools); let _ = tx_out.send(WsSend::Client { msg }); } - } } ScanEvent::Error(e) => { app.events.push(ChatEvent::Server(kittycad::types::MlCopilotServerMessage::Error{ detail: e })); } } diff --git a/src/ml/copilot/state.rs b/src/ml/copilot/state.rs index 31755dc6..ad977047 100644 --- a/src/ml/copilot/state.rs +++ b/src/ml/copilot/state.rs @@ -146,10 +146,10 @@ pub fn parse_slash_command(input: &str) -> Option { return Some(SlashCommand::ForceTool(tool)); } let normalized = name.replace('-', "_"); - if normalized != name { - if let Ok(tool) = normalized.parse::() { - return Some(SlashCommand::ForceTool(tool)); - } + if normalized != name + && let Ok(tool) = normalized.parse::() + { + return Some(SlashCommand::ForceTool(tool)); } return None; } @@ -164,10 +164,10 @@ pub fn parse_slash_command(input: &str) -> Option { return Some(SlashCommand::System(cmd)); } let normalized = name.replace('-', "_"); - if normalized != name { - if let Ok(cmd) = normalized.parse::() { - return Some(SlashCommand::System(cmd)); - } + if normalized != name + && let Ok(cmd) = normalized.parse::() + { + return Some(SlashCommand::System(cmd)); } } None @@ -192,10 +192,10 @@ impl App { /// Handle a key event with richer outcomes, including Exit on Ctrl+C. pub fn handle_key_action(&mut self, key: KeyEvent) -> KeyAction { // Ctrl+C always exits - if key.modifiers.contains(KeyModifiers::CONTROL) { - if let KeyCode::Char('c') | KeyCode::Char('C') = key.code { - return KeyAction::Exit; - } + if key.modifiers.contains(KeyModifiers::CONTROL) + && let KeyCode::Char('c') | KeyCode::Char('C') = key.code + { + return KeyAction::Exit; } match key.code { @@ -297,22 +297,20 @@ impl App { /// On EndOfStream, mark not awaiting, and if files are ready and a queue exists, return next to send. pub fn on_end_of_stream(&mut self, files_ready: bool) -> Option { self.awaiting_response = false; - if files_ready { - if let Some(next) = self.queue.pop_front() { - self.awaiting_response = true; - return Some(next); - } + if files_ready && let Some(next) = self.queue.pop_front() { + self.awaiting_response = true; + return Some(next); } None } /// On scanning done, if not awaiting, return next queued to send. pub fn on_scan_done(&mut self) -> Option { - if !self.awaiting_response { - if let Some(next) = self.queue.pop_front() { - self.awaiting_response = true; - return Some(next); - } + if !self.awaiting_response + && let Some(next) = self.queue.pop_front() + { + self.awaiting_response = true; + return Some(next); } None } diff --git a/tests/kcl_process.rs b/tests/kcl_process.rs index f7c4ea43..c54d6678 100644 --- a/tests/kcl_process.rs +++ b/tests/kcl_process.rs @@ -61,10 +61,10 @@ fn apply_test_auth_env(cmd: &mut Command) { } } - if let Ok(host) = std::env::var("ZOO_TEST_HOST") { - if !host.is_empty() { - cmd.env("ZOO_HOST", host); - } + if let Ok(host) = std::env::var("ZOO_TEST_HOST") + && !host.is_empty() + { + cmd.env("ZOO_HOST", host); } }