diff --git a/src/args.rs b/src/args.rs index 1505508..14ccb3a 100644 --- a/src/args.rs +++ b/src/args.rs @@ -9,7 +9,7 @@ use std::{ }; use crate::{ - arg_parsers::{parse_path_and_format, FileFormat, InputFileArg, Location}, + arg_parsers::{FileFormat, InputFileArg, Location}, error::MagickError, plan::ExecutionPlan, wm_err, @@ -72,6 +72,7 @@ pub enum Arg { Strip, Thumbnail, Unsharp, + Write, } impl Arg { @@ -98,6 +99,7 @@ impl Arg { Arg::Strip => false, Arg::Thumbnail => true, Arg::Unsharp => true, + Arg::Write => true, } } @@ -124,10 +126,60 @@ impl Arg { Arg::Strip => "strip image of all profiles and comments", Arg::Thumbnail => "create a thumbnail of the image", Arg::Unsharp => "sharpen the image", + Arg::Write => "write current image sequence to an output file", } } } +/// Handed to operations that consume an argument to parse it in-context. +#[derive(Debug)] +pub struct ArgParseCtx { + /// Function to check if a path refers to an actual file, or may be a pattern. + /// + /// We punt the actual IO work to be configured on the plan so that the plan may be used in + /// contexts where we should not be dependent on the real filesystem. + exists: ExistsFn, +} + +impl ArgParseCtx { + /// An argument parsing context that will access the host file system directly. + pub fn with_file_system() -> Self { + Self { + exists: ExistsFn { + debug: "real_fs", + call: Box::new(|path: &'_ Path| -> bool { + matches!(std::fs::exists(path), Ok(true)) + }), + }, + } + } + + pub(crate) fn parse_output_file(&self, input: &OsStr) -> (Location, Option) { + Self::parse_output_file_inner(input, &*self.exists.call) + } + + fn parse_output_file_inner( + input: &OsStr, + exists: &dyn Fn(&Path) -> bool, + ) -> (Location, Option) { + let mut output_file = Location::from_arg(input); + let mut output_format = None; + // "-" is parsed as (Stdio, None) no matter what. + // "png:-" is parsed as: + // - (Path("png:-"), None) if a file or dir named "png:-" exists. + // - (Stdio, Some("png")) otherwise. + if let Location::Path(path) = &output_file { + if !exists(path) { + if let Some((path, format)) = crate::arg_parsers::parse_path_and_format(input) { + output_file = Location::from_arg(&path); + output_format = Some(format); + } + } + } + (output_file, output_format) + } +} + pub fn parse_args(mut args: Vec) -> Result { // TODO: whole lotta stuff: https://imagemagick.org/script/command-line-processing.php @@ -148,11 +200,9 @@ pub fn parse_args(mut args: Vec) -> Result )); } + let ctx = ArgParseCtx::with_file_system(); let mut plan = ExecutionPlan::default(); - let (output_file, output_format) = parse_output_file(&output_filename, |path| { - matches!(std::fs::exists(path), Ok(true)) - }); - plan.set_output_file(output_file, output_format); + plan.set_output_file(&output_filename, &ctx); let mut iter = args.into_iter().skip(1); // skip argv[0], path to our binary while let Some(raw_arg) = iter.next() { @@ -168,7 +218,7 @@ pub fn parse_args(mut args: Vec) -> Result } else { None }; - plan.apply_arg(SignedArg { sign, arg }, value.as_deref())?; + plan.apply_arg(SignedArg { sign, arg }, value.as_deref(), &ctx)?; } else { plan.add_input_file(InputFileArg::parse(&raw_arg)?); } @@ -176,27 +226,6 @@ pub fn parse_args(mut args: Vec) -> Result Ok(plan) } -fn parse_output_file( - input: &OsStr, - exists: impl FnOnce(&Path) -> bool, -) -> (Location, Option) { - let mut output_file = Location::from_arg(input); - let mut output_format = None; - // "-" is parsed as (Stdio, None) no matter what. - // "png:-" is parsed as: - // - (Path("png:-"), None) if a file or dir named "png:-" exists. - // - (Stdio, Some("png")) otherwise. - if let Location::Path(path) = &output_file { - if !exists(path) { - if let Some((path, format)) = parse_path_and_format(input) { - output_file = Location::from_arg(&path); - output_format = Some(format); - } - } - } - (output_file, output_format) -} - /// Checks if the string starts with a `-` or a `+`, followed by an ASCII letter fn optionlike(arg: &OsStr) -> bool { matches!( @@ -214,29 +243,42 @@ fn sign_and_arg_name(raw_arg: OsString) -> Result<(ArgSign, String), MagickError Ok((ArgSign::try_from(sign)?, string)) } +struct ExistsFn { + debug: &'static str, + call: Box bool>, +} + +impl core::fmt::Debug for ExistsFn { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("ExistsFn") + .field("debug_name", &self.debug) + .finish_non_exhaustive() + } +} + #[cfg(test)] mod tests { + use super::*; + use image::ImageFormat; use std::path::PathBuf; - use super::*; - #[test] fn test_parse_output_file() { assert_eq!( - parse_output_file(OsStr::new("-"), |_| false), + ArgParseCtx::parse_output_file_inner(OsStr::new("-"), &|_| false), (Location::Stdio, None), ); assert_eq!( - parse_output_file(OsStr::new("-"), |_| true), + ArgParseCtx::parse_output_file_inner(OsStr::new("-"), &|_| true), (Location::Stdio, None), ); assert_eq!( - parse_output_file(OsStr::new("png:-"), |_| false), + ArgParseCtx::parse_output_file_inner(OsStr::new("png:-"), &|_| false), (Location::Stdio, Some(FileFormat::Format(ImageFormat::Png))), ); assert_eq!( - parse_output_file(OsStr::new("png:-"), |_| true), + ArgParseCtx::parse_output_file_inner(OsStr::new("png:-"), &|_| true), (Location::Path(PathBuf::from("png:-")), None), ); } diff --git a/src/plan.rs b/src/plan.rs index 42b5943..694d6d3 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -8,7 +8,7 @@ use crate::arg_parsers::{ FileFormat, GrayscaleMethod, IdentifyFormat, InputFileArg, Location, ResizeGeometry, UnsharpenGeometry, }; -use crate::args::{Arg, ArgSign, SignedArg}; +use crate::args::{Arg, ArgParseCtx, ArgSign, SignedArg}; use crate::decode::decode; use crate::image::Image; use crate::utils::filename::insert_suffix_before_extension_in_path; @@ -77,6 +77,7 @@ enum ExecutionStep { /// magick -extract 20x20 rose: +write 'null:' out.png # out is a 20x20 file /// ``` InputFile(FilePlan), + Write(Location, Option), } impl ExecutionPlan { @@ -84,18 +85,20 @@ impl ExecutionPlan { &mut self, signed_arg: SignedArg, value: Option<&OsStr>, + ctx: &ArgParseCtx, ) -> Result<(), MagickError> { let arg_string: &'static str = signed_arg.arg.into(); if signed_arg.needs_value() != value.is_some() { return Err(wm_err!("argument requires a value: {arg_string}")); }; - self.apply_arg_inner(signed_arg, value).map_err(|arg_err| { - wm_err!( - "{}", - arg_err.display_with_arg(arg_string, value.unwrap_or_default()) - ) - })?; + self.apply_arg_inner(signed_arg, value, ctx) + .map_err(|arg_err| { + wm_err!( + "{}", + arg_err.display_with_arg(arg_string, value.unwrap_or_default()) + ) + })?; Ok(()) } @@ -106,6 +109,7 @@ impl ExecutionPlan { &mut self, signed_arg: SignedArg, value: Option<&OsStr>, + ctx: &ArgParseCtx, ) -> Result<(), ArgParseErr> { match signed_arg.arg { Arg::AutoOrient => self.add_operation(Operation::AutoOrient), @@ -207,6 +211,9 @@ impl ExecutionPlan { Arg::Unsharp => self.add_operation(Operation::Unsharpen(UnsharpenGeometry::try_from( value.unwrap(), )?)), + Arg::Write => { + self.add_output(value.unwrap(), ctx)?; + } }; Ok(()) @@ -240,6 +247,16 @@ impl ExecutionPlan { } } + fn add_output(&mut self, loc: &OsStr, ctx: &ArgParseCtx) -> Result<(), ArgParseErr> { + if self.execution.is_empty() { + return Err(ArgParseErr::with_msg("no images for write -write")); + } + + let (loc, format) = ctx.parse_output_file(loc); + self.execution.push(ExecutionStep::Write(loc, format)); + Ok(()) + } + pub fn add_input_file(&mut self, file: InputFileArg) { let mut file_plan = FilePlan { location: file.location, @@ -282,9 +299,10 @@ impl ExecutionPlan { self.execution.push(ExecutionStep::InputFile(file_plan)); } - pub fn set_output_file(&mut self, file: Location, format: Option) { - self.output_file = file; - self.output_format = format; + pub fn set_output_file(&mut self, file: &OsStr, ctx: &ArgParseCtx) { + let (output_file, output_format) = ctx.parse_output_file(file); + self.output_file = output_file; + self.output_format = output_format; } pub fn execute(&self) -> Result<(), MagickError> { @@ -297,7 +315,10 @@ impl ExecutionPlan { crate::init::init(); let mut sequence: Vec = vec![]; - for step in &self.execution { + + let output = ExecutionStep::Write(self.output_file.clone(), self.output_format); + + for step in self.execution.iter().chain([&output]) { match step { ExecutionStep::Map(op) => { for image in &mut sequence { @@ -316,19 +337,15 @@ impl ExecutionPlan { ExecutionStep::Rewrite(op) => { op.execute(&mut sequence)?; } + ExecutionStep::Write(location, format) => { + let output_locations = Self::output_locations(location, &sequence); + for (image, specific_location) in sequence.iter_mut().zip(output_locations) { + encode::encode(image, &specific_location, *format, &self.modifiers)?; + } + } } } - let output_locations = Self::output_locations(&self.output_file, &sequence); - for (image, specific_location) in sequence.iter_mut().zip(output_locations) { - encode::encode( - image, - &specific_location, - self.output_format, - &self.modifiers, - )?; - } - Ok(()) } @@ -400,6 +417,7 @@ mod tests { use super::*; use image::ImageFormat; + use std::path::PathBuf; #[test] fn test_output_locations() { @@ -453,6 +471,6 @@ mod tests { let outputs = ExecutionPlan::output_locations(&Location::Stdio, &two_sequence); - assert_eq!(outputs, vec![Location::Stdio, Location::Stdio],); + assert_eq!(outputs, vec![Location::Stdio, Location::Stdio]); } } diff --git a/tests/wm_convert_integration_tests.rs b/tests/wm_convert_integration_tests.rs index 97694f2..b9622a0 100644 --- a/tests/wm_convert_integration_tests.rs +++ b/tests/wm_convert_integration_tests.rs @@ -85,6 +85,45 @@ fn combine_succeeds() { assert!(String::from_utf8(identify.stdout).unwrap().contains("sRGB")); } +#[test] +fn test_write_as_intermediate() { + let _guard = LOCK.lock().unwrap(); + + let binary = env!("CARGO_BIN_EXE_wm-convert"); + let tmp_dir = env!("CARGO_TARGET_TMPDIR"); + + let output_path = format!("{}/resized.jpg", tmp_dir); + let copy_path = format!("{}/copy.png", tmp_dir); + let _ = fs::remove_file(&output_path); + let _ = fs::remove_file(©_path); + + let write = Command::new(binary) + .args(&[ + "./tests/sample.png", + "-write", + ©_path, + "-resize", + "50%x50%", + &output_path, + ]) + .output() + .expect("convert did not exit successfully"); + + assert!(write.status.success()); + assert!(std::path::Path::new(&output_path).exists()); + assert!(std::path::Path::new(©_path).exists()); + + let copy_img = image::open(©_path).unwrap(); + let output_img = image::open(&output_path).unwrap(); + + use image::GenericImageView as _; + assert_ne!( + copy_img.dimensions(), + output_img.dimensions(), + "Intermediate write should have twice the dimensions of the final output", + ); +} + #[test] fn combine_upgrades_to_rgba() { let _guard = LOCK.lock().unwrap();