Merge pull request #954 from SteamSmith/fix/file-urls

engine: build well-formed file:// URLs for Edit Original, Reveal and File.execute

crates/engine/src/commands/app_more.rs

@@ -194,7 +194,7 @@ fn execute_file(s: &mut Session, p: &Value) -> Result<Value> {
         s.events.push(Event::Frontend { command: "app.confirmExecute".into(), params: json!({"path": path}) });
         return Ok(json!({"needsConfirmation": path}));
     }
-    let url = format!("file://{}", path.replace(' ', "%20"));
+    let url = super::layer_menu::file_url(path);
     s.events.push(Event::OpenUrl(url));
     Ok(json!({"opened": path}))
 }
@@ -621,3 +621,25 @@ pub fn specs() -> Vec<CommandSpec> {
         query!("help.systemInfo", "System Information", "{}", |s, _| Ok(system_report(s))),
     ]
 }
+
+#[cfg(test)]
+mod tests {
+    use serde_json::json;
+
+    /// `File.execute()` opens the file as a well-formed `file://` URL (spaces, `#`, `%`).
+    #[test]
+    fn execute_file_opens_an_encoded_file_url() {
+        let dir = std::env::temp_dir().join(format!("ec-exec-url-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let f = dir.join("run #1 100%.txt");
+        std::fs::write(&f, "x").unwrap();
+        let mut s = crate::Session::default();
+        s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap();
+        s.drain_events();
+        s.execute("file.executeFile", json!({"path": f.to_string_lossy(), "confirmed": true})).unwrap();
+        let expected = super::super::layer_menu::file_url(&f.to_string_lossy());
+        assert!(expected.ends_with("/run%20%231%20100%25.txt"), "{expected}");
+        assert!(s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(u) if *u == expected)));
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}

crates/engine/src/commands/edit.rs

@@ -809,7 +809,7 @@ fn original_path(s: &Session) -> Option<String> {
 
 fn edit_original(s: &mut Session, _: &Value) -> Result<Value> {
     let path = original_path(s).ok_or_else(|| super::bad("edit.editOriginal", "select a footage item or layer"))?;
-    let url = format!("file://{path}");
+    let url = super::layer_menu::file_url(&path);
     s.events.push(crate::Event::OpenUrl(url.clone()));
     Ok(json!({"url": url}))
 }
@@ -938,3 +938,25 @@ fn layers_or_keys(s: &Session) -> std::result::Result<(), String> {
 fn always_ok(_: &Session) -> std::result::Result<(), String> {
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use serde_json::json;
+
+    /// Edit Original opens the footage as a well-formed `file://` URL, whatever its name contains.
+    #[test]
+    fn edit_original_opens_an_encoded_file_url() {
+        let mut s = crate::Session::default();
+        let id = s
+            .edit("add", None, |p, _| {
+                let f = effectcraft_project::Footage { path: "/clips/take #1.mov".into(), ..Default::default() };
+                Ok(p.add_item("take", effectcraft_color::Label::Lavender, None, effectcraft_project::ItemKind::Footage(f)))
+            })
+            .unwrap();
+        s.state.project_selection = vec![id];
+        s.drain_events();
+        let r = s.execute("edit.editOriginal", json!({})).unwrap();
+        assert_eq!(r["url"], "file:///clips/take%20%231.mov");
+        assert!(s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(u) if u == "file:///clips/take%20%231.mov")));
+    }
+}

crates/engine/src/commands/layer_menu.rs

@@ -661,7 +661,24 @@ fn open_source(s: &mut Session, p: &Value) -> Result<Value> {
 /// Folder containing a file as a `file://` URL.
 pub(crate) fn folder_url(path: &str) -> String {
     let dir = std::path::Path::new(path).parent().map(|d| d.to_string_lossy().to_string()).unwrap_or_default();
-    format!("file://{dir}")
+    file_url(&dir)
+}
+
+/// `path` as a `file://` URL: backslashes become `/`, a Windows drive path gets the third slash
+/// (`file:///C:/…`), and every byte that isn't a plain path character is percent-encoded (spaces,
+/// `#`, `%`, `?`, non-ASCII as UTF-8), so the URL names exactly this file.
+pub(crate) fn file_url(path: &str) -> String {
+    use std::fmt::Write as _;
+    let path = path.replace('\\', "/");
+    let mut url = String::from(if path.starts_with('/') { "file://" } else { "file:///" });
+    for b in path.bytes() {
+        if b.is_ascii_alphanumeric() || b"/-._~:@!$&'()*+,;=".contains(&b) {
+            url.push(char::from(b));
+        } else {
+            let _ = write!(url, "%{b:02X}");
+        }
+    }
+    url
 }
 
 fn reveal_in_finder(s: &mut Session, p: &Value) -> Result<Value> {
@@ -845,3 +862,21 @@ pub fn specs() -> Vec<CommandSpec> {
         ),
     ]
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{file_url, folder_url};
+
+    #[test]
+    fn file_urls_are_well_formed() {
+        assert_eq!(file_url("C:\\Users\\Ann\\Footage\\clip 1.mov"), "file:///C:/Users/Ann/Footage/clip%201.mov");
+        assert_eq!(file_url("/home/ann/a#b%c?d.mov"), "file:///home/ann/a%23b%25c%3Fd.mov");
+        assert_eq!(file_url("/Users/ann/Café/x.mov"), "file:///Users/ann/Caf%C3%A9/x.mov");
+        assert_eq!(file_url("/plain/path-1_2.~(x).mov"), "file:///plain/path-1_2.~(x).mov", "plain characters stay");
+    }
+
+    #[test]
+    fn folder_url_names_the_containing_folder() {
+        assert_eq!(folder_url("/media/My Clips/a.mov"), "file:///media/My%20Clips");
+    }
+}