Merge #1160: M0.9: figcraft-cli export names files as the app's export does (#1154) (@switch87)

apps/figcraft-cli/src/files.rs

@@ -207,9 +207,11 @@ pub fn info(path: &str, as_json: bool) -> CliResult<ExitCode> {
     Ok(ExitCode::SUCCESS)
 }
 
+/// The file name a layer exports to: the app's rule ([`figcraft_engine::cmd::file::safe_file_name`],
+/// so `/` becomes `_` too), without leading or trailing spaces and dots, at most 100 characters.
 fn sanitize(name: &str) -> String {
-    let s: String = name.chars().map(|c| if c.is_alphanumeric() || matches!(c, '-' | '_' | ' ' | '.') { c } else { '_' }).collect();
-    let s = s.trim().trim_matches('.').to_string();
+    let s = figcraft_engine::cmd::file::safe_file_name(name);
+    let s = s.trim_matches(|c: char| c == '.' || c.is_whitespace()).to_string();
     if s.is_empty() { "layer".into() } else { s.chars().take(100).collect() }
 }
 
@@ -411,6 +413,13 @@ mod tests {
         assert_eq!(sanitize("Home / Hero"), "Home _ Hero");
         assert_eq!(sanitize("../.."), "_");
         assert_eq!(sanitize(""), "layer");
+        // The same characters the app's export keeps (#1154): only what file systems refuse goes.
+        assert_eq!(sanitize("Homepage - desktop (live + verbeteringen)"), "Homepage - desktop (live + verbeteringen)");
+        assert_eq!(sanitize("Q&A, 50% #1 [v2] {x} ~é"), "Q&A, 50% #1 [v2] {x} ~é");
+        assert_eq!(sanitize("a\\b:c*d?e\"f<g>h|i\tj"), "a_b_c_d_e_f_g_h_i_j");
+        assert_eq!(sanitize(" .hidden. "), "hidden");
+        assert_eq!(sanitize(" . .name. . "), "name", "spaces and dots at the edges, mixed");
+        assert_eq!(sanitize(&"x".repeat(300)).chars().count(), 100);
     }
 
     #[test]

apps/figcraft-cli/tests/it/cli.rs

@@ -114,6 +114,30 @@ fn run_export_info_convert() {
     std::fs::remove_dir_all(&d).ok();
 }
 
+/// `export` into a folder names the files as the app's export does (#1154): brackets, `+`, `&`,
+/// `%`, `#` and commas stay, what file systems refuse becomes `_`.
+#[test]
+fn export_file_names_keep_what_the_app_keeps() {
+    let d = tmp("names");
+    let script = d.join("s.json");
+    let names = ["Homepage - desktop (live + verbeteringen)", "Q&A, 50% #1 [v2]", "a:b*c?d|e"];
+    let steps: Vec<serde_json::Value> = names
+        .iter()
+        .enumerate()
+        .map(|(i, n)| serde_json::json!({"command": "frame.create", "params": {"x": i * 200, "y": 0, "width": 20, "height": 20, "name": n}}))
+        .collect();
+    std::fs::write(&script, serde_json::to_string(&steps).unwrap()).unwrap();
+    let file = d.join("doc.figcraft");
+    assert!(bin().arg("run").arg(&script).arg("--out").arg(&file).output().unwrap().status.success());
+    let dir = d.join("out");
+    let out = bin().arg("export").arg(&file).arg("-o").arg(&dir).output().unwrap();
+    assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr));
+    for want in ["Homepage - desktop (live + verbeteringen).png", "Q&A, 50% #1 [v2].png", "a_b_c_d_e.png"] {
+        assert!(dir.join(want).is_file(), "{want} in {:?}", std::fs::read_dir(&dir).unwrap().flatten().map(|e| e.file_name()).collect::<Vec<_>>());
+    }
+    std::fs::remove_dir_all(&d).ok();
+}
+
 #[test]
 fn run_takes_the_documented_gradient_shorthand() {
     // #252: only MCP used to accept it; `run` said "`fills` must be paints or colours".

contributors/people.toml

@@ -71,3 +71,8 @@ sync_github_name = true
 
 [people.AganzeFelicite]
 sync_github_name = true
+
+[people.switch87]
+real_name = "Gert Pellin"
+display_name = "Gert Pellin"
+sync_github_name = false

crates/engine/src/cmd/file.rs

@@ -1181,8 +1181,11 @@ fn export_path(name: &str) -> String {
     if parts.is_empty() { "Untitled".into() } else { parts.join("/") }
 }
 
-/// `name` without path separators and characters file systems reject.
-fn safe_file_name(name: &str) -> String {
+/// `name` without path separators and characters file systems reject (`/ \ : * ? " < > |` and
+/// control characters become `_`); everything else is kept, up to the first 200 characters.
+/// `figcraft-cli export` names its files by this rule too (and keeps at most 100 characters before
+/// the number and extension it adds).
+pub fn safe_file_name(name: &str) -> String {
     name.chars().take(200).map(|c| if c.is_control() || "/\\:*?\"<>|".contains(c) { '_' } else { c }).collect()
 }
 

docs/cli.md

@@ -53,6 +53,10 @@ page 1 "Screens": 380 nodes, 4 top-level
 
 - `<out>` is a directory: one file per top-level frame of the page (default the first page), named
   after the frame (`Home.png`, `Settings.png`…); with no frames, one file per top-level layer.
+  The name is kept as it is, except for the characters file systems refuse (`/ \ : * ? " < > |`
+  and control characters), which become `_`, and leading or trailing spaces and dots, which are
+  dropped. A name is cut at 100 characters. The app's export replaces the same characters (there a
+  `/` makes a folder). Two layers with the same name get `Name 2`, `Name 3`…
 - `<out>` has an image extension: one file with the given `--node`, or the whole page composited.
 - `--node` takes a layer id (`12` or `0:12`) or a layer name.
 - `--format fig` (or an `<out>` ending in `.fig`) writes the whole file as `.fig`, experimental: