mirror of
https://github.com/storytold/filmcraft.git
synced 2026-10-14 19:26:59 -04:00
* Web: Link Media Locate… links the chosen file to the selected missing clip (#111) Link Media ▸ Locate…, `media.attachProxies` and `media.reconnectFullRes` all need the chosen file's path back synchronously so they can call the engine with `item` + `path`. The existing `pick_files` hook imports every chosen file as a new project item, so on the web build (where the browser picker is asynchronous and the hook returned `Vec::new()`) the command ran without a path and the file silently landed as a new project item instead of being relinked. On native the same hook worked because rfd returns paths synchronously. Add a separate `pick_file_for_relink` hook that returns the chosen file's virtual path synchronously. Native hosts open an rfd dialog; the web host registers the file in `/files/<name>` via a pending slot and runs `media.relink` itself when the picker resolves. The three call sites prefer the new hook and fall back to `pick_files` when the host has not installed it (so the import flow used by every other dialog stays unchanged). Fixes #111 Co-Authored-By: ninso112 <[email protected]> * Co-ship: Luna161 ack'd and reviewed fix/111-link-media-locate Reviewed the pick_file_for_relink hook routing, the take_files relink path, and the regression test. Looks correct end to end; ready for upstream review. Co-Authored-By: ninso112 <[email protected]> * #111: relink hint names its command; no pending state after a cancelled picker - RelinkHint carries the command (media.relink / attachProxies / reconnectFullRes), so the web host no longer runs media.relink for Attach Proxies and Reconnect Full Resolution. - The web picker hands the hint to its own callback instead of a thread-local slot, so a cancelled picker can't turn the next ordinary import into a relink. - Replace the hook-only unit tests with a UI test that drives Link Media ▸ Locate…. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: ninso112 <[email protected]> Co-authored-by: Brandon Thomas <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
5 files changed
+130
-8
No files matched your search
@@ -10,7 +10,7 @@ use std::cell::RefCell;
|
||||
use std::rc::Rc;
|
||||
|
||||
use filmcraft_engine::Services;
|
||||
use filmcraft_ui_egui::FilmcraftApp;
|
||||
use filmcraft_ui_egui::{FilmcraftApp, RelinkHint};
|
||||
use serde_json::{Value, json};
|
||||
use wasm_bindgen::JsCast;
|
||||
use wasm_bindgen::prelude::*;
|
||||
@@ -153,12 +153,43 @@ pub fn take_files(files: Vec<web_sys::File>) {
|
||||
});
|
||||
}
|
||||
|
||||
/// Files a picker returned: imported, or with a relink hint, the first one is registered under
|
||||
/// `/files/<name>` and the hint's command runs on it (#111).
|
||||
fn picked(files: Vec<web_sys::File>, relink: Option<RelinkHint>) {
|
||||
let Some(hint) = relink else { return take_files(files) };
|
||||
let Some(f) = files.into_iter().next() else { return };
|
||||
wasm_bindgen_futures::spawn_local(async move {
|
||||
let path = fs::register_blob(&f.name(), f.clone().into());
|
||||
crate::recovery::keep_media(&path, &f);
|
||||
let mut params = hint.params;
|
||||
if !params.is_object() {
|
||||
params = json!({});
|
||||
}
|
||||
params["path"] = json!(path);
|
||||
let command = hint.command;
|
||||
let outcome = on_ui(move |w, _| w.app.session.execute(&command, params).map_err(|e| e.to_string())).await;
|
||||
crate::post(move |w, _| match outcome {
|
||||
Ok(v) => {
|
||||
if let Some(n) = v.get("relinked").and_then(Value::as_array).map(Vec::len) {
|
||||
w.app.status(format!("Linked {n} clip(s)"));
|
||||
}
|
||||
}
|
||||
Err(e) => w.app.status(e),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
fn has_fsa_picker() -> bool {
|
||||
web_sys::window().is_some_and(|w| js_sys::Reflect::has(&w, &"showOpenFilePicker".into()).unwrap_or(false))
|
||||
}
|
||||
|
||||
/// Let the user pick files: `showOpenFilePicker` when available, else a file input.
|
||||
pub fn pick(exts: &[&str], multiple: bool) {
|
||||
pick_then(exts, multiple, None);
|
||||
}
|
||||
|
||||
/// [`pick`], handing the files to [`picked`]: a cancelled picker leaves nothing pending.
|
||||
fn pick_then(exts: &[&str], multiple: bool, relink: Option<RelinkHint>) {
|
||||
let accept: Vec<String> = exts.iter().map(|e| format!(".{e}")).collect();
|
||||
if has_fsa_picker() {
|
||||
let opts = js_sys::Object::new();
|
||||
@@ -178,7 +209,7 @@ pub fn pick(exts: &[&str], multiple: bool) {
|
||||
files.push(f);
|
||||
}
|
||||
}
|
||||
take_files(files);
|
||||
picked(files, relink);
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -190,7 +221,7 @@ pub fn pick(exts: &[&str], multiple: bool) {
|
||||
let inp = input.clone();
|
||||
let on_change = Closure::<dyn FnMut()>::new(move || {
|
||||
if let Some(list) = inp.files() {
|
||||
take_files((0..list.length()).filter_map(|i| list.get(i)).collect());
|
||||
picked((0..list.length()).filter_map(|i| list.get(i)).collect(), relink.clone());
|
||||
}
|
||||
});
|
||||
input.set_onchange(Some(on_change.as_ref().unchecked_ref()));
|
||||
@@ -231,4 +262,12 @@ pub fn install_hooks(app: &mut FilmcraftApp) {
|
||||
// Saving writes the file in memory and offers it as a download (`fs::WebServices`).
|
||||
app.hooks.pick_save = Some(Box::new(|name: &str| Some(format!("/projects/{name}"))));
|
||||
app.hooks.pick_save_as = Some(Box::new(|_filter: &str, _exts: &[&str], name: &str| Some(format!("/exports/{name}"))));
|
||||
// Link Media ▸ Locate…, Attach Proxies, Reconnect Full Resolution: the picker is async, so the
|
||||
// hint's command runs once the user has chosen (#111).
|
||||
app.hooks.pick_file_for_relink = Some(Box::new(|exts: &[&str], hint: Option<RelinkHint>| {
|
||||
if hint.is_some() {
|
||||
pick_then(exts, false, hint);
|
||||
}
|
||||
None
|
||||
}));
|
||||
}
|
||||
@@ -170,6 +170,9 @@ fn main() -> eframe::Result {
|
||||
app.hooks.pick_files = Some(Box::new(|exts: &[&str]| {
|
||||
rfd::FileDialog::new().add_filter("Media", exts).pick_files().unwrap_or_default().into_iter().map(|p| p.to_string_lossy().to_string()).collect()
|
||||
}));
|
||||
// Link Media ▸ Locate…, Attach Proxies, Reconnect Full Resolution: one path, not imported.
|
||||
app.hooks.pick_file_for_relink =
|
||||
Some(Box::new(|exts: &[&str], _hint| rfd::FileDialog::new().add_filter("Media", exts).pick_file().map(|p| p.to_string_lossy().to_string())));
|
||||
app.hooks.pick_save = Some(Box::new(|name: &str| {
|
||||
rfd::FileDialog::new().add_filter("FilmCraft Project", &["fcproj"]).set_file_name(name).save_file().map(|p| p.to_string_lossy().to_string())
|
||||
}));
|
||||
|
||||
@@ -88,6 +88,11 @@ pub struct HostHooks {
|
||||
pub pick_open_file: Option<Box<dyn FnMut(&str, &[&str]) -> Option<String>>>,
|
||||
/// Folder picker (Link Media search, proxy and Project Manager destinations).
|
||||
pub pick_folder: Option<Box<dyn FnMut() -> Option<String>>>,
|
||||
/// Pick one file for a command that relinks to it (Link Media ▸ Locate…, Attach Proxies,
|
||||
/// Reconnect Full Resolution) instead of importing it, as [`Self::pick_files`] does. Native
|
||||
/// hosts return the path. A host whose picker is asynchronous (the web) returns `None` and
|
||||
/// runs `hint.command` with `hint.params` plus `"path"` itself once the user has chosen.
|
||||
pub pick_file_for_relink: Option<Box<dyn FnMut(&[&str], Option<RelinkHint>) -> Option<String>>>,
|
||||
/// Bring the window on screen for control-channel UI requests *without* taking keyboard focus
|
||||
/// (macOS: `orderFrontRegardless`). Without it the app only requests a repaint: it never
|
||||
/// activates itself for an agent, because the user's keystrokes would land here.
|
||||
@@ -97,6 +102,16 @@ pub struct HostHooks {
|
||||
pub open_path: Option<Box<dyn FnMut(&str, bool) -> Result<(), String>>>,
|
||||
}
|
||||
|
||||
/// The command a [`HostHooks::pick_file_for_relink`] caller runs with the chosen file, for hosts
|
||||
/// that can only run it later.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct RelinkHint {
|
||||
/// `media.relink`, `media.attachProxies` or `media.reconnectFullRes`.
|
||||
pub command: String,
|
||||
/// Its parameters, without `"path"`.
|
||||
pub params: serde_json::Value,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum Dialog {
|
||||
About,
|
||||
|
||||
@@ -17,8 +17,8 @@
|
||||
use egui::{Color32, RichText};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::FilmcraftApp;
|
||||
use crate::state::{LinkMediaDraft, ProjectManagerDraft, ProxyDraft};
|
||||
use crate::{FilmcraftApp, RelinkHint};
|
||||
|
||||
type Elems = Vec<(String, egui::Rect, String)>;
|
||||
|
||||
@@ -81,9 +81,16 @@ pub fn route(app: &mut FilmcraftApp, id: &str, params: &Value) -> Option<Result<
|
||||
let item = app.session.state.project_selection.first().copied();
|
||||
let Some(item) = item else { return Some(Err("select a clip in the Project panel".into())) };
|
||||
let exts: Vec<&str> = filmcraft_media::VIDEO_EXTENSIONS.to_vec();
|
||||
let path = app.hooks.pick_files.as_mut().and_then(|f| f(&exts).into_iter().next());
|
||||
let Some(path) = path else { return Some(Ok(Value::Null)) };
|
||||
Some(app.session.execute(id, json!({"item": item.0, "path": path})).map_err(|e| e.to_string()))
|
||||
let hint = RelinkHint { command: id.to_string(), params: json!({"item": item.0}) };
|
||||
let picked = if let Some(picker) = app.hooks.pick_file_for_relink.as_mut() {
|
||||
picker(&exts, Some(hint))
|
||||
} else if let Some(picker) = app.hooks.pick_files.as_mut() {
|
||||
picker(&exts).into_iter().next()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let Some(picked) = picked else { return Some(Ok(Value::Null)) };
|
||||
Some(app.session.execute(id, json!({"item": item.0, "path": picked})).map_err(|e| e.to_string()))
|
||||
}
|
||||
"file.projectManager" => Some(enabled(app, id).map(|_| {
|
||||
let seqs = app.session.state.active_sequence.map(|s| vec![s.0]).unwrap_or_default();
|
||||
@@ -327,7 +334,17 @@ fn link_media(app: &mut FilmcraftApp, ctx: &egui::Context) {
|
||||
Some("locate") => {
|
||||
let exts: Vec<&str> =
|
||||
filmcraft_media::VIDEO_EXTENSIONS.iter().chain(filmcraft_media::AUDIO_EXTENSIONS).chain(filmcraft_media::STILL_EXTENSIONS).copied().collect();
|
||||
if let Some(path) = app.hooks.pick_files.as_mut().and_then(|f| f(&exts).into_iter().next()) {
|
||||
let mut params = match_params(&d);
|
||||
params["item"] = json!(item);
|
||||
let hint = RelinkHint { command: "media.relink".into(), params };
|
||||
let picked = if let Some(picker) = app.hooks.pick_file_for_relink.as_mut() {
|
||||
picker(&exts, Some(hint))
|
||||
} else if let Some(picker) = app.hooks.pick_files.as_mut() {
|
||||
picker(&exts).into_iter().next()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if let Some(path) = picked {
|
||||
relink(app, &mut d, path);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -258,3 +258,51 @@ fn offline_badges_proxy_toggle_and_dialogs() {
|
||||
assert!(d.app().ui.project_manager.is_none());
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
/// #111: Link Media ▸ Locate… relinks the selected clip to the chosen file instead of importing it,
|
||||
/// both with a host that returns the path and with one (the web) that runs the hint's command later.
|
||||
#[test]
|
||||
fn locate_relinks_instead_of_importing() {
|
||||
let root = tmp_dir("locate");
|
||||
let path = moved_project(&root);
|
||||
let mut s = Session::default();
|
||||
s.execute("file.open", json!({"path": path})).unwrap();
|
||||
let mut d = Driver::new(s);
|
||||
d.frames(6);
|
||||
let items = d.app().session.project.items.len();
|
||||
let imported = Arc::new(std::sync::Mutex::new(0));
|
||||
let n = imported.clone();
|
||||
d.app().hooks.pick_files = Some(Box::new(move |_| {
|
||||
*n.lock().unwrap() += 1;
|
||||
vec![]
|
||||
}));
|
||||
// an async host: nothing comes back now, the hint says what to run later
|
||||
let hints = Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
let h = hints.clone();
|
||||
d.app().hooks.pick_file_for_relink = Some(Box::new(move |_, hint| {
|
||||
h.lock().unwrap().push(hint);
|
||||
None
|
||||
}));
|
||||
d.click("linkMedia.locate");
|
||||
let hint = hints.lock().unwrap().pop().flatten().expect("the picker got a hint");
|
||||
assert_eq!(hint.command, "media.relink");
|
||||
let item = hint.params["item"].as_u64().expect("the hint names the clip");
|
||||
assert!(hint.params["match"].is_object(), "and the dialog's match options: {}", hint.params);
|
||||
// what the web host does once the user has chosen
|
||||
let file = root.join("Moved/Media/Harbour.mov").to_string_lossy().into_owned();
|
||||
let mut p = hint.params.clone();
|
||||
p["path"] = json!(file);
|
||||
d.exec(&hint.command, p);
|
||||
// a host that returns the path relinks the other clip from the dialog itself
|
||||
let other = root.join("Moved/Media/Night.mov").to_string_lossy().into_owned();
|
||||
d.app().hooks.pick_file_for_relink = Some(Box::new(move |_, _| Some(other.clone())));
|
||||
if d.app().ui.link_media.is_some() {
|
||||
d.click("linkMedia.locate");
|
||||
d.frames(4);
|
||||
}
|
||||
assert_eq!(*imported.lock().unwrap(), 0, "Locate… never goes through the import picker");
|
||||
assert_eq!(d.app().session.project.items.len(), items, "no new project item");
|
||||
let st = d.exec("media.status", json!({}));
|
||||
assert!(st.as_array().unwrap().iter().all(|m| m["status"] == "online"), "item {item}: {st}");
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
Reference in new issue
Block a user