diff --git a/apps/effectcraft-web/src/files.rs b/apps/effectcraft-web/src/files.rs index 3f3f3a99f..d6e6f5a06 100644 --- a/apps/effectcraft-web/src/files.rs +++ b/apps/effectcraft-web/src/files.rs @@ -151,6 +151,10 @@ impl Services for WebServices { fn is_dir(&self, _path: &str) -> bool { false } + /// The table's keys are the paths themselves. + fn full_path(&self, path: &str) -> String { + path.to_string() + } } /// Media import from the table (the bytes are also handed to the media pool, which decodes them). diff --git a/crates/engine/src/commands/file_more.rs b/crates/engine/src/commands/file_more.rs index e92b43dc0..cee7e3236 100644 --- a/crates/engine/src/commands/file_more.rs +++ b/crates/engine/src/commands/file_more.rs @@ -463,7 +463,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { return Err(bad("file.runScript", JSXBIN)); } let bytes = s.services.read_file(path).map_err(|e| EngineError::Other(format!("cannot read {path}: {e}")))?; - remember_script(s, crate::prefs::RecentScript::Path(path.to_string())); + let full = s.services.full_path(path); + remember_script(s, crate::prefs::RecentScript::Path(full)); let code = String::from_utf8_lossy(&bytes).to_string(); let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); let out = run_js(s, &code, path, false)?; @@ -473,7 +474,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { (Some(Value::Array(a)), _) => a.clone(), (_, Some(path)) => { let bytes = s.services.read_file(path).map_err(|e| EngineError::Other(format!("cannot read {path}: {e}")))?; - remember_script(s, crate::prefs::RecentScript::Path(path.to_string())); + let full = s.services.full_path(path); + remember_script(s, crate::prefs::RecentScript::Path(full)); let text = String::from_utf8_lossy(&bytes).to_string(); match serde_json::from_str::(&text) { Ok(Value::Array(a)) => a, diff --git a/crates/engine/src/commands/scripts.rs b/crates/engine/src/commands/scripts.rs index 62701cd10..a1b49ccd8 100644 --- a/crates/engine/src/commands/scripts.rs +++ b/crates/engine/src/commands/scripts.rs @@ -603,6 +603,58 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } + /// Reads relative paths from a working directory that a test can change. + struct InFolder(std::sync::Mutex); + + impl InFolder { + fn at(&self, path: &str) -> PathBuf { + self.0.lock().unwrap().join(path) + } + } + + impl crate::Services for InFolder { + fn read_file(&self, path: &str) -> std::io::Result> { + std::fs::read(self.at(path)) + } + fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + std::fs::write(self.at(path), data) + } + fn full_path(&self, path: &str) -> String { + self.at(path).to_string_lossy().into_owned() + } + } + + /// A script run from a relative path runs the same file again from Recent Script Files after + /// the working directory changes, and two scripts with the same file name show their paths. + #[test] + fn recent_script_files_keep_the_file_that_ran() { + let (mut s, root) = session_in_tmp("recent-relative"); + let (a, b) = (root.join("a"), root.join("b")); + for (dir, code) in [(&a, "a"), (&b, "b")] { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(dir.join("Tool.jsx"), code).unwrap(); + } + let services = Arc::new(InFolder(std::sync::Mutex::new(a.clone()))); + s.services = services.clone(); + assert_eq!(s.execute("file.runScript", json!({"path": "Tool.jsx"})).unwrap()["ran"], "a"); + *services.0.lock().unwrap() = b.clone(); + assert_eq!(s.execute("file.runScript", json!({"recent": 0})).unwrap()["ran"], "a"); + let (e, _) = crate::menus::dynamic(&s, "recentScripts", &Default::default()); + assert_eq!(e.iter().map(|e| e.label.as_str()).collect::>(), ["Tool.jsx"]); + assert_eq!(s.execute("file.runScript", json!({"path": "Tool.jsx"})).unwrap()["ran"], "b"); + let (e, _) = crate::menus::dynamic(&s, "recentScripts", &Default::default()); + let (a, b) = (a.join("Tool.jsx").to_string_lossy().into_owned(), b.join("Tool.jsx").to_string_lossy().into_owned()); + assert_eq!(e.iter().map(|e| e.label.as_str()).collect::>(), [b.as_str(), a.as_str()]); + let _ = std::fs::remove_dir_all(&root); + } + + /// Native script paths are kept absolute. + #[test] + fn native_full_paths_are_absolute() { + use crate::Services; + assert!(std::path::Path::new(&crate::FsServices.full_path("Tool.jsx")).is_absolute()); + } + #[test] fn only_scripts_are_read_or_removed() { // A settings file in the Scripts folder that is not a script (.jsx / .js). diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 382534a44..ec63c51ee 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -132,6 +132,12 @@ pub trait Services: Send + Sync { fn is_dir(&self, path: &str) -> bool { std::path::Path::new(path).is_dir() } + /// `path` as a list of recent files keeps it (File ▸ Scripts ▸ Recent Script Files): a + /// relative path is made absolute against the working directory, so it opens the same file + /// after the working directory changes. + fn full_path(&self, path: &str) -> String { + std::path::absolute(path).map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|_| path.to_string()) + } } /// Native filesystem. diff --git a/crates/engine/src/menus.rs b/crates/engine/src/menus.rs index ded26956f..df07c633b 100644 --- a/crates/engine/src/menus.rs +++ b/crates/engine/src/menus.rs @@ -61,6 +61,12 @@ fn file_name(p: &str) -> String { std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| p.to_string()) } +/// The labels of a recent-files list: `names[i]`, or the whole of `full[i]` when another entry +/// has the same name, so entries for two files named `Tool.jsx` tell them apart. +fn distinct_labels(names: Vec, full: &[String]) -> Vec { + names.iter().zip(full).map(|(n, f)| if names.iter().filter(|m| *m == n).count() > 1 { f.clone() } else { n.clone() }).collect() +} + fn dyn_entry(label: String, command: &str, params: Value) -> MenuEntry { MenuEntry { label, command: command.into(), params, shortcut: None, alt_shortcuts: vec![] } } @@ -80,23 +86,26 @@ fn holder_of(s: &Session, keys: &str) -> Option { pub fn dynamic(s: &Session, name: &str, cx: &DynCtx) -> (Vec, Option<&'static str>) { use serde_json::json; let indexed = |list: &[String], cmd: &str, stem: bool| -> Vec { - list.iter() - .enumerate() - .map(|(i, p)| { - let n = - if stem { std::path::Path::new(p).file_stem().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| p.clone()) } else { file_name(p) }; - dyn_entry(n, cmd, json!({"index": i})) - }) - .collect() + let names = list + .iter() + .map( + |p| if stem { std::path::Path::new(p).file_stem().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| p.clone()) } else { file_name(p) }, + ) + .collect(); + distinct_labels(names, list).into_iter().enumerate().map(|(i, n)| dyn_entry(n, cmd, json!({"index": i}))).collect() }; match name { "recentProjects" => (indexed(&s.prefs.recent_projects, "file.openRecent", false), Some("No Recent Projects")), "recentFootage" => (indexed(&s.prefs.recent_footage, "file.importRecent", false), Some("No Recent Footage")), "recentPresets" => (indexed(&s.prefs.recent_presets, "anim.applyRecentPreset", true), Some("No Recent Presets")), - "recentScripts" => ( - s.prefs.recent_scripts.iter().enumerate().map(|(i, r)| dyn_entry(file_name(r.text()), "file.runScript", json!({"recent": i}))).collect(), - Some("No Recent Script Files"), - ), + "recentScripts" => { + let full: Vec = s.prefs.recent_scripts.iter().map(|r| r.text().to_string()).collect(); + let names = full.iter().map(|p| file_name(p)).collect(); + ( + distinct_labels(names, &full).into_iter().enumerate().map(|(i, n)| dyn_entry(n, "file.runScript", json!({"recent": i}))).collect(), + Some("No Recent Script Files"), + ) + } "history" => ( s.history.undo.iter().rev().enumerate().map(|(i, (l, _))| dyn_entry(format!("Undo {l}"), "edit.history", json!({"steps": i + 1}))).collect(), Some("No History"), diff --git a/crates/engine/src/tests_m145.rs b/crates/engine/src/tests_m145.rs index 68b897617..07bcba4a4 100644 --- a/crates/engine/src/tests_m145.rs +++ b/crates/engine/src/tests_m145.rs @@ -708,6 +708,11 @@ fn dynamic_menus_and_window_viewer_labels() { s.prefs.push_recent("/p/One.ecproj"); let (e, _) = crate::menus::dynamic(&s, "recentProjects", &Default::default()); assert_eq!((e[0].label.as_str(), e[0].command.as_str()), ("One.ecproj", "file.openRecent")); + // Two projects with the same file name show their paths. + s.prefs.push_recent("/q/One.ecproj"); + s.prefs.push_recent("/p/Two.ecproj"); + let (e, _) = crate::menus::dynamic(&s, "recentProjects", &Default::default()); + assert_eq!(e.iter().map(|e| e.label.as_str()).collect::>(), ["Two.ecproj", "/q/One.ecproj", "/p/One.ecproj"]); assert_eq!(crate::menus::dynamic(&s, "history", &Default::default()).0.len(), s.history.undo.len()); let a = solid(&mut s, "#ff0000"); s.state.selected_layers = vec![LayerId(a)];