diff --git a/src/cmd_file.rs b/src/cmd_file.rs index c9f7a991..fb059d11 100644 --- a/src/cmd_file.rs +++ b/src/cmd_file.rs @@ -99,6 +99,7 @@ pub struct CmdFileConvert { #[async_trait::async_trait(?Send)] impl crate::cmd::Command for CmdFileConvert { async fn run(&self, ctx: &mut crate::context::Context) -> Result<()> { + let format = ctx.format(&self.format)?; // Make sure the output dir is a directory. if !self.output_dir.is_dir() { anyhow::bail!( @@ -137,11 +138,9 @@ impl crate::cmd::Command for CmdFileConvert { } else { std::fs::write(&path, data)?; } - writeln!( - ctx.io.out, - "wrote file `{}` to {}", - filename, - path.to_str().unwrap_or("") + ctx.io.write_status( + &format, + format_args!("wrote file `{}` to {}", filename, path.to_str().unwrap_or("")), )?; } } else { @@ -156,7 +155,6 @@ impl crate::cmd::Command for CmdFileConvert { file_conversion.outputs = None; // Print the output of the conversion. - let format = ctx.format(&self.format)?; ctx.io.write_output(&format, &file_conversion)?; Ok(()) diff --git a/src/cmd_project.rs b/src/cmd_project.rs index cce333b0..7ed90e9e 100644 --- a/src/cmd_project.rs +++ b/src/cmd_project.rs @@ -359,6 +359,7 @@ pub struct CmdProjectPublish { #[async_trait::async_trait(?Send)] impl crate::cmd::Command for CmdProjectPublish { async fn run(&self, ctx: &mut crate::context::Context) -> Result<()> { + let format = ctx.format(&self.format)?; let environment = ctx.project_cloud_environment_name("")?; let target = resolve_project_target(&self.input, &environment)?; let project_id = target.id(); @@ -369,14 +370,15 @@ impl crate::cmd::Command for CmdProjectPublish { if let ProjectTarget::Local { local, .. } = target { crate::project::persist_cloud_project_id(&local.project_toml, &environment, project.id)?; } - writeln!( - ctx.io.out, - "{} Submitted Zoo cloud project {} for publication review", - ctx.io.color_scheme().success_icon(), - project.id + ctx.io.write_status( + &format, + format_args!( + "{} Submitted Zoo cloud project {} for publication review", + ctx.io.color_scheme().success_icon(), + project.id + ), )?; - let format = ctx.format(&self.format)?; write_project_output(ctx, &format, &project)?; Ok(()) } @@ -417,6 +419,7 @@ pub struct CmdProjectUpload { #[async_trait::async_trait(?Send)] impl crate::cmd::Command for CmdProjectUpload { async fn run(&self, ctx: &mut crate::context::Context) -> Result<()> { + let format = ctx.format(&self.format)?; let local = crate::project::resolve_local_project(&self.input)?; let environment = ctx.project_cloud_environment_name("")?; let existing_id = match self.id { @@ -443,16 +446,17 @@ impl crate::cmd::Command for CmdProjectUpload { }; crate::project::persist_cloud_project_id(&local.project_toml, &environment, project.id)?; - writeln!( - ctx.io.out, - "{} {} Zoo cloud project id {} in {}", - ctx.io.color_scheme().success_icon(), - if existing_id.is_some() { "Updated" } else { "Stored" }, - project.id, - local.project_toml.display() + ctx.io.write_status( + &format, + format_args!( + "{} {} Zoo cloud project id {} in {}", + ctx.io.color_scheme().success_icon(), + if existing_id.is_some() { "Updated" } else { "Stored" }, + project.id, + local.project_toml.display() + ), )?; - let format = ctx.format(&self.format)?; write_project_output(ctx, &format, &project)?; Ok(()) } diff --git a/src/iostreams.rs b/src/iostreams.rs index 3e24fe0e..48c6eaf4 100644 --- a/src/iostreams.rs +++ b/src/iostreams.rs @@ -226,6 +226,22 @@ impl IoStreams { crate::colors::ColorScheme::new(self.color_enabled(), self.color_support_256(), self.has_true_color()) } + /// Write a status line without mixing prose into structured stdout. + /// The format must be resolved through `Context::format` to honor configuration. + pub fn write_status( + &mut self, + format: &crate::types::FormatOutput, + message: std::fmt::Arguments<'_>, + ) -> Result<()> { + let out = if format.is_machine_friendly_output() { + &mut self.err_out + } else { + &mut self.out + }; + writeln!(out, "{message}")?; + Ok(()) + } + #[allow(dead_code)] pub fn write_output_for_vec( &mut self, diff --git a/src/tests.rs b/src/tests.rs index f640dfd3..c164f696 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -13,6 +13,74 @@ macro_rules! svec { }; } +/// Tests that we don't write natural language status messages +/// if the CLI is supposed to output json/yaml. +#[test] +fn status_output_respects_format() -> Result<()> { + use crate::types::FormatOutput::{Json, Table, Yaml}; + + enum WrittenTo { + Stdout, + Stderr, + } + + #[rustfmt::skip] + let tests = [ + // configured, explicit, expected + (None, None, WrittenTo::Stdout ), + (None, Some(Table), WrittenTo::Stdout ), + (None, Some(Json), WrittenTo::Stderr ), + (None, Some(Yaml), WrittenTo::Stderr ), + (Some("json"), None, WrittenTo::Stderr ), + (Some("yaml"), None, WrittenTo::Stderr ), + (Some("json"), Some(Table), WrittenTo::Stdout ), + (Some("table"), Some(Json), WrittenTo::Stderr ), + ]; + for (configured, explicit, expected) in tests { + // Set up context for this test. + let mut config = TestConfig::new()?; + if let Some(format) = configured { + config.set("", "format", Some(format))?; + } + let (io, stdout_path, stderr_path) = crate::iostreams::IoStreams::test(); + let mut ctx = crate::context::Context { + config: &mut config, + io, + debug: false, + override_host: None, + }; + + // Action: Write a status message wherever the context is configured. + let format = ctx.format(&explicit)?; + ctx.io.write_status(&format, format_args!("processed {} files", 4))?; + drop(ctx); + + // Validate that the status message was written where we expect. + let actual_stdout = std::fs::read_to_string(&stdout_path)?; + let actual_stderr = std::fs::read_to_string(&stderr_path)?; + std::fs::remove_file(stdout_path)?; + std::fs::remove_file(stderr_path)?; + let status = "processed 4 files\n"; + let expected_stderr = match expected { + WrittenTo::Stdout => "", + WrittenTo::Stderr => status, + }; + let expected_stdout = match expected { + WrittenTo::Stdout => status, + WrittenTo::Stderr => "", + }; + assert_eq!( + expected_stdout, actual_stdout, + "configured={configured:?}, explicit={explicit:?}" + ); + assert_eq!( + expected_stderr, actual_stderr, + "configured={configured:?}, explicit={explicit:?}" + ); + } + Ok(()) +} + macro_rules! cli_tests { ($($name:ident($ctx:ident) => $body:block)+) => { $( diff --git a/src/types.rs b/src/types.rs index 47aae243..d403dd73 100644 --- a/src/types.rs +++ b/src/types.rs @@ -9,6 +9,7 @@ pub enum FormatOutput { Yaml, #[default] Table, + // If you add another variant, add it to the `variants()` method below too. } #[derive(Debug, Clone, PartialEq, Eq, FromStr, Display, clap::ValueEnum, Copy)] @@ -27,6 +28,16 @@ impl FormatOutput { pub const fn variants() -> &'static [&'static str] { &["table", "json", "yaml"] } + + /// Whether stdout must be reserved for machine-readable output. + /// Human-readable status messages belong on stderr for these formats. + pub const fn is_machine_friendly_output(&self) -> bool { + match self { + FormatOutput::Json => true, + FormatOutput::Yaml => true, + FormatOutput::Table => false, + } + } } #[derive(Deserialize)]