Add a -write option

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<FileFormat>) {
+        Self::parse_output_file_inner(input, &*self.exists.call)
+    }
+
+    fn parse_output_file_inner(
+        input: &OsStr,
+        exists: &dyn Fn(&Path) -> bool,
+    ) -> (Location, Option<FileFormat>) {
+        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<OsString>) -> Result<ExecutionPlan, MagickError> {
     // TODO: whole lotta stuff: https://imagemagick.org/script/command-line-processing.php
 
@@ -148,11 +200,9 @@ pub fn parse_args(mut args: Vec<OsString>) -> Result<ExecutionPlan, MagickError>
         ));
     }
 
+    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,35 +218,14 @@ pub fn parse_args(mut args: Vec<OsString>) -> Result<ExecutionPlan, MagickError>
             } 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)?);
         }
     }
     Ok(plan)
 }
 
-fn parse_output_file(
-    input: &OsStr,
-    exists: impl FnOnce(&Path) -> bool,
-) -> (Location, Option<FileFormat>) {
-    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<dyn Fn(&Path) -> 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),
         );
     }

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,25 +77,28 @@ enum ExecutionStep {
     /// magick -extract 20x20 rose: +write 'null:' out.png # out is a 20x20 file
     /// ```
     InputFile(FilePlan),
+    Write(Location, Option<FileFormat>),
 }
 
 impl ExecutionPlan {
     pub fn apply_arg(
         &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<FileFormat>) {
-        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<Image> = 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]);
     }
 }

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(&copy_path);
+
+    let write = Command::new(binary)
+        .args(&[
+            "./tests/sample.png",
+            "-write",
+            &copy_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(&copy_path).exists());
+
+    let copy_img = image::open(&copy_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();