From c2011fb11d7d0e5ef7c845f54665994576935c98 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 08:03:16 -0700 Subject: [PATCH] Scripts: Recent Script Files keeps a script's full path, and same-named entries show their paths A script run from a relative path is stored with its absolute path, so choosing it again runs the same file after the working directory changes. The web build keeps its file keys as they are (Services::full_path). Two recent scripts, projects, footage files or presets with the same file name are listed with their paths instead of the name alone. --- apps/effectcraft-web/src/files.rs | 4 ++ crates/engine/src/commands/file_more.rs | 6 ++- crates/engine/src/commands/scripts.rs | 52 +++++++++++++++++++++++++ crates/engine/src/lib.rs | 6 +++ crates/engine/src/menus.rs | 33 ++++++++++------ crates/engine/src/tests_m145.rs | 5 +++ 6 files changed, 92 insertions(+), 14 deletions(-) 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)];