mirror of
https://github.com/storytold/filmcraft.git
synced 2026-10-30 21:17:36 -06:00
#266: quitting with unsaved changes asks to save first
Problem: Quit, Cmd+Q or the window's close button exited at once with unsaved changes, leaving them to the recovery journal, so every launch after an unsaved session offered a recovery (docs/project-files.md: no "Save changes?" prompt on quit yet). Fix: a close request with unsaved changes is cancelled and the existing Save Project prompt asks Cancel / Don't Save / Save (automation ids quit.*), as in Premiere. Don't Save discards the changes and their recovery snapshot; Save on a never-saved project (here and in Close Project) opens the Save dialog and returns to the prompt when cancelled. The control channel's app.quit still quits without asking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
2140d9c9ab
commit
1580ebb520
@@ -429,6 +429,8 @@ pub fn handle(app: &mut FilmcraftApp, ctx: &egui::Context, req: &ControlRequest)
|
||||
ok(Value::Null)
|
||||
}
|
||||
"app.quit" => {
|
||||
// scripted quits don't wait on the save prompt; unsaved changes stay in the recovery journal
|
||||
app.quit_confirmed = true;
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::Close);
|
||||
ok(Value::Null)
|
||||
}
|
||||
|
||||
@@ -291,6 +291,9 @@ pub struct FilmcraftApp {
|
||||
/// Fault injection for robustness tests: the next UI pass panics.
|
||||
#[doc(hidden)]
|
||||
pub panic_next_frame: bool,
|
||||
/// The next close request quits without asking to save (Save / Don't Save was chosen in the
|
||||
/// quit prompt, or the control channel's `app.quit`).
|
||||
pub quit_confirmed: bool,
|
||||
fonts_ready: bool,
|
||||
pub integrated_titlebar: bool,
|
||||
pub last_timeline_width: f32,
|
||||
@@ -511,6 +514,7 @@ impl FilmcraftApp {
|
||||
styled: false,
|
||||
ui_error: None,
|
||||
panic_next_frame: false,
|
||||
quit_confirmed: false,
|
||||
fonts_ready: false,
|
||||
integrated_titlebar: false,
|
||||
last_timeline_width: 1000.0,
|
||||
@@ -1882,6 +1886,10 @@ impl eframe::App for FilmcraftApp {
|
||||
self.playback.hidden = true;
|
||||
}
|
||||
self.timeline_still = if self.ui.timeline.animating() { 0 } else { self.timeline_still.saturating_add(1) };
|
||||
// Quitting with unsaved changes asks to save them first.
|
||||
if ctx.input(|i| i.viewport().close_requested()) && panels::clip_dialogs::intercept_quit(self) {
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose);
|
||||
}
|
||||
let had_synthetic = !self.synthetic.is_empty();
|
||||
self.drain_control(ctx);
|
||||
if !self.synthetic.is_empty() && !had_synthetic {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Edit / Clip / File menu dialogs (M3.10): Paste Attributes, Remove Attributes, Offline File,
|
||||
//! Make Subclip, Edit Subclip, Modify ▸ Audio Channels, Modify ▸ Timecode, Frame Hold Options,
|
||||
//! Field Options, Clip Speed / Duration, Nest… (Nested Sequence Name), and the Close Project
|
||||
//! "save changes?" prompt.
|
||||
//! Field Options, Clip Speed / Duration, Nest… (Nested Sequence Name), and the Close Project and
|
||||
//! Quit "save changes?" prompts.
|
||||
//!
|
||||
//! The open dialog lives in `UiState::clip_dialog` (serde): the engine command it runs and the
|
||||
//! parameters being edited — the same JSON the command takes — so agents can open a dialog from
|
||||
@@ -26,7 +26,9 @@
|
||||
//! - Field Options `fieldOptions.*`: `reverseFieldDominance`, `processing.<none|alwaysDeinterlace|flickerRemoval>`;
|
||||
//! - Clip Speed / Duration `speedDuration.*`: `speed`, `reverse`, `ripple`,
|
||||
//! `interpolation.<frameSampling|frameBlending|opticalFlow>`;
|
||||
//! - Close Project `closeProject.save`, `closeProject.dontSave`, `closeProject.cancel`.
|
||||
//! - Close Project `closeProject.save`, `closeProject.dontSave`, `closeProject.cancel`;
|
||||
//! - Quit (asked when the window is closed with unsaved changes, see [`intercept_quit`])
|
||||
//! `quit.save`, `quit.dontSave`, `quit.cancel`.
|
||||
|
||||
use egui::{Align2, RichText};
|
||||
use filmcraft_project::{AudioChannelMap, AudioChannels, ItemKind, TrackKind};
|
||||
@@ -56,6 +58,7 @@ fn meta(command: &str) -> Option<(&'static str, &'static str)> {
|
||||
"clip.fieldOptions" => (tl!("Field Options"), "fieldOptions"),
|
||||
"clip.speedDuration" => (tl!("Clip Speed / Duration"), "speedDuration"),
|
||||
"file.closeProject" => (tl!("Save Project"), "closeProject"),
|
||||
"app.quit" => (tl!("Save Project"), "quit"),
|
||||
_ => return None,
|
||||
})
|
||||
}
|
||||
@@ -89,7 +92,7 @@ pub fn route(app: &mut FilmcraftApp, id: &str, params: &Value) -> Option<Result<
|
||||
if id == "file.saveAll" && app.session.path.is_none() {
|
||||
return Some(app.file_dialog("file.save", params));
|
||||
}
|
||||
if id == "file.closeProject" && !app.session.is_dirty() {
|
||||
if (id == "file.closeProject" && !app.session.is_dirty()) || id == "app.quit" {
|
||||
return None;
|
||||
}
|
||||
meta(id)?;
|
||||
@@ -102,6 +105,19 @@ pub fn route(app: &mut FilmcraftApp, id: &str, params: &Value) -> Option<Result<
|
||||
Some(Ok(json!({"dialog": meta(id).map(|m| m.1)})))
|
||||
}
|
||||
|
||||
/// A request to close the window (Quit, the title bar's close button, the OS): with unsaved
|
||||
/// changes it is cancelled and Save / Don't Save / Cancel is asked first, as in Premiere (#266).
|
||||
/// Returns whether the close must be cancelled.
|
||||
pub fn intercept_quit(app: &mut FilmcraftApp) -> bool {
|
||||
if app.quit_confirmed || !app.session.is_dirty() {
|
||||
return false;
|
||||
}
|
||||
if app.ui.clip_dialog.as_ref().is_none_or(|d| d.command != "app.quit") {
|
||||
app.ui.clip_dialog = Some(ClipDialogDraft { command: "app.quit".into(), params: json!({}), info: Value::Null, error: String::new() });
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Initial parameters (and display info) of a command's dialog.
|
||||
fn defaults(app: &FilmcraftApp, id: &str) -> (Value, Value) {
|
||||
let s = &app.session;
|
||||
@@ -290,7 +306,8 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) {
|
||||
let mut elems: Elems = Vec::new();
|
||||
let mut action: Option<&'static str> = None;
|
||||
let project_name = app.session.project.name.clone();
|
||||
let can_save = app.session.path.is_some();
|
||||
// a never-saved project is saved through the Save dialog
|
||||
let can_save = app.session.path.is_some() || app.hooks.pick_save.is_some();
|
||||
// the title is translated; the window keeps one id whatever the interface language
|
||||
let id = egui::Id::new(("clip-dialog", pre));
|
||||
egui::Window::new(title).id(id).collapsible(false).resizable(false).default_width(360.0).anchor(Align2::CENTER_CENTER, [0.0, 0.0]).show(ctx, |ui| {
|
||||
@@ -520,7 +537,7 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) {
|
||||
}
|
||||
}
|
||||
}
|
||||
"file.closeProject" => {
|
||||
"file.closeProject" | "app.quit" => {
|
||||
ui.label(tlf!("Save changes to “{project_name}” before closing?", project_name));
|
||||
ui.add_space(8.0);
|
||||
ui.horizontal(|ui| {
|
||||
@@ -586,16 +603,33 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) {
|
||||
}
|
||||
}
|
||||
Some("dontSave") | Some("save") => {
|
||||
if action == Some("save")
|
||||
&& let Err(e) = app.session.execute("file.save", json!({}))
|
||||
{
|
||||
app.ui.status = e.to_string();
|
||||
app.ui.clip_dialog = None;
|
||||
return;
|
||||
if action == Some("save") {
|
||||
let saved = if app.session.path.is_some() {
|
||||
app.session.execute("file.save", json!({})).map(|_| ()).map_err(|e| e.to_string())
|
||||
} else {
|
||||
app.file_dialog("file.save", &json!({})).map(|_| ())
|
||||
};
|
||||
if let Err(e) = saved {
|
||||
app.ui.status = e;
|
||||
app.ui.clip_dialog = None;
|
||||
return;
|
||||
}
|
||||
// the Save dialog was cancelled: back to the prompt
|
||||
if app.session.is_dirty() {
|
||||
app.ui.clip_dialog = Some(d);
|
||||
return;
|
||||
}
|
||||
}
|
||||
if let Err(e) = app.session.execute("file.closeProject", json!({"force": true})) {
|
||||
// Quit ▸ Don't Save discards the changes: closing the project also clears their recovery
|
||||
// snapshot, so the next launch doesn't offer them back
|
||||
let close = d.command != "app.quit" || app.session.is_dirty();
|
||||
if close && let Err(e) = app.session.execute("file.closeProject", json!({"force": true})) {
|
||||
app.ui.status = e.to_string();
|
||||
}
|
||||
if d.command == "app.quit" {
|
||||
app.quit_confirmed = true;
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::Close);
|
||||
}
|
||||
app.ui.clip_dialog = None;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -88,6 +88,25 @@ impl Driver {
|
||||
fn item_named(&mut self, name: &str) -> u64 {
|
||||
self.app().session.project.items.values().find(|i| i.name == name).unwrap().id.0
|
||||
}
|
||||
|
||||
/// The window is asked to close (Quit, Cmd+Q, its close button) for one frame.
|
||||
fn request_close(&mut self) {
|
||||
self.harness.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
|
||||
self.frames(1);
|
||||
}
|
||||
|
||||
/// The app sent `cmd` to the window in the last frame.
|
||||
fn sent(&self, cmd: &egui::ViewportCommand) -> bool {
|
||||
self.harness.output().viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(cmd))
|
||||
}
|
||||
|
||||
/// Make an unsaved change.
|
||||
fn edit(&mut self) {
|
||||
let c = self.v1(0);
|
||||
self.exec("timeline.select", json!({"clips": [c.id.0]}));
|
||||
self.menu("edit.label.yellow");
|
||||
assert!(self.app().session.is_dirty());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -269,3 +288,56 @@ fn speed_duration_from_the_clip_menu_opens_the_dialog() {
|
||||
assert_eq!(got.speed, 2.0);
|
||||
assert_eq!(got.time_interpolation, filmcraft_project::TimeInterpolation::FrameBlending);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn quitting_with_unsaved_changes_asks_to_save() {
|
||||
let mut d = Driver::new();
|
||||
d.edit();
|
||||
// the close is cancelled and the prompt asks instead
|
||||
d.request_close();
|
||||
assert!(d.sent(&egui::ViewportCommand::CancelClose), "the window must stay open");
|
||||
d.frames(2);
|
||||
assert!(d.has("quit.save") && d.has("quit.dontSave") && d.has("quit.cancel"));
|
||||
// Cancel keeps the app and the changes
|
||||
d.click("quit.cancel");
|
||||
assert!(d.app().ui.clip_dialog.is_none());
|
||||
assert!(d.app().session.is_dirty() && !d.app().quit_confirmed);
|
||||
d.request_close();
|
||||
assert!(d.sent(&egui::ViewportCommand::CancelClose), "asks again");
|
||||
d.frames(2);
|
||||
// Don't Save discards the changes (and their recovery snapshot) and closes the window
|
||||
d.click("quit.dontSave");
|
||||
assert!(d.app().quit_confirmed && !d.app().session.is_dirty());
|
||||
assert!(d.app().ui.clip_dialog.is_none());
|
||||
d.request_close();
|
||||
assert!(!d.sent(&egui::ViewportCommand::CancelClose), "the confirmed quit goes through");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn quit_save_on_a_new_project_uses_the_save_dialog_and_scripted_quit_does_not_ask() {
|
||||
let path = std::env::temp_dir().join(format!("fc-quit-save-{}.fcproj", std::process::id()));
|
||||
let _ = std::fs::remove_file(&path);
|
||||
let mut d = Driver::new();
|
||||
d.app().session.path = None;
|
||||
d.edit();
|
||||
let picked = path.to_string_lossy().into_owned();
|
||||
let mut answers = vec![None, Some(picked)];
|
||||
d.app().hooks.pick_save = Some(Box::new(move |_| if answers.is_empty() { None } else { answers.remove(0) }));
|
||||
d.request_close();
|
||||
d.frames(2);
|
||||
// the Save dialog is cancelled: back to the prompt, nothing quits
|
||||
d.click("quit.save");
|
||||
assert!(d.app().ui.clip_dialog.is_some() && !d.app().quit_confirmed);
|
||||
// saved: the project is written and the window closes
|
||||
d.click("quit.save");
|
||||
assert!(d.app().quit_confirmed && !d.app().session.is_dirty());
|
||||
assert!(path.exists(), "saved to the chosen file");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
d.request_close();
|
||||
assert!(!d.sent(&egui::ViewportCommand::CancelClose));
|
||||
// the control channel's app.quit quits without the prompt
|
||||
let mut d = Driver::new();
|
||||
d.edit();
|
||||
d.ok("app.quit", json!({}));
|
||||
assert!(d.app().quit_confirmed);
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@ Methods (handlers in `crates/ui-egui/src/control.rs`):
|
||||
| `ui.timeline.hit` / `ui.timeline.locate` | `{x,y,modifiers?}` / `{clip, edge?}` | timeline hit-testing. `hit` reports a clip's `edge` (`In`/`Out`) within 7 px of it (a third of a narrow clip); just left of a cut that is the left clip's Out, from the cut rightwards the right clip's In. `kind` is the trim a press of the current tool with `modifiers` starts there (`trim`, `ripple`, `roll`, `rateStretch`, `remix`), `volume` on an audio clip's Volume line (Selection and Pen tools), or null |
|
||||
| `ui.playback` | `{action: play|stop|toggle, speed?}` | |
|
||||
| `ui.screenshot` | `{path?, panel?}` | PNG of the window or one panel; fails after 10 s when no frame is presented (window hidden, display asleep) |
|
||||
| `ui.resize`, `ui.focus`, `app.quit` | | `ui.focus` is the only request that activates the app and takes keyboard focus |
|
||||
| `ui.resize`, `ui.focus`, `app.quit` | | `ui.focus` is the only request that activates the app and takes keyboard focus. `app.quit` quits without the save prompt (unsaved changes stay in the recovery journal); the command `app.quit` (Quit, Cmd+Q) asks first |
|
||||
| `perf.stats` | – | performance counters (also the command `perf.stats`, so `engine.execute` and MCP `command_run` reach it; headless sessions return the engine part). `decode`: GOP-cache requests / hits / `cacheHitRate`, decoder seeks, samples decoded and skipped while catching up, `draftFrames` (decoded in draft mode), `h264Threads` (frame-threading workers per H.264 decoder), `liveDecoders` (sources holding a decoder now), `cacheMB` / `cacheBudgetMB` (decoded frames all sources hold, and the shared budget), `planePoolMB` / `planesReused` (idle buffers of the frame pool, the planes of evicted frames and the compositor's float images, which a standalone export frees when it ends; and buffers taken from it instead of allocated), decoder ms (total and per sample), `framesDecoded`, `hardware` (Settings ▸ Playback ▸ Hardware decoding: `enabled`, pictures from hardware decoders `frames` vs `softwareFrames`, `sessions`, `declined` streams, mid-stream `fallbacks`); `playback`: playing, shown / dropped frames and `dropRate` of the current or last play, resolution, `draftDecode` (Settings ▸ Playback ▸ Draft decoding), `audio` (desktop: sound is mixed 200 ms ahead of the device; device `callbacks`, `underruns` and `missingFrames` played as silence because the mixer fell behind, `minLeadMs` the least sound left buffered); `frames`: frame-worker jobs, cancelled, `requestHitRate`, `decodeMs` / `renderMs` (mean, p50, p95 of the last 256 jobs), queue, render-cost estimate, cache use; `monitor`: the `frame` and `revision` of the picture the Program monitor shows against `playheadFrame` and `projectRevision` (how far a drag or a scrub is ahead of the picture); `ui`: fps and frame ms; `process`: CPU seconds; `media`, `jobs`. Counters are cumulative: diff two readings to measure an interval ([performance.md](performance.md)) |
|
||||
|
||||
**Timeline thumbnails.** `ui.set {timeline: {thumbnails}}` sets the Timeline wrench menu's Show Video
|
||||
|
||||
@@ -152,9 +152,11 @@ Premiere's auto-save can lose up to one interval of work. FilmCraft also keeps a
|
||||
over the original file. The recovered state goes into the new session's own journal before the old
|
||||
journal is deleted.
|
||||
- A normal quit with nothing unsaved removes the session directory. Quitting with unsaved changes
|
||||
keeps the snapshot, marked `cleanExit`, and FilmCraft offers it on the next launch ("FilmCraft was
|
||||
closed while … had unsaved changes"). There is no "Save changes?" prompt on quit yet, so this
|
||||
keeps the work.
|
||||
(Quit, Cmd+Q or the window's close button) first asks *"Save changes to 'X' before closing?"*
|
||||
with **Cancel**, **Don't Save** and **Save** (Save on a never-saved project opens the Save dialog);
|
||||
Don't Save discards the changes and their snapshot. A quit that does not ask (the control
|
||||
channel's `app.quit`, the OS ending the session) keeps the snapshot, marked `cleanExit`, and FilmCraft offers it on the next launch ("FilmCraft was
|
||||
closed while … had unsaved changes").
|
||||
|
||||
Desktop flags: `--recover` recovers the newest candidate without asking. `--no-recover` starts
|
||||
without asking; the changes stay available through File ▸ Recover Unsaved Changes…. `--data-dir
|
||||
|
||||
Reference in New Issue
Block a user