Merge PR #716 from switch87: CLI: convert and run refuse output formats DeckCraft can't write (#715)

apps/deckcraft-cli/src/main.rs

@@ -240,9 +240,9 @@ fn render_transition(args: &[String]) -> Result<(), String> {
 
 fn convert(args: &[String]) -> Result<(), String> {
     let (Some(input), Some(out)) = (args.first(), args.get(1)) else { return Err("convert needs IN and OUT".into()) };
+    let fmt = output_format(out)?;
     let mut s = Session::new();
     open(&mut s, Some(input), false)?;
-    let fmt = deckcraft_engine::cmd::file::format_for_path(out);
     let r = if matches!(fmt, "png" | "jpeg") {
         s.execute("file.export", &json!({"path": out, "slide": 0}))
     } else {
@@ -253,6 +253,16 @@ fn convert(args: &[String]) -> Result<(), String> {
     Ok(())
 }
 
+/// The format `convert`, `run --save` and `run --export` write to `path`, or an error naming the
+/// supported extensions when `path`'s extension isn't one (rather than native bytes under that name).
+fn output_format(path: &str) -> Result<&'static str, String> {
+    use deckcraft_engine::cmd::file::{WRITABLE_EXTENSIONS, writable_format_for_path};
+    writable_format_for_path(path).ok_or_else(|| {
+        let ext = std::path::Path::new(path).extension().map(|e| format!(".{}", e.to_string_lossy())).unwrap_or_else(|| "(none)".into());
+        format!("{path}: unsupported output format {ext} (supported: .{})", WRITABLE_EXTENSIONS.join(", ."))
+    })
+}
+
 fn run(args: &[String]) -> Result<(), String> {
     let mut input = None;
     let mut sample = false;
@@ -272,6 +282,9 @@ fn run(args: &[String]) -> Result<(), String> {
             other => return Err(format!("unknown option `{other}`")),
         }
     }
+    for p in save.iter().chain(&export) {
+        output_format(p)?;
+    }
     let mut s = Session::new();
     open(&mut s, input.as_deref(), sample)?;
     for c in cmds {

apps/deckcraft-cli/tests/convert.rs

@@ -0,0 +1,57 @@
+//! `deckcraft-cli convert` writes the format OUT's extension names, or fails without writing.
+
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output};
+
+fn cli(args: &[&Path]) -> Output {
+    Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(args).output().unwrap()
+}
+
+fn temp_dir(name: &str) -> PathBuf {
+    let dir = std::env::temp_dir().join(format!("deckcraft-cli-{name}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    dir
+}
+
+#[test]
+fn convert_rejects_output_formats_it_cannot_write() {
+    let dir = temp_dir("convert-unsupported");
+    let input = dir.join("in.pptx");
+    let out = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(["run", "--sample", "--export"]).arg(&input).output().unwrap();
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+
+    for name in ["out.odp", "out.gif", "out.svg", "out.outline", "out"] {
+        let target = dir.join(name);
+        let out = cli(&[Path::new("convert"), &input, &target]);
+        assert!(!out.status.success(), "{name}: convert should fail");
+        let err = String::from_utf8_lossy(&out.stderr);
+        assert!(err.contains("unsupported output format") && err.contains(".pptx"), "{name}: {err}");
+        assert!(!target.exists(), "{name}: nothing is written");
+    }
+
+    let target = dir.join("out.deckcraft");
+    let out = cli(&[Path::new("convert"), &input, &target]);
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    assert!(std::fs::read(&target).unwrap().starts_with(b"PK\x03\x04"));
+
+    let target = dir.join("out.txt");
+    let out = cli(&[Path::new("convert"), &input, &target]);
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    assert!(!std::fs::read(&target).unwrap().starts_with(b"PK"), "an outline is text, not a zip");
+
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+#[test]
+fn run_rejects_unsupported_save_and_export_paths_before_running() {
+    let dir = temp_dir("run-unsupported");
+    for flag in ["--save", "--export"] {
+        let target = dir.join("deck.odp");
+        let out = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(["run", "--sample", flag]).arg(&target).output().unwrap();
+        assert!(!out.status.success(), "{flag}: run should fail");
+        assert!(String::from_utf8_lossy(&out.stderr).contains("unsupported output format .odp"), "{flag}");
+        assert!(!target.exists(), "{flag}: nothing is written");
+    }
+    let _ = std::fs::remove_dir_all(&dir);
+}

crates/engine/src/cmd/file.rs

@@ -191,6 +191,17 @@ pub fn pdf_bytes(doc: &Presentation, p: &Value) -> Result<Vec<u8>> {
     deckcraft_pdf::export(doc, &pdf_options(p, doc.slides.len())?).map_err(|e| EngineError::Other(e.to_string()))
 }
 
+/// The output extensions [`format_for_path`] maps to a writer of that format.
+pub const WRITABLE_EXTENSIONS: &[&str] = &["deckcraft", "pptx", "potx", "ppsx", "pdf", "png", "jpg", "jpeg", "txt", "md"];
+
+/// The format a file named `path` is written in, or `None` when its extension names no format
+/// DeckCraft writes (`.odp`, `.gif`, `.svg`, no extension …): [`format_for_path`] would save
+/// those as native `.deckcraft` bytes under the wrong name.
+pub fn writable_format_for_path(path: &str) -> Option<&'static str> {
+    let ext = std::path::Path::new(path).extension()?.to_str()?.to_ascii_lowercase();
+    WRITABLE_EXTENSIONS.contains(&ext.as_str()).then(|| format_for_path(path))
+}
+
 pub fn format_for_path(path: &str) -> &'static str {
     let l = path.to_ascii_lowercase();
     if l.ends_with(".pptx") || l.ends_with(".potx") || l.ends_with(".ppsx") {

crates/engine/src/tests.rs

@@ -181,6 +181,19 @@ fn sample_deck_builds_and_renders() {
     assert!(v["slides"].as_array().unwrap().len() >= 8);
 }
 
+#[test]
+fn writable_format_for_path_rejects_formats_deckcraft_does_not_write() {
+    use crate::cmd::file::writable_format_for_path as f;
+    assert_eq!(f("a.deckcraft"), Some("deckcraft"));
+    assert_eq!(f("dir/A.PPTX"), Some("pptx"));
+    assert_eq!(f("a.pdf"), Some("pdf"));
+    assert_eq!(f("a.jpg"), Some("jpeg"));
+    assert_eq!(f("a.txt"), Some("outline"));
+    for path in ["a.odp", "a.gif", "a.svg", "a.outline", "a", "dir.v2/a"] {
+        assert_eq!(f(path), None, "{path}");
+    }
+}
+
 #[test]
 fn save_and_reopen_native() {
     let mut s = Session::new();