From f43f654cb0c61d11510b9def674ff912200c3d89 Mon Sep 17 00:00:00 2001 From: mavrick Date: Mon, 26 Oct 2026 07:14:22 -0500 Subject: [PATCH 1/4] feat(ui): ask to save unsaved changes before quitting or replacing the project The desktop app asks "Save changes to ?" (Save, Don't Save, Cancel) before quitting, closing the window, or replacing the project with New, Open, the sample, a template or a dropped project file. Commands stay silent: the interface's dispatch points go through RustGisApp::run_by_user, which asks; scripts, the AI Assistant and attached agents call commands directly. Undo back to the state last saved now leaves nothing unsaved: the session gives each change a state, undo steps remember theirs, and Session::dirty() compares the state with the one saved. Keyboard shortcuts don't act behind a modal dialog, so Escape answers the dialog rather than switching tools. Closes #906. Co-Authored-By: Claude Opus 5.5 --- apps/rustgis-web/src/web.rs | 2 +- apps/rustgis/src/main.rs | 2 +- crates/engine/src/cmd/design.rs | 2 +- crates/engine/src/cmd/history.rs | 2 +- crates/engine/src/cmd/project.rs | 6 +- crates/engine/src/lib.rs | 70 ++- crates/engine/src/sample.rs | 2 +- crates/engine/src/tests.rs | 123 ++++-- crates/engine/src/tests/task_reentry.rs | 4 +- crates/engine/tests/portable_projects.rs | 4 +- .../engine/tests/portable_projects/repair.rs | 2 +- crates/engine/tests/project_save.rs | 10 +- crates/ui-egui/src/autosave.rs | 8 +- crates/ui-egui/src/backstage.rs | 7 +- crates/ui-egui/src/chrome.rs | 14 +- crates/ui-egui/src/dialogs.rs | 26 +- crates/ui-egui/src/lib.rs | 30 +- crates/ui-egui/src/panes.rs | 4 +- crates/ui-egui/src/saveprompt.rs | 400 ++++++++++++++++++ crates/ui-egui/src/widgets.rs | 8 +- crates/ui-egui/tests/autosave.rs | 2 +- crates/ui-egui/tests/smoke.rs | 8 +- docs/user-guide/workspace.md | 18 +- 23 files changed, 668 insertions(+), 86 deletions(-) create mode 100644 crates/ui-egui/src/saveprompt.rs diff --git a/apps/rustgis-web/src/web.rs b/apps/rustgis-web/src/web.rs index 84129246a..9c81df1bc 100644 --- a/apps/rustgis-web/src/web.rs +++ b/apps/rustgis-web/src/web.rs @@ -444,7 +444,7 @@ impl eframe::App for WebShell { embed.exchange(&mut self.app, ctx); } self.app.logic(ctx); - self.dirty.set(self.app.session.dirty); + self.dirty.set(self.app.session.dirty()); // The shortcuts can be changed in the app: the key listener follows, rebuilt only when // the keymap changed. let print = self.app.shortcuts_fingerprint(); diff --git a/apps/rustgis/src/main.rs b/apps/rustgis/src/main.rs index ef6dd9a66..7be44bb67 100644 --- a/apps/rustgis/src/main.rs +++ b/apps/rustgis/src/main.rs @@ -391,7 +391,7 @@ fn main() -> eframe::Result { for e in open_files(&mut app, &files) { eprintln!("rustgis: {e}"); } - app.session.dirty = false; + app.session.mark_saved(); let mut viewport = egui::ViewportBuilder::default() .with_title("RustGIS") .with_app_id("org.rustgis.RustGIS") diff --git a/crates/engine/src/cmd/design.rs b/crates/engine/src/cmd/design.rs index 4caf16dfd..eaa2b0503 100644 --- a/crates/engine/src/cmd/design.rs +++ b/crates/engine/src/cmd/design.rs @@ -275,7 +275,7 @@ pub fn commands() -> Vec { .iter() .filter_map(|(id, c)| s.map().layer(*id).map(|l| json!({"layer": l.name, "added": c.added, "modified": c.modified, "deleted": c.deleted}))) .collect(); - Ok(json!({"unsaved": s.dirty, "layers": rows})) + Ok(json!({"unsaved": s.dirty(), "layers": rows})) }) .view(), C::new("edit.template", "Manage Templates", "Edit › Manage Edits", template).when(need_feature_layer).params( diff --git a/crates/engine/src/cmd/history.rs b/crates/engine/src/cmd/history.rs index df6cb72ca..157c3ec80 100644 --- a/crates/engine/src/cmd/history.rs +++ b/crates/engine/src/cmd/history.rs @@ -50,7 +50,7 @@ fn script(s: &mut Session, _: &Value) -> CmdResult { // there until the project is saved again. if s.path.is_none() { text.push_str("# The project hasn't been saved yet: save it as this file first.\n"); - } else if s.dirty { + } else if s.dirty() { text.push_str("# The project has changes that aren't saved: save it first, so its layers are there.\n"); } text.push_str(&format!("import rustgis\n\ns = rustgis.Session({})\n", quoted(&project_file(s)))); diff --git a/crates/engine/src/cmd/project.rs b/crates/engine/src/cmd/project.rs index 3f0da190d..52204d4c3 100644 --- a/crates/engine/src/cmd/project.rs +++ b/crates/engine/src/cmd/project.rs @@ -140,10 +140,12 @@ fn open_text(s: &mut Session, p: &Value) -> CmdResult { Ok(Value::Null) } +/// Saves the project to `path` (`.rgis` added when it has no extension) and makes it the +/// project's file. The project as it is now is the saved state undo can return to. fn write(s: &mut Session, path: PathBuf) -> CmdResult { let path = if path.extension().is_none() { path.with_extension(crate::project::EXTENSION) } else { path }; let stripped = s.project.save(&path)?; - s.dirty = false; + s.mark_saved(); s.edit_counts.clear(); s.notify(format!("Saved {}", path.display()), false); if stripped > 0 { @@ -295,7 +297,7 @@ fn info(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({ "name": s.project.name, "path": s.path.as_ref().map(|p| p.display().to_string()), - "unsaved": s.dirty, + "unsaved": s.dirty(), "maps": s.project.maps.iter().map(|m| m.name.clone()).collect::>(), "activeMap": m.name, "basemap": m.basemap.code(), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5496c90ca..c9dcc0e54 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -416,8 +416,9 @@ pub struct Notice { const UNDO_LIMIT: usize = 40; -/// An undo or redo step: the project before it, the layout open then, its label and who made it. -type Step = (Project, Option, &'static str, Option); +/// An undo or redo step: the project before it, the layout open then, its label, who made it, +/// and which state of the project it is ([`Session::dirty`]). +type Step = (Project, Option, &'static str, Option, u64); /// What a project holds in the session beside itself: what [`Session::clear_history`] forgets /// when another project starts, and what opening one sets (its file, layer, tool…). When another @@ -436,7 +437,8 @@ struct PutAside { /// The projection those cameras are in: without it, the extent history given back would be /// taken for another projection's and cleared. history_projection: String, - dirty: bool, + state: u64, + saved: Option, trusted_tasks: std::collections::HashSet, trusted_models: std::collections::HashSet, trusted_notebooks: std::collections::HashSet, @@ -475,7 +477,13 @@ struct PutAside { pub struct Session { pub project: Project, pub path: Option, - pub dirty: bool, + /// Which state of the project is open: a new one after each change, an earlier one again + /// after undo. Each is given once in a session (`states` counts them). + state: u64, + states: u64, + /// The state last saved, or opened from its file; `None` when no state of the open project is + /// (recovered work). The project has unsaved changes while another is open ([`Session::dirty`]). + saved: Option, /// Favourite, recent and saved data sources (kept with the app's preferences, not the /// project). pub sources: sources::Sources, @@ -703,7 +711,9 @@ impl Session { Session { project, path: None, - dirty: false, + state: 0, + states: 0, + saved: Some(0), sources: sources::Sources::default(), connections: connections::Connections::default(), s3_bindings: std::collections::BTreeMap::new(), @@ -952,6 +962,7 @@ impl Session { /// and restored if `f` fails or panics. pub(crate) fn guarded(&mut self, label: &'static str, mutates: bool, f: impl FnOnce(&mut Session) -> CmdResult) -> CmdResult { let snapshot = mutates.then(|| self.project.clone()); + let state = self.state; // The merge key asked for applies to this command only, whatever it does; and to a // command run by itself, not to those it runs. let merge = if self.depth == 0 { self.merge.take() } else { None }; @@ -1012,7 +1023,7 @@ impl Session { // back to before the first of them. Only a command run by itself merges. let key = merge; if key.is_none() || key != self.merged || self.undo.is_empty() { - self.undo.push((snap, open_layout, label, self.actor.clone())); + self.undo.push((snap, open_layout, label, self.actor.clone(), state)); if self.undo.len() > UNDO_LIMIT { self.undo.remove(0); } @@ -1021,7 +1032,7 @@ impl Session { self.merged = key; } self.redo.clear(); - self.dirty = true; + self.state = self.new_state(); } (Err(_), Some(snap)) => { // The project as it was before the command, so the layout open then, whatever @@ -1034,6 +1045,7 @@ impl Session { self.give_back(opened); } self.opened = opened; + self.state = state; if let Some(c) = counts { self.edit_counts = c; } @@ -1090,7 +1102,8 @@ impl Session { forward: take(&mut self.forward), last_camera: self.last_camera.take(), history_projection: self.history_projection.clone(), - dirty: std::mem::replace(&mut self.dirty, false), + state: self.state, + saved: self.saved, trusted_tasks: take(&mut self.trusted_tasks), trusted_models: take(&mut self.trusted_models), trusted_notebooks: take(&mut self.trusted_notebooks), @@ -1135,7 +1148,8 @@ impl Session { forward, last_camera, history_projection, - dirty, + state, + saved, trusted_tasks, trusted_models, trusted_notebooks, @@ -1167,7 +1181,8 @@ impl Session { } = *a; (self.undo, self.redo, self.back, self.forward, self.last_camera) = (undo, redo, back, forward, last_camera); self.history_projection = history_projection; - (self.dirty, self.project_starts, self.history_from_file) = (dirty, project_starts, history_from_file); + (self.state, self.saved) = (state, saved); + (self.project_starts, self.history_from_file) = (project_starts, history_from_file); (self.trusted_tasks, self.trusted_models, self.trusted_notebooks) = (trusted_tasks, trusted_models, trusted_notebooks); (self.active_layer, self.table_layer, self.table_standalone, self.tool) = (active_layer, table_layer, table_standalone, tool); (self.layer_selection, self.layer_selection_of) = (layer_selection, layer_selection_of); @@ -1241,30 +1256,57 @@ impl Session { pub fn clear_history(&mut self) { drop(self.take_project_state()); self.project_starts = self.project_starts.wrapping_add(1); + // A state of its own: undo back past a replacement made within a command (a task's step) + // reaches a state of the project before, which isn't this one's saved state. + self.state = self.new_state(); + self.saved = Some(self.state); + } + + /// Whether the project has changes that aren't saved: it is in another state than the one + /// last saved or opened. Undo or redo back to that state leaves none. + pub fn dirty(&self) -> bool { + self.saved != Some(self.state) + } + + /// The project as it is now is saved (written to its file, or downloaded in the browser). + pub fn mark_saved(&mut self) { + self.saved = Some(self.state); + } + + /// The project has changes that aren't saved, whatever undo does (work recovered after a + /// crash: no state of it is in a file). + pub fn mark_unsaved(&mut self) { + self.saved = None; + } + + /// A state of the project not given before in this session, for a change just made. + fn new_state(&mut self) -> u64 { + self.states = self.states.wrapping_add(1); + self.states } pub(crate) fn undo(&mut self) -> bool { self.merged = None; - let Some((p, saved, label, actor)) = self.undo.pop() else { return false }; + let Some((p, saved, label, actor, state)) = self.undo.pop() else { return false }; let (open, was) = (self.open_layout_name(), self.active_layout); let cur = std::mem::replace(&mut self.project, p); self.project.keep_table_ids(&cur); self.reopen_layout(open, saved); self.keep_time_position(&cur); - self.redo.push((cur, was, label, actor)); + self.redo.push((cur, was, label, actor, std::mem::replace(&mut self.state, state))); self.touch_all_geometry(); true } pub(crate) fn redo(&mut self) -> bool { self.merged = None; - let Some((p, saved, label, actor)) = self.redo.pop() else { return false }; + let Some((p, saved, label, actor, state)) = self.redo.pop() else { return false }; let (open, was) = (self.open_layout_name(), self.active_layout); let cur = std::mem::replace(&mut self.project, p); self.project.keep_table_ids(&cur); self.reopen_layout(open, saved); self.keep_time_position(&cur); - self.undo.push((cur, was, label, actor)); + self.undo.push((cur, was, label, actor, std::mem::replace(&mut self.state, state))); self.touch_all_geometry(); true } diff --git a/crates/engine/src/sample.rs b/crates/engine/src/sample.rs index fa827048a..520a417f5 100644 --- a/crates/engine/src/sample.rs +++ b/crates/engine/src/sample.rs @@ -46,7 +46,7 @@ fn countries_in(w: Worldview) -> &'static str { /// that is unsaved, unchanged and has nothing to undo, and only a layer named `Countries` holding /// exactly the built-in countries of `from`. Returns whether a layer was swapped. pub fn swap_countries(s: &mut crate::Session, from: Worldview, to: Worldview) -> bool { - if from == to || s.dirty || s.path.is_some() || s.can_undo() { + if from == to || s.dirty() || s.path.is_some() || s.can_undo() { return false; } let read = |w| read_geojson(countries_in(w), "Countries").ok().map(|(ds, _)| ds); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2a2e386de..cfd92fb3c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -34,26 +34,26 @@ fn worldview_swaps_only_untouched_sample_countries() { use sample::{Worldview, swap_countries}; let countries = |s: &Session| s.map().layer_by_name("Countries").unwrap().dataset.features.len(); let mut s = world(); - s.dirty = false; + s.mark_saved(); let n = countries(&s); // The layer holds the default countries, not China's. assert!(!swap_countries(&mut s, Worldview::China, Worldview::Default)); assert!(swap_countries(&mut s, Worldview::Default, Worldview::China)); assert_ne!(countries(&s), n); - assert!(!s.dirty); + assert!(!s.dirty()); assert!(swap_countries(&mut s, Worldview::China, Worldview::Default)); assert_eq!(countries(&s), n); // Not in a project with changes, or a saved one. - s.dirty = true; + s.mark_unsaved(); assert!(!swap_countries(&mut s, Worldview::Default, Worldview::China)); - s.dirty = false; + s.mark_saved(); s.path = Some("atlas.rgis".into()); assert!(!swap_countries(&mut s, Worldview::Default, Worldview::China)); // Not someone else's layer named Countries. let mut t = world(); - t.dirty = false; + t.mark_saved(); t.run("edit.deleteField", &json!({"layer": "Countries", "field": "pop_est"})).unwrap(); - t.dirty = false; + t.mark_saved(); assert!(!swap_countries(&mut t, Worldview::Default, Worldview::China)); } @@ -257,7 +257,7 @@ fn undo_steps_share_the_layers_they_leave_alone() { s.run("layer.rename", &json!({"layer": "Nations", "name": "States"})).unwrap(); let live = s.map().layer_by_name("States").unwrap(); let id = live.id; - for (snap, _, _, _) in &s.undo { + for (snap, _, _, _, _) in &s.undo { let old = snap.maps.iter().flat_map(|m| &m.layers).find(|l| l.id == id).unwrap(); assert!(old.dataset.features.shares(&live.dataset.features), "the snapshot copied the features"); assert_eq!(old.projected_cache().as_ptr(), live.projected_cache().as_ptr(), "the snapshot copied the projected geometry"); @@ -266,7 +266,7 @@ fn undo_steps_share_the_layers_they_leave_alone() { let before = s.map().layer_by_name("Cities").unwrap().dataset.features.len(); s.run("edit.addFeature", &json!({"layer": "Cities", "geometry": {"type": "Point", "coordinates": [1.0, 2.0]}})).unwrap(); let cities = s.map().layer_by_name("Cities").unwrap(); - let (snap, _, _, _) = s.undo.last().unwrap(); + let (snap, _, _, _, _) = s.undo.last().unwrap(); let old = snap.maps.iter().flat_map(|m| &m.layers).find(|l| l.id == cities.id).unwrap(); assert!(!old.dataset.features.shares(&cities.dataset.features)); assert_eq!((old.dataset.features.len(), cities.dataset.features.len()), (before, before + 1)); @@ -1071,17 +1071,17 @@ fn a_file_inside_a_geodatabase_adds_the_geodatabase() { #[test] fn save_clears_dirty_and_open_clears_history() { let mut s = world(); - assert!(!s.dirty && !s.can_undo(), "a fresh sample is clean"); + assert!(!s.dirty() && !s.can_undo(), "a fresh sample is clean"); s.run("layer.color", &json!({"layer": "Countries", "color": "#336699"})).unwrap(); - assert!(s.dirty && s.can_undo()); + assert!(s.dirty() && s.can_undo()); let dir = std::env::temp_dir().join(format!("rustgis-dirty-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let path = dir.join("a.rgis"); s.run("project.saveAs", &json!({"path": path.display().to_string()})).unwrap(); - assert!(!s.dirty, "saving clears the dirty marker"); + assert!(!s.dirty(), "saving clears the dirty marker"); assert!(s.can_undo(), "saving keeps the edit history"); s.run("project.open", &json!({"path": path.display().to_string()})).unwrap(); - assert!(!s.dirty && !s.can_undo() && !s.can_redo(), "opening starts a fresh history"); + assert!(!s.dirty() && !s.can_undo() && !s.can_redo(), "opening starts a fresh history"); assert!(s.run("edit.undo", &Value::Null).is_err()); std::fs::remove_dir_all(&dir).ok(); } @@ -1122,7 +1122,7 @@ fn open_from_text() { assert!(s.run("layer.addDataText", &json!({"fileName": "x.geojson", "text": "{"})).is_err()); assert!(s.run("project.openText", &json!({"text": "not json"})).is_err()); s.run("project.openText", &json!({"text": text})).unwrap(); - assert!(s.map().layer_by_name("pts").is_none() && !s.dirty && !s.can_undo()); + assert!(s.map().layer_by_name("pts").is_none() && !s.dirty() && !s.can_undo()); } #[test] @@ -8241,12 +8241,12 @@ fn a_pending_answer_is_no_undo_step() { let r = s.guarded("Add Data From URL", true, |s| s.request_url(&p)); assert_eq!(r.unwrap()["pending"], true); assert_eq!(s.undo.len(), undo); - assert!(!s.dirty); + assert!(!s.dirty()); assert_eq!(s.take_url_requests().len(), 1); let r = s.guarded("Plugin Command", true, |_| Ok(json!({"pending": true}))); assert!(r.is_ok()); assert_eq!(s.undo.len(), undo + 1, "an answer alone doesn't skip the step"); - assert!(s.dirty); + assert!(s.dirty()); assert!(s.run("map.new", &json!({"name": "B"})).is_ok()); assert_eq!(s.undo.len(), undo + 2); // A command that changed the project and then, through a command of its own, left a @@ -8989,9 +8989,9 @@ fn blank_project_starts_in_a_projected_crs() { assert!(s.map().layers.is_empty()); let r = s.run("map.projection", &json!({"type": "EPSG:3010"})).unwrap(); assert_eq!(r["projection"], "EPSG:3010"); - assert!(s.dirty && s.can_undo()); + assert!(s.dirty() && s.can_undo()); s.clear_history(); - assert!(!s.dirty && !s.can_undo()); + assert!(!s.dirty() && !s.can_undo()); assert_eq!(s.map().projection.code(), "EPSG:3010"); // It opens on the country, and reports that view back. s.run("map.zoomToExtent", &json!({"extent": [10.5, 55.0, 24.5, 69.5]})).unwrap(); @@ -11483,9 +11483,9 @@ fn s3_connections_listings_and_objects() { ); s.run("s3.disconnect", &json!({"name": "s3-blank"})).unwrap(); // A level of the bucket, and the connection's buckets; listing isn't an edit of the project. - assert!(!s.dirty && !s.can_undo()); + assert!(!s.dirty() && !s.can_undo()); let r = s.run("s3.list", &json!({"url": "s3://s3-test-bucket/data/"})).unwrap(); - assert!(!s.dirty && !s.can_undo(), "a listing is no edit"); + assert!(!s.dirty() && !s.can_undo(), "a listing is no edit"); assert_eq!(r["prefixes"], json!(["data/more/"])); assert_eq!(r["objects"][0]["url"], "s3://s3-test-bucket/data/places.geojson"); let r = s.run("s3.list", &json!({"connection": "s3-test"})).unwrap(); @@ -12099,7 +12099,7 @@ fn connections_commands() { assert!(crate::cmd::plugins::user_only(id), "{id}"); } assert!(!crate::cmd::plugins::user_only("connection.list")); - assert!(!s.dirty && !s.can_undo()); + assert!(!s.dirty() && !s.can_undo()); } #[test] @@ -14465,7 +14465,7 @@ fn a_failed_command_keeps_the_project_open_before_it() { s.run("map.rename", &json!({"name": "Edited"})).unwrap(); s.trusted_tasks.insert("mine".into()); let (name, opened, undo) = (s.project.name.clone(), s.opened, s.undo_label()); - assert!(undo.is_some() && s.dirty); + assert!(undo.is_some() && s.dirty()); let r = s.guarded("Task", true, |s| { s.run("project.new", &json!({"name": "Other"}))?; Err("a later step failed".into()) @@ -14475,7 +14475,7 @@ fn a_failed_command_keeps_the_project_open_before_it() { // It comes back with what it held: its undo history, its unsaved state, the trust it had, and // its file, so Save writes there. assert_eq!(s.undo_label(), undo); - assert!(s.dirty && s.trusted_tasks.contains("mine")); + assert!(s.dirty() && s.trusted_tasks.contains("mine")); assert_eq!(s.path.as_deref(), Some(std::path::Path::new("atlas.rgis"))); s.run("edit.undo", &Value::Null).unwrap(); assert_ne!(s.map().name, "Edited", "the edit before still undoes"); @@ -14624,13 +14624,13 @@ fn a_run_is_copied_from_the_history_as_code() { assert!(text.contains("\"World Atlas.rgis\""), "{text}"); // Saved, then changed: the script says to save it again first. s.path = Some("atlas.rgis".into()); - s.dirty = true; + s.mark_unsaved(); let text = s.run("gp.historyScript", &Value::Null).unwrap()["text"].as_str().unwrap().to_string(); assert!( text.contains("# The project has changes that aren't saved: save it first") && !text.contains("hasn't been saved yet"), "{text}" ); - s.dirty = false; + s.mark_saved(); let text = s.run("gp.historyScript", &Value::Null).unwrap()["text"].as_str().unwrap().to_string(); assert!(!text.contains("aren't saved") && text.contains("rustgis.Session(\"atlas.rgis\")"), "{text}"); s.path = None; @@ -15357,7 +15357,7 @@ fn raster_stretch_custom_bounds() { fn default_raster_style_applies_to_new_rasters() { use rustgis_map::{RasterRenderer, Resampling, Stretch}; let mut s = world(); - s.dirty = false; + s.mark_saved(); let undo = s.undo_label(); // The layer just added (rasters go in under the vectors, the newest on top of them). let style = |s: &Session| s.map().layer(s.active_layer.unwrap()).unwrap().raster.as_ref().unwrap().style.clone(); @@ -15373,7 +15373,7 @@ fn default_raster_style_applies_to_new_rasters() { ) .unwrap(); assert_eq!(shown, json!({"ramp": "Viridis", "invert": false, "stretch": "standardDeviation", "n": 2.5, "resampling": "Bilinear"})); - assert!(!s.dirty && s.undo_label() == undo, "an app setting, not a project change"); + assert!(!s.dirty() && s.undo_label() == undo, "an app setting, not a project change"); s.run("layer.addDataBytes", &json!({"fileName": "dem.tif", "data": dem_b64()})).unwrap(); let st = style(&s); assert_eq!(st.renderer, RasterRenderer::Stretch { band: 0, ramp: rustgis_map::Ramp::Viridis, invert: false }); @@ -16298,6 +16298,77 @@ fn undo_steps_remember_who_made_them() { assert_eq!((s.undo_actor(), s.redo_actor()), (Some(Some("agent 1")), None)); } +/// Unsaved means unsaved (#906): undo or redo back to the state last saved leaves no unsaved +/// changes, whichever way it gets there; an edit after undoing past the save leaves that state +/// out of reach. +#[test] +fn undo_back_to_the_saved_state_leaves_nothing_unsaved() { + let dir = std::env::temp_dir().join(format!("rustgis-saved-state-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("atlas.rgis").display().to_string(); + let mut s = world(); + let rename = + |s: &mut Session, name: &str| s.run("layer.rename", &json!({"layer": s.map().layers[0].name.clone(), "name": name})).unwrap(); + rename(&mut s, "One"); + s.run("project.saveAs", &json!({"path": path})).unwrap(); + assert!(!s.dirty()); + rename(&mut s, "Two"); + rename(&mut s, "Three"); + assert!(s.dirty()); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(s.dirty(), "one step from the save"); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(!s.dirty(), "back at the save"); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(s.dirty(), "before the save"); + s.run("edit.redo", &Value::Null).unwrap(); + assert!(!s.dirty(), "redo back to the save"); + s.run("edit.redo", &Value::Null).unwrap(); + assert!(s.dirty()); + // Saving there: the states either side are unsaved. + s.run("project.save", &Value::Null).unwrap(); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(s.dirty()); + s.run("edit.redo", &Value::Null).unwrap(); + assert!(!s.dirty()); + // Steps merged into one: undo takes them all back to the save. + s.merge_undo("nudge"); + rename(&mut s, "Four"); + s.merge_undo("nudge"); + rename(&mut s, "Five"); + assert!(s.dirty()); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(!s.dirty()); + // A failed command leaves the state as it was, and a project it replaced on the way gives + // back its saved state with its history. + assert!(s.run("layer.rename", &json!({"layer": "Two", "name": ""})).is_err()); + assert!(!s.dirty()); + rename(&mut s, "Six"); + let r = s.guarded("Task", true, |s| { + s.run("project.new", &json!({"name": "Other"}))?; + Err("a later step failed".into()) + }); + assert!(r.is_err() && s.dirty()); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(!s.dirty(), "the save is still the one undo returns to"); + // An edit after undoing past the save: no way back to it. States are compared, not contents: + // the same name typed again is a change. + s.run("edit.undo", &Value::Null).unwrap(); + rename(&mut s, "Two"); + assert!(s.dirty() && !s.can_redo()); + // Recovered work is unsaved whatever undo does; opening a project starts saved. + s.mark_unsaved(); + assert!(s.dirty()); + s.run("project.open", &json!({"path": path})).unwrap(); + assert!(!s.dirty()); + // A project opened by a command that is then undone isn't the one saved. + let r = s.guarded("Task", true, |s| s.run("project.new", &json!({"name": "Other"}))); + assert!(r.is_ok() && s.dirty()); + s.run("edit.undo", &Value::Null).unwrap(); + assert!(s.dirty(), "the project before the task, not the new one's saved state"); + std::fs::remove_dir_all(&dir).ok(); +} + /// A legend can leave layers out though the map draws them: set when added or later, checked /// against its frame's map, kept through a save (not written when empty), and following a /// renamed layer. diff --git a/crates/engine/src/tests/task_reentry.rs b/crates/engine/src/tests/task_reentry.rs index 91560f50d..97b5b05cb 100644 --- a/crates/engine/src/tests/task_reentry.rs +++ b/crates/engine/src/tests/task_reentry.rs @@ -65,13 +65,13 @@ fn task_reentry_failure_rolls_back_outer_project_transaction() { ]), ); let undo = s.undo_depth(); - s.dirty = false; + s.mark_saved(); let before = serde_json::to_value(&s.project).unwrap(); assert!(s.run("task.run", &json!({"task":"Loop"})).unwrap_err().contains("already running")); assert_eq!(serde_json::to_value(&s.project).unwrap(), before); assert_eq!(s.undo_depth(), undo); assert!(s.edit_counts.is_empty()); - assert!(!s.dirty && !s.task_running); + assert!(!s.dirty() && !s.task_running); } /// Invalid parameters and imported/direct-step refusals leave the guard usable and trust intact. diff --git a/crates/engine/tests/portable_projects.rs b/crates/engine/tests/portable_projects.rs index 851509437..74a64522f 100644 --- a/crates/engine/tests/portable_projects.rs +++ b/crates/engine/tests/portable_projects.rs @@ -183,7 +183,7 @@ fn missing_stream_can_be_repaired_undone_redone_and_saved() { std::fs::rename(&old, &replacement).unwrap(); let project = f.file("survey/map.rgis"); let mut s = open(&project); - assert!(!s.dirty); + assert!(!s.dirty()); let l = s.map().layer(layers.cities).unwrap(); let name = l.name.clone(); let error = l.source.as_ref().unwrap().error.clone(); @@ -195,7 +195,7 @@ fn missing_stream_can_be_repaired_undone_redone_and_saved() { let result = s.run("layer.repairSource", &json!({"layer": layers.cities, "path": replacement})).unwrap(); assert_eq!(result["layer"], layers.cities); assert_eq!(result["path"], replacement.to_str().unwrap()); - assert!(s.dirty); + assert!(s.dirty()); assert_stream(&s, layers.cities, &replacement); assert_eq!(s.map().layer(layers.cities).unwrap().name, name); assert_eq!(configuration(&s), config); diff --git a/crates/engine/tests/portable_projects/repair.rs b/crates/engine/tests/portable_projects/repair.rs index b28d5375b..1045ea180 100644 --- a/crates/engine/tests/portable_projects/repair.rs +++ b/crates/engine/tests/portable_projects/repair.rs @@ -83,7 +83,7 @@ fn renamed_raster_repairs_with_its_renderer_and_data_intact() { assert!(raster.error.is_none()); assert_eq!(raster.data.as_ref().unwrap().band_count(), 3); assert_eq!(serde_json::to_value(&raster.style).unwrap(), style); - assert!(s.dirty); + assert!(s.dirty()); } #[test] diff --git a/crates/engine/tests/project_save.rs b/crates/engine/tests/project_save.rs index 68799f9f4..ec7367abf 100644 --- a/crates/engine/tests/project_save.rs +++ b/crates/engine/tests/project_save.rs @@ -23,7 +23,11 @@ fn failed_save_as_preserves_project_state_and_a_later_save_succeeds() -> Result< s.run("project.rename", &json!({"name": "pending"}))?; s.run("project.rename", &json!({"name": "redo name"}))?; s.run("edit.undo", &Value::Null)?; - s.dirty = dirty; + if dirty { + s.mark_unsaved() + } else { + s.mark_saved() + } s.edit_counts.insert(1, EditCounts { added: 2, modified: 3, deleted: 1 }); let before = serde_json::to_value(&s.project).map_err(|e| e.to_string())?; let path = s.path.clone(); @@ -35,7 +39,7 @@ fn failed_save_as_preserves_project_state_and_a_later_save_succeeds() -> Result< assert!(s.run("project.saveAs", &json!({"path": target})).is_err()); assert_eq!(serde_json::to_value(&s.project).map_err(|e| e.to_string())?, before); assert_eq!(s.path, path); - assert_eq!(s.dirty, dirty); + assert_eq!(s.dirty(), dirty); assert_eq!(s.undo_depth(), depth); assert_eq!(s.edit_counts, counts); assert_eq!((s.undo_label(), s.redo_label()), history); @@ -57,7 +61,7 @@ fn failed_save_as_preserves_project_state_and_a_later_save_succeeds() -> Result< assert_eq!(s.project.name, "renamed"); assert_eq!(Project::load(&expected)?.name, "renamed"); assert_eq!(s.path.as_ref(), Some(&expected)); - assert!(!s.dirty); + assert!(!s.dirty()); assert!(s.edit_counts.is_empty()); assert_eq!(s.undo_depth(), depth); assert!(s.can_redo()); diff --git a/crates/ui-egui/src/autosave.rs b/crates/ui-egui/src/autosave.rs index 2c9522323..274a730c8 100644 --- a/crates/ui-egui/src/autosave.rs +++ b/crates/ui-egui/src/autosave.rs @@ -186,7 +186,7 @@ pub(crate) fn tick(app: &mut RustGisApp, ctx: &egui::Context) { app.session.notify(text, true); } } - if app.autosave.minutes == 0 || !app.session.dirty { + if app.autosave.minutes == 0 || !app.session.dirty() { if app.autosave.written.take().is_some() { store.clear(); } @@ -260,13 +260,13 @@ pub(crate) fn recover(app: &mut RustGisApp, p: &Value) -> Result } // Opening it replaces the open project: never one with work of its own not saved (after // Later), which it would lose with its recovery copy. - if app.session.dirty { + if app.session.dirty() { return Err("the open project has unsaved changes: save them before recovering other work".into()); } let text = store.read_pending()?; app.run("project.openText", json!({"text": text}))?; app.session.path = offered.path.as_ref().map(std::path::PathBuf::from); - app.session.dirty = true; + app.session.mark_unsaved(); // The copy becomes this run's as it is: a crash now loses nothing, and nothing is written. if let Some(store) = &app.services.recovery { store.adopt_pending(); @@ -340,7 +340,7 @@ pub(crate) fn offer(app: &mut RustGisApp, ctx: &egui::Context) { } // Recovering replaces the open project: not one with unsaved work of its own (an edit // made before answering). Recover comes back once that work is saved. - let blocked = app.session.dirty; + let blocked = app.session.dirty(); if blocked { ui.add_space(4.0); ui.label( diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 124d7fbac..b6c98f703 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -107,12 +107,12 @@ fn start(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.horizontal(|ui| { ui.spacing_mut().item_spacing = vec2(18.0, 0.0); if card(ui, t, "Map", "A blank map with a basemap", "new_map") { - let _ = app.run("project.new", Value::Null); + let _ = app.run_by_user("project.new", Value::Null); app.ui.backstage = false; app.map.invalidate(); } if card(ui, t, "World Atlas", "Countries, cities, rivers and lakes", "basemap") { - let _ = app.run("project.sample", Value::Null); + let _ = app.run_by_user("project.sample", Value::Null); app.ui.backstage = false; app.map.invalidate(); } @@ -143,8 +143,11 @@ fn start(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { }); } +/// A New Project card: a thumbnail with its title and line beneath, a button named by its title. +/// True when clicked. fn card(ui: &mut Ui, t: &Tokens, title: &str, sub: &str, icon: &str) -> bool { let (r, resp) = ui.allocate_exact_size(vec2(200.0, 160.0), Sense::click()); + resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, tl!(title))); ui.painter().rect_filled(r, CornerRadius::same(10), if resp.hovered() { t.hover } else { t.pane }); ui.painter().rect_stroke( r, diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 9a6951acf..95382294a 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -203,7 +203,7 @@ pub fn header(app: &mut RustGisApp, ui: &mut Ui) { } // Title: centred when it fits, otherwise in the free gap; the map name and then the // project name drop out when there is no room. - let title = format!("{}{}", app.session.project.name, if app.session.dirty { " •" } else { "" }); + let title = format!("{}{}", app.session.project.name, if app.session.dirty() { " •" } else { "" }); let sub = format!(" — {}", app.session.map().name); let (title_w, sub_w) = (text_w(ui, &title, semibold(12.5)), text_w(ui, &sub, regular(12.0))); let (a, b) = (qat_end + 8.0, sr.min.x - 16.0); @@ -486,6 +486,12 @@ fn parse_scale(text: &str) -> Option { /// Global keyboard shortcuts. pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { + // Behind a dialog (Save changes?, Recover unsaved work?, Command Search…) the keys are the + // dialog's: Escape answers it rather than switching tools, and Ctrl+N can't start what it + // asks about. + if ctx.memory(|m| m.top_modal_layer()).is_some() { + return; + } crate::modelbuilder::shortcuts(app, ctx); // The search box was typed in until egui took its focus away on Escape: that Escape clears // the box, not the tool. @@ -626,7 +632,7 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { } continue; } - let _ = app.run(id, p); + let _ = app.run_by_user(id, p); } } @@ -991,7 +997,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { let _ = app.run("ui.pane.workflows", json!({"section": section})); } else if let Some(action) = search_action(&id) { let _ = match action { - SearchAction::Command(command) => app.run(command, Value::Null), + SearchAction::Command(command) => app.run_by_user(command, Value::Null), SearchAction::Dialog(name) => app.run("ui.dialog", json!({"name": name})), SearchAction::OpenItem(kind) => app.run("ui.openItem", json!({"kind": kind})), SearchAction::SaveItem(kind) => app.run("ui.saveItem", json!({"kind": kind})), @@ -1002,7 +1008,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { } }; } else { - let _ = app.run(&id, Value::Null); + let _ = app.run_by_user(&id, Value::Null); } } if keep { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index ad1842dce..066bfad4a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1299,7 +1299,9 @@ fn select_by_attributes(app: &mut RustGisApp, ui: &mut egui::Ui, t: &Tokens) -> }) } -#[cfg(not(target_arch = "wasm32"))] +/// Asks for a file to open with the system's dialog, offering `exts` under `label` (and all +/// files); `None` when cancelled. +#[cfg(all(not(target_arch = "wasm32"), not(test)))] pub fn pick_open(exts: &[&str], label: &str) -> Option { let dialog = rfd::FileDialog::new(); let dialog = if exts.is_empty() { dialog } else { dialog.add_filter(label, exts) }; @@ -1311,11 +1313,31 @@ pub fn pick_folder() -> Option { rfd::FileDialog::new().pick_folder().map(|p| p.display().to_string()) } -#[cfg(not(target_arch = "wasm32"))] +/// Asks where to save, suggesting file `name`, with the system's dialog; `None` when cancelled. +#[cfg(all(not(target_arch = "wasm32"), not(test)))] pub fn pick_save(name: &str, exts: &[&str], label: &str) -> Option { rfd::FileDialog::new().set_file_name(name).add_filter(label, exts).save_file().map(|p| p.display().to_string()) } +// Unit tests can't answer the system's file dialogs: a picker answers what the test put in +// `PICKED` instead, once, and is cancelled when it holds nothing. +#[cfg(test)] +thread_local! { + pub(crate) static PICKED: std::cell::RefCell> = const { std::cell::RefCell::new(None) }; +} + +/// In a unit test: the path the test put in [`PICKED`], or cancelled. +#[cfg(all(not(target_arch = "wasm32"), test))] +pub fn pick_open(_: &[&str], _: &str) -> Option { + PICKED.with(|p| p.borrow_mut().take()) +} + +/// In a unit test: the path the test put in [`PICKED`], or cancelled. +#[cfg(all(not(target_arch = "wasm32"), test))] +pub fn pick_save(_: &str, _: &[&str], _: &str) -> Option { + PICKED.with(|p| p.borrow_mut().take()) +} + #[cfg(target_arch = "wasm32")] pub fn pick_open(_: &[&str], _: &str) -> Option { None diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4c34eea8b..7d154e81a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -61,6 +61,7 @@ mod repairsource; mod reportpane; mod ribbon; mod s3ui; +mod saveprompt; mod scriptcolor; mod scriptcomplete; mod searchbox; @@ -828,6 +829,8 @@ pub struct RustGisApp { pub services: Services, /// Autosave's interval, its last copy, and the copy an earlier run left (#873). pub(crate) autosave: autosave::AutosaveState, + /// The question asked before unsaved changes are discarded, and a quit the user agreed to. + pub(crate) save_prompt: saveprompt::SavePromptState, /// The MCP relay for `rustgis-cli mcp --attach`: a loopback listener on the session this app /// already has open, started by the `ui.mcpRelay` command and off until then. #[cfg(not(target_arch = "wasm32"))] @@ -976,6 +979,7 @@ impl RustGisApp { shown_lang: i18n::Lang::EN, seen_project: None, autosave: Default::default(), + save_prompt: Default::default(), } } @@ -1363,7 +1367,7 @@ impl RustGisApp { false, ); } - self.session.dirty = false; + self.session.mark_saved(); self.ui.web_file = Some(file); Ok(()) } @@ -1409,7 +1413,7 @@ impl RustGisApp { rustgis_engine::sample::apply_worldview(if cfg!(test) { "default" } else { i18n::AUTO }); let mut s = Session::new(Project::default()); let _ = s.run("project.sample", &Value::Null); - s.dirty = false; + s.mark_saved(); RustGisApp::new(s) } @@ -1541,6 +1545,18 @@ impl RustGisApp { self.ui.last_project = Some(path); } + /// Runs a command the user asked for in the interface (a button, a menu, a shortcut, Command + /// Search, a file dropped on the window): one that would replace a project with unsaved + /// changes asks to save them first, and runs once answered. Scripts, the AI Assistant and + /// attached agents run commands without asking ([`Self::run`]). + pub fn run_by_user(&mut self, id: &str, params: Value) -> Result { + if saveprompt::asks(self, id) { + saveprompt::ask(self, saveprompt::Discard::Replace(id.to_string(), params)); + return Ok(Value::Null); + } + self.run(id, params) + } + /// Runs a command by id: `ui.*` here, everything else in the engine. pub fn run(&mut self, id: &str, params: Value) -> Result { if !matches!(id, "ui.menu" | "ui.ribbon" | "ui.language" | "ui.worldview") { @@ -2433,7 +2449,7 @@ impl RustGisApp { // A QGIS project opens as a new map of this one. let lower = path.to_ascii_lowercase(); let cmd = if lower.ends_with(".qgs") || lower.ends_with(".qgz") { "project.importQgis" } else { "project.open" }; - let r = self.run(cmd, json!({"path": path})); + let r = self.run_by_user(cmd, json!({"path": path})); self.ui.backstage = false; self.map.invalidate(); return r; @@ -2594,7 +2610,7 @@ impl RustGisApp { self.ui.pending_open = Some(cmd); open(if kind == "ramps" { "ramp" } else { "item" }); } else if let Some(path) = dialogs::pick_open(exts, tl!(label)) { - let r = self.run(cmd, json!({"path": path})); + let r = self.run_by_user(cmd, json!({"path": path})); self.map.invalidate(); return r; } @@ -3489,6 +3505,7 @@ impl RustGisApp { // on the session directly) becomes Last Project now, not a frame later the app may not // reach. self.note_last_project(); + saveprompt::close_requested(self, ctx); autosave::tick(self, ctx); self.ensure_script_font(); // Fonts downloaded by the web app. @@ -3577,7 +3594,7 @@ impl RustGisApp { } continue; } - let _ = self.run(rustgis_engine::project::open_command(&p), json!({"path": p})); + let _ = self.run_by_user(rustgis_engine::project::open_command(&p), json!({"path": p})); self.map.invalidate(); } } @@ -3643,6 +3660,7 @@ impl RustGisApp { dialogs::show(self, &ctx); mapview::confirm_delete(self, &ctx); autosave::offer(self, &ctx); + saveprompt::show(self, &ctx); pluginsui::show(self, &ctx); update::show(self, &ctx); showme::tick(self, &ctx); @@ -3660,7 +3678,7 @@ impl RustGisApp { if let Some(url) = self.ui.open_url.take() { ctx.open_url(egui::OpenUrl::new_tab(url)); } - let title = format!("{}{} - RustGIS", self.session.project.name, if self.session.dirty { " •" } else { "" }); + let title = format!("{}{} - RustGIS", self.session.project.name, if self.session.dirty() { " •" } else { "" }); if title != self.sent_title { ctx.send_viewport_cmd(egui::ViewportCommand::Title(title.clone())); self.sent_title = title; diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 8dc0eee03..098e7670b 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -2284,7 +2284,7 @@ fn folder_browser(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { if is_dir { app.table.folder = Some(path.clone()); } else if rustgis_engine::project::opens_as_project(&name) { - let _ = app.run("project.open", json!({"path": path.display().to_string()})); + let _ = app.run_by_user("project.open", json!({"path": path.display().to_string()})); app.map.invalidate(); } else if name.ends_with(".rgismap") || name.ends_with(".rgismpk") { let _ = app.run("map.import", json!({"path": path.display().to_string()})); @@ -5056,7 +5056,7 @@ fn save_history_script(app: &mut RustGisApp) { return; } // The script opens the project's saved file, which lacks what changed since. - if app.session.dirty || app.session.path.is_none() { + if app.session.dirty() || app.session.path.is_none() { app.session.notify(tl!("Save the project too: the script opens its saved file, without changes made since."), false); } } diff --git a/crates/ui-egui/src/saveprompt.rs b/crates/ui-egui/src/saveprompt.rs new file mode 100644 index 000000000..a582b0b0f --- /dev/null +++ b/crates/ui-egui/src/saveprompt.rs @@ -0,0 +1,400 @@ +//! Asks to save unsaved changes before the interface discards them (#906): quitting, closing the +//! window, or replacing the project (New, Open, the World Atlas sample, a template, a project +//! file dropped on the window). Save, Don't Save or Cancel. +//! +//! Commands never ask: `project.new` and the rest run as before for the CLI, scripts, the AI +//! Assistant and attached agents. What the user does in the interface goes through +//! [`RustGisApp::run_by_user`], which asks first. The desktop app asks; the browser warns before +//! its page is left instead, and saves by downloading, so it doesn't. + +use egui::vec2; +use serde_json::Value; + +use crate::RustGisApp; +use crate::theme::{Tokens, semibold}; + +/// The commands that replace the open project. +const REPLACES: &[&str] = &["project.new", "project.sample", "project.open", "project.openText", "project.fromTemplate"]; + +/// What waits for the user's answer. +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum Discard { + /// A command replacing the project, with its parameters. + Replace(String, Value), + /// Quitting: the window closes once the answer allows it. + Quit, +} + +#[derive(Default)] +pub(crate) struct SavePromptState { + /// What the dialog asks about, while it shows. + pub asking: Option, + /// The user chose to quit: the next close request goes through. + pub closing: bool, +} + +/// Whether running command `id` from the interface asks first: it replaces the project, which +/// has unsaved changes, in the desktop app. +pub(crate) fn asks(app: &RustGisApp, id: &str) -> bool { + REPLACES.contains(&id) && app.session.dirty() && app.opens_files() +} + +/// Shows the dialog for `what`. One already showing keeps its question: a file dropped on the +/// window meanwhile doesn't replace it. Quitting does, as the user asked for it last. +pub(crate) fn ask(app: &mut RustGisApp, what: Discard) { + if app.save_prompt.asking.is_none() || what == Discard::Quit { + app.save_prompt.asking = Some(what); + } +} + +/// Each frame, before anything draws: a request to close the window (its close button, the +/// system's Quit, or `ui.quit`) waits while the project has unsaved changes, and the dialog asks. +pub(crate) fn close_requested(app: &mut RustGisApp, ctx: &egui::Context) { + if !ctx.input(|i| i.viewport().close_requested()) || app.save_prompt.closing { + return; + } + if !(app.session.dirty() && app.opens_files()) { + return; + } + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + // `ui.quit` asked the shell to close: it would ask again each frame. + app.quit_requested = false; + ask(app, Discard::Quit); +} + +/// The dialog: Save (then go on, once the project is saved), Don't Save (go on), or Cancel. +pub(crate) fn show(app: &mut RustGisApp, ctx: &egui::Context) { + let Some(what) = app.save_prompt.asking.clone() else { return }; + let t = Tokens::get(ctx); + let name = &app.session.project.name; + let name = if name.trim().is_empty() { tl_raw!("Untitled").to_string() } else { name.clone() }; + let mut answer = None; + egui::Modal::new(egui::Id::unique("save_prompt")).show(ctx, |ui| { + ui.set_width(380.0); + ui.label(egui::RichText::new(crate::i18n::fmt("Save changes to {name}?", &[("name", &name)])).font(semibold(15.0))); + ui.add_space(4.0); + ui.label(tl!("Your changes will be lost if you don't save them.")); + ui.add_space(10.0); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Min), |ui| { + if crate::widgets::dialog_button(ui, tl!("Save"), Some(t.accent), vec2(84.0, 28.0)).clicked() { + answer = Some(Answer::Save); + } + if crate::widgets::dialog_button(ui, tl!("Don't Save"), None, vec2(84.0, 28.0)).clicked() { + answer = Some(Answer::DontSave); + } + if crate::widgets::dialog_button(ui, tl!("Cancel"), None, vec2(84.0, 28.0)).clicked() + || ui.input(|i| i.key_pressed(egui::Key::Escape)) + { + answer = Some(Answer::Cancel); + } + }); + }); + let Some(answer) = answer else { return }; + app.save_prompt.asking = None; + // The shell closes the window, or the replaced project draws, on the next frame. + ctx.request_repaint(); + match answer { + // Save As when the project has no file. A save that fails says why (its notice), and one + // cancelled leaves the changes unsaved: either way nothing is discarded. + Answer::Save => { + if app.run("ui.save", Value::Null).is_ok() && !app.session.dirty() { + proceed(app, what); + } + } + Answer::DontSave => proceed(app, what), + Answer::Cancel => {} + } +} + +#[derive(Clone, Copy)] +enum Answer { + Save, + DontSave, + Cancel, +} + +/// Does what waited for the answer. Quitting removes the recovery copy as a clean exit does +/// (`on_exit`); a replacement leaves no unsaved changes, so autosave removes it too. +fn proceed(app: &mut RustGisApp, what: Discard) { + match what { + Discard::Replace(id, params) => { + let _ = app.run(&id, params); + app.ui.backstage = false; + app.map.invalidate(); + } + Discard::Quit => { + app.save_prompt.closing = true; + app.quit_requested = true; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::dialogs::PICKED; + use egui::accesskit::Role; + use egui_kittest::kittest::Queryable; + use serde_json::json; + use std::path::PathBuf; + + type Harness = egui_kittest::Harness<'static, RustGisApp>; + + /// The desktop app on the World Atlas sample, never saved, drawing its frames. + fn launch() -> Harness { + let mut app = RustGisApp::with_sample(); + app.start(&Default::default()).unwrap(); + egui_kittest::Harness::builder().with_size(vec2(1400.0, 860.0)).with_max_steps(64).build_ui_state( + |ui, app: &mut RustGisApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ) + } + + /// A folder of its own for test `name`, emptied first. + fn folder(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("rustgis-save-prompt-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// An unsaved change: the map renamed (a new project has no layers to change). + fn edit(h: &mut Harness, name: &str) { + h.state_mut().run("map.rename", json!({"name": name})).unwrap(); + assert!(h.state().session.dirty()); + } + + /// Runs `n` frames: a click is handled on the frame after it. + fn frames(h: &mut Harness, n: usize) { + for _ in 0..n { + h.step(); + } + } + + /// Whether the dialog shows. + fn asking(h: &Harness) -> bool { + h.query_by_label_contains("Save changes to").is_some() + } + + /// Clicks the dialog's button `label` and lets the answer take effect. + fn click(h: &mut Harness, label: &str) { + h.get_by_role_and_label(Role::Button, label).click(); + frames(h, 3); + } + + /// A file dropped on the window, as the desktop shell passes it: by its path. + #[derive(Debug)] + struct Dropped(PathBuf); + + impl egui::DroppedFile for Dropped { + /// Where the file is. + fn path(&self) -> &std::path::Path { + &self.0 + } + /// The file's contents, read when asked. + fn bytes(&self) -> Result, String> { + std::fs::read(&self.0).map_err(|e| e.to_string()) + } + } + + /// The window's close button, or the system's Quit: what eframe passes the app. + fn close_window(h: &mut Harness) { + h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + h.step(); + } + + /// Whether the last frame held back the window's close (`CancelClose`). + fn close_cancelled(h: &Harness) -> bool { + h.output().viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose)) + } + + /// Closing the window with unsaved changes waits for an answer: Cancel keeps the app open + /// with the changes, Don't Save closes it, and so does `ui.quit` once answered. + #[test] + fn quitting_asks_and_each_answer_does_what_it_says() { + let mut h = launch(); + frames(&mut h, 2); + // Nothing unsaved: the window closes without a word. + close_window(&mut h); + assert!(!close_cancelled(&h) && !asking(&h)); + edit(&mut h, "Edited"); + close_window(&mut h); + assert!(close_cancelled(&h), "the close waits"); + frames(&mut h, 2); + assert!(asking(&h) && h.query_by_label("Save changes to World Atlas?").is_some()); + click(&mut h, "Cancel"); + assert!(!asking(&h) && !h.state().quit_requested && h.state().session.dirty()); + // Escape is Cancel too. + close_window(&mut h); + frames(&mut h, 2); + h.key_press(egui::Key::Escape); + frames(&mut h, 2); + assert!(!asking(&h) && !h.state().quit_requested); + // `ui.quit` asks the shell to close, which comes back as a close request: it asks once, + // and stops asking the shell meanwhile. + h.state_mut().run("ui.quit", Value::Null).unwrap(); + close_window(&mut h); + assert!(close_cancelled(&h) && !h.state().quit_requested); + frames(&mut h, 2); + click(&mut h, "Don't Save"); + assert!(h.state().quit_requested && !asking(&h)); + close_window(&mut h); + assert!(!close_cancelled(&h), "the window closes"); + } + + /// Save on a project never saved asks where to write it: cancelling that keeps the project + /// and its changes; a path picked saves it, then quits. + #[test] + fn save_on_a_new_project_saves_as_first() { + let dir = folder("save-as"); + let mut h = launch(); + frames(&mut h, 2); + edit(&mut h, "Edited"); + close_window(&mut h); + frames(&mut h, 2); + PICKED.with(|p| *p.borrow_mut() = None); + click(&mut h, "Save"); + assert!(!h.state().quit_requested && h.state().session.dirty() && !asking(&h), "Save As cancelled: nothing is discarded"); + let file = dir.join("atlas.rgis"); + close_window(&mut h); + frames(&mut h, 2); + PICKED.with(|p| *p.borrow_mut() = Some(file.display().to_string())); + click(&mut h, "Save"); + assert!(file.exists() && !h.state().session.dirty() && h.state().quit_requested); + let _ = std::fs::remove_dir_all(&dir); + } + + /// A save that fails discards nothing: the project and its changes stay, and it says why. + #[test] + fn a_failed_save_discards_nothing() { + let dir = folder("failed"); + let mut h = launch(); + frames(&mut h, 2); + edit(&mut h, "Edited"); + // Its file is in a folder that has gone. + h.state_mut().session.path = Some(dir.join("gone").join("atlas.rgis")); + h.state_mut().run_by_user("project.new", Value::Null).unwrap(); + frames(&mut h, 2); + assert!(asking(&h)); + click(&mut h, "Save"); + let app = h.state(); + assert!(app.session.dirty() && app.session.project.name == "World Atlas" && app.session.map().name == "Edited"); + assert!(app.session.notices.iter().any(|n| n.error), "the failure is said"); + let _ = std::fs::remove_dir_all(&dir); + } + + /// New from the toolbar: Cancel keeps the project, Don't Save replaces it, and Save writes it + /// to its file first. A project with nothing unsaved is replaced without a word, as is one + /// undone back to the state it was saved in. + #[test] + fn new_asks_before_replacing_unsaved_work() { + let dir = folder("new"); + let file = dir.join("atlas.rgis"); + let mut h = launch(); + frames(&mut h, 2); + h.state_mut().run("project.saveAs", json!({"path": file.display().to_string()})).unwrap(); + edit(&mut h, "Edited"); + h.get_by_label("New project").click(); + frames(&mut h, 3); + assert!(asking(&h) && h.state().session.project.name == "atlas", "it waits for the answer"); + click(&mut h, "Cancel"); + assert_eq!(h.state().session.map().name, "Edited"); + h.get_by_label("New project").click(); + frames(&mut h, 3); + click(&mut h, "Save"); + let saved = std::fs::read_to_string(&file).unwrap(); + assert!(saved.contains("\"Edited\"") && h.state().session.project.name == "Untitled" && !asking(&h)); + // Undone back to the save: nothing is unsaved, so nothing asks. + h.state_mut().run("project.open", json!({"path": file.display().to_string()})).unwrap(); + edit(&mut h, "Again"); + h.state_mut().run("edit.undo", Value::Null).unwrap(); + assert!(!h.state().session.dirty()); + h.get_by_label("New project").click(); + frames(&mut h, 3); + assert!(!asking(&h) && h.state().session.project.name == "Untitled"); + let _ = std::fs::remove_dir_all(&dir); + } + + /// The keyboard shortcut and the Project tab's cards ask as the toolbar does; once answered, + /// the Project tab closes on the project that replaced this one. + #[test] + fn shortcuts_and_the_project_tab_ask_too() { + let mut h = launch(); + frames(&mut h, 2); + edit(&mut h, "Edited"); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::N); + frames(&mut h, 3); + assert!(asking(&h) && h.state().session.project.name == "World Atlas"); + // Shortcuts don't act behind the dialog: Escape answers it, not the map's tool. + h.state_mut().run("tool.set", json!({"tool": "measure"})).unwrap(); + h.key_press(egui::Key::Escape); + frames(&mut h, 3); + assert!(!asking(&h) && h.state().session.tool == rustgis_engine::Tool::Measure); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::N); + frames(&mut h, 3); + click(&mut h, "Don't Save"); + assert_eq!(h.state().session.project.name, "Untitled"); + edit(&mut h, "Edited"); + h.state_mut().run("ui.backstage", json!({"open": true})).unwrap(); + frames(&mut h, 3); + h.get_by_role_and_label(Role::Button, "World Atlas").click(); + frames(&mut h, 3); + assert!(asking(&h) && h.state().session.project.name == "Untitled"); + click(&mut h, "Don't Save"); + assert!(h.state().session.project.name == "World Atlas" && !h.state().ui.backstage); + } + + /// Opening a project, picked or dropped on the window, asks first; Don't Save opens it. A + /// QGIS project, which opens as a map of this one, doesn't ask. + #[test] + fn open_and_drop_ask_before_replacing_unsaved_work() { + let dir = folder("open"); + let other = dir.join("other.rgis"); + let mut h = launch(); + frames(&mut h, 2); + h.state_mut().run("project.saveAs", json!({"path": other.display().to_string()})).unwrap(); + h.state_mut().run("project.sample", Value::Null).unwrap(); + edit(&mut h, "Edited"); + PICKED.with(|p| *p.borrow_mut() = Some(other.display().to_string())); + h.state_mut().run_by_user("ui.open", Value::Null).unwrap(); + frames(&mut h, 2); + assert!(asking(&h) && h.state().session.path.is_none()); + click(&mut h, "Cancel"); + assert!(h.state().session.dirty() && h.state().session.path.is_none()); + h.input_mut().dropped_files.push(std::sync::Arc::new(Dropped(other.clone()))); + frames(&mut h, 2); + assert!(asking(&h)); + click(&mut h, "Don't Save"); + assert!(!h.state().session.dirty() && h.state().session.path.as_deref() == Some(other.as_path())); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Commands run directly never ask: scripts, the AI Assistant and attached agents replace the + /// project as they always have. + #[test] + fn commands_run_directly_never_ask() { + let mut h = launch(); + frames(&mut h, 2); + edit(&mut h, "Edited"); + h.state_mut().run("project.new", Value::Null).unwrap(); + frames(&mut h, 2); + assert!(!asking(&h) && h.state().session.project.name == "Untitled"); + // An agent's command, and a script's line. + edit(&mut h, "Edited"); + h.state_mut().session.run("project.sample", &Value::Null).unwrap(); + assert_eq!(h.state().session.project.name, "World Atlas"); + edit(&mut h, "Edited"); + h.state_mut().run("script.run", json!({"script": "project.new"})).unwrap(); + frames(&mut h, 2); + assert!(!asking(&h) && h.state().session.project.name == "Untitled"); + // What the user runs that doesn't replace the project doesn't ask either. + edit(&mut h, "Edited"); + h.state_mut().run_by_user("map.zoomFull", Value::Null).unwrap(); + frames(&mut h, 2); + assert!(!asking(&h)); + } +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index eb60430e8..2773a652a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -136,7 +136,7 @@ pub fn big( let resp = tooltip(app, resp, &label.replace('\n', " "), desc, id, ¶ms); crate::showme::ring(ui, app, r, id, ¶ms); if resp.clicked() && on && !menu { - let _ = app.run(id, params); + let _ = app.run_by_user(id, params); } resp } @@ -225,7 +225,7 @@ pub fn big_split_menu( let arrow = tooltip(app, arrow, menu_label, menu_desc, "ui.menu", &Value::Null); crate::showme::ring(ui, app, primary.rect, id, ¶ms); if primary.clicked() { - let _ = app.run(id, params); + let _ = app.run_by_user(id, params); } egui::Popup::menu(&arrow).show(|ui| { ui.set_min_width(200.0); @@ -259,7 +259,7 @@ pub fn small(ui: &mut Ui, app: &mut RustGisApp, icon: &str, label: &str, desc: & let resp = tooltip(app, resp, title, if label.is_empty() { "" } else { desc }, id, ¶ms); crate::showme::ring(ui, app, r, id, ¶ms); if resp.clicked() && on { - let _ = app.run(id, params); + let _ = app.run_by_user(id, params); } resp } @@ -347,7 +347,7 @@ pub fn menu_item(ui: &mut Ui, app: &mut RustGisApp, icon: &str, label: &str, id: let on = enabled(app, id); let shortcut = crate::keymap::shortcut_text(ui.ctx(), app, id, ¶ms); if menu_row(ui, icon, tl!(label), &shortcut, on).clicked() { - let _ = app.run(id, params); + let _ = app.run_by_user(id, params); ui.close(); } } diff --git a/crates/ui-egui/tests/autosave.rs b/crates/ui-egui/tests/autosave.rs index 4e80c3e29..d4d325d22 100644 --- a/crates/ui-egui/tests/autosave.rs +++ b/crates/ui-egui/tests/autosave.rs @@ -126,7 +126,7 @@ fn unsaved_work_is_kept_and_offered_back_after_a_crash() { let app = h.state(); assert!(app.session.map().layer_by_name("Inspections").is_some()); assert_eq!(app.session.path.as_deref(), Some(std::path::absolute(&project).unwrap().as_path()), "Save writes back to its file"); - assert!(app.session.dirty, "recovered work is unsaved"); + assert!(app.session.dirty(), "recovered work is unsaved"); // The copy is this run's now, unchanged, until the work is saved. assert!(!file(&dir.0, PENDING).exists() && file(&dir.0, LIVE).is_file()); h.state_mut().run("project.save", json!({})).unwrap(); diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index c4ab4fa43..e0b46b26b 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -801,7 +801,7 @@ fn start_up_preferences() { app.start(&Default::default()).unwrap(); assert!(app.session.map().layers.is_empty()); assert_eq!(app.session.map().projection.code(), "EPSG:3010"); - assert!(!app.session.dirty && !app.session.can_undo()); + assert!(!app.session.dirty() && !app.session.can_undo()); // The web app's `?projection=robinson` without `?blank`: the sample still goes when the // preference says blank; an explicit `Some(false)` keeps it. @@ -929,7 +929,7 @@ fn start_with_a_project_file_or_the_last_one() { assert_eq!(started.session.path.as_deref(), Some(a.as_path())); assert_eq!(started.session.map().projection.code(), "EPSG:3010"); assert!(started.ui.start_view.is_none(), "the Start-up extent isn't applied"); - assert!(!started.session.dirty && !started.session.can_undo()); + assert!(!started.session.dirty() && !started.session.can_undo()); // A shell's blank wins: a file on the command line (true) or --sample (false). for (blank, sample) in [(true, false), (false, true)] { let mut shell = RustGisApp::with_sample().with_services(services()); @@ -2212,7 +2212,7 @@ fn web_save_asks_for_the_file_name() { assert_eq!(h.state().dialogs.open, None); assert_eq!(names(&got), ["Trip plan.rgis"]); assert_eq!(h.state().session.project.name, "Trip plan"); - assert!(!h.state().session.dirty); + assert!(!h.state().session.dirty()); // Saved once: Save downloads under that name again without asking; Save As asks. h.state_mut().run("ui.save", json!(null)).unwrap(); assert_eq!(h.state().dialogs.open, None); @@ -2254,7 +2254,7 @@ fn save_project_dialog_needs_downloads() { h.step(); assert!(h.state().dialogs.error.is_some()); assert_eq!(h.state().session.project.name, "World Atlas"); - assert!(h.state().session.dirty); + assert!(h.state().session.dirty()); } #[test] diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 10564aa09..e5045ded4 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -15,6 +15,20 @@ templates, packages and files for individual items. command line. A package opens as a new, unsaved project; Save writes a plain `.rgis` file. Opening or creating a project clears undo history; saving does not. +A `•` after the project's name in the title bar means it has unsaved changes. Undoing or redoing +back to the state you last saved clears it. Before the desktop app discards unsaved changes, it +asks **Save changes to…?** That happens when you quit or close the window, and when New, Open, the +World Atlas sample, a template or a project dropped on the window would replace the project: + +- **Save** saves first and then goes on. A project that was never saved asks where to write it. + If you cancel that, or the save fails, nothing is discarded. +- **Don't Save** goes on without saving. +- **Cancel** (or Escape) goes back to the project. + +Commands don't ask: `project.new`, `project.open` and the rest replace the project as they always +have when a script, a task, the AI Assistant, the command line or an attached agent runs them. +The browser app doesn't ask yet; it warns before you leave a page with unsaved changes. + Saved layer addresses omit passwords, access keys, tokens and signatures. The open session keeps them, but after restarting RustGIS a protected source may need you to sign in to its connection again or add it again with credentials. @@ -46,8 +60,8 @@ that has unsaved changes of its own: save it first. The same choices are commands: `ui.recover` opens the copy, and `ui.recover={"discard": true}` deletes it. `ui.autosave` reports the interval and any copy waiting. -Quitting doesn't ask about unsaved changes, and their copy is removed: save before you quit. A -second copy of RustGIS running alongside the first doesn't autosave, so it can't replace the first +Quitting asks about unsaved changes first (see [Projects](#projects)); once you quit, their copy +is removed. A second copy of RustGIS running alongside the first doesn't autosave, so it can't replace the first one's copy; it says so as it starts. Nor does RustGIS autosave when its settings folder can't lock files (some network or synced folders), where it can't tell a second copy apart. The browser app doesn't autosave yet; it warns before you leave a page with unsaved changes. From c9151835543e7b9681698c4b99790613dc623799 Mon Sep 17 00:00:00 2001 From: mavrick Date: Mon, 26 Oct 2026 09:47:28 -0500 Subject: [PATCH 2/4] fix(ui): keep the Project tab open while the save prompt asks Choosing New, World Atlas or Open from the Project tab closed the tab even when the save prompt only asked, so Cancel dropped the user on the map. The tab now stays until the answer. Also: say why reading the save's result at once is safe (the desktop saves synchronously); note in the user guide that macOS Quit (Cmd+Q, the Dock) doesn't ask yet; and move the tests main gained since to Session::dirty(). Co-Authored-By: Claude Opus 5.5 --- crates/engine/tests/move_shared_vertex.rs | 6 +++--- crates/ui-egui/src/backstage.rs | 12 ++++++++---- crates/ui-egui/src/lib.rs | 8 ++++++-- crates/ui-egui/src/saveprompt.rs | 16 +++++++++++++++- crates/ui-egui/tests/move_shared_vertex.rs | 2 +- docs/user-guide/workspace.md | 4 +++- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/engine/tests/move_shared_vertex.rs b/crates/engine/tests/move_shared_vertex.rs index 2e1805c4c..aab384fc3 100644 --- a/crates/engine/tests/move_shared_vertex.rs +++ b/crates/engine/tests/move_shared_vertex.rs @@ -232,13 +232,13 @@ fn later_rule_rejection_rolls_back_the_whole_batch() { s.run("edit.undo", &Value::Null).unwrap(); let before = serde_json::to_value(&s.project).unwrap(); let counts = s.edit_counts.clone(); - let dirty = s.dirty; + let dirty = s.dirty(); assert!(s.can_redo()); let error = s.run("edit.moveSharedVertex", &json!({"layer":id,"row":0,"index":0,"lon":0.2,"lat":0.3})).unwrap_err(); assert!(error.contains("reject later parcel")); assert_eq!(serde_json::to_value(&s.project).unwrap(), before); assert_eq!(s.edit_counts, counts); - assert_eq!(s.dirty, dirty); + assert_eq!(s.dirty(), dirty); assert_eq!(s.undo_depth(), 0); assert!(s.can_redo()); } @@ -250,7 +250,7 @@ fn same_position_runs_calculations_and_keeps_existing_undo_behaviour() { assert_eq!(move_to(&mut s, id, Coord::new(0., 0.))["modified"], 0); assert_eq!(s.edit_counts[&id].modified, 4); assert_eq!(s.undo_depth(), 1); - assert!(s.dirty); + assert!(s.dirty()); s.map_mut().layer_mut(id).unwrap().design.rules.push(AttributeRule { name: "increment".into(), kind: RuleKind::Calculation, diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index b6c98f703..2c3f9dbc5 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -108,13 +108,17 @@ fn start(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ui.spacing_mut().item_spacing = vec2(18.0, 0.0); if card(ui, t, "Map", "A blank map with a basemap", "new_map") { let _ = app.run_by_user("project.new", Value::Null); - app.ui.backstage = false; - app.map.invalidate(); + if !crate::saveprompt::waiting(app) { + app.ui.backstage = false; + app.map.invalidate(); + } } if card(ui, t, "World Atlas", "Countries, cities, rivers and lakes", "basemap") { let _ = app.run_by_user("project.sample", Value::Null); - app.ui.backstage = false; - app.map.invalidate(); + if !crate::saveprompt::waiting(app) { + app.ui.backstage = false; + app.map.invalidate(); + } } if card(ui, t, "Open…", "Open a .rgis project", "open") { let _ = app.run("ui.open", Value::Null); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index b6ac3e024..f67ea5a59 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2460,8 +2460,12 @@ impl RustGisApp { let lower = path.to_ascii_lowercase(); let cmd = if lower.ends_with(".qgs") || lower.ends_with(".qgz") { "project.importQgis" } else { "project.open" }; let r = self.run_by_user(cmd, json!({"path": path})); - self.ui.backstage = false; - self.map.invalidate(); + // Asking to save first: the Project tab stays until the answer, so Cancel + // goes back to it. + if !saveprompt::waiting(self) { + self.ui.backstage = false; + self.map.invalidate(); + } return r; } } diff --git a/crates/ui-egui/src/saveprompt.rs b/crates/ui-egui/src/saveprompt.rs index a582b0b0f..89cc2ea15 100644 --- a/crates/ui-egui/src/saveprompt.rs +++ b/crates/ui-egui/src/saveprompt.rs @@ -47,6 +47,12 @@ pub(crate) fn ask(app: &mut RustGisApp, what: Discard) { } } +/// Whether the dialog is waiting for an answer: what asked it waits too (the Project tab stays +/// open, so Cancel goes back to it; it closes once the project is replaced). +pub(crate) fn waiting(app: &RustGisApp) -> bool { + app.save_prompt.asking.is_some() +} + /// Each frame, before anything draws: a request to close the window (its close button, the /// system's Quit, or `ui.quit`) waits while the project has unsaved changes, and the dialog asks. pub(crate) fn close_requested(app: &mut RustGisApp, ctx: &egui::Context) { @@ -95,7 +101,10 @@ pub(crate) fn show(app: &mut RustGisApp, ctx: &egui::Context) { ctx.request_repaint(); match answer { // Save As when the project has no file. A save that fails says why (its notice), and one - // cancelled leaves the changes unsaved: either way nothing is discarded. + // cancelled leaves the changes unsaved: either way nothing is discarded. This reads the + // result at once: on the desktop, the only shell that asks (`asks`), `ui.save` writes the + // file, and Save As's picker returns, before it does. A shell that saved later (the + // browser's download dialog) would have to go on from there instead. Answer::Save => { if app.run("ui.save", Value::Null).is_ok() && !app.session.dirty() { proceed(app, what); @@ -344,6 +353,11 @@ mod tests { h.get_by_role_and_label(Role::Button, "World Atlas").click(); frames(&mut h, 3); assert!(asking(&h) && h.state().session.project.name == "Untitled"); + // Cancel goes back to the Project tab, not the map. + click(&mut h, "Cancel"); + assert!(h.state().ui.backstage && h.state().session.project.name == "Untitled"); + h.get_by_role_and_label(Role::Button, "World Atlas").click(); + frames(&mut h, 3); click(&mut h, "Don't Save"); assert!(h.state().session.project.name == "World Atlas" && !h.state().ui.backstage); } diff --git a/crates/ui-egui/tests/move_shared_vertex.rs b/crates/ui-egui/tests/move_shared_vertex.rs index 1d19e14da..e492d1cc5 100644 --- a/crates/ui-egui/tests/move_shared_vertex.rs +++ b/crates/ui-egui/tests/move_shared_vertex.rs @@ -99,7 +99,7 @@ fn ribbon_toggle_routes_polygon_drag_and_undoes_all_neighbours() { h.run(); assert!(h.state().map.move_shared_vertices); assert_eq!(h.state().session.undo_depth(), 0); - assert!(!h.state().session.dirty); + assert!(!h.state().session.dirty()); let to = drag(&mut h, c(0., 0.), c(45., 30.)); let at = h.state().map.vertex_drag.unwrap().at; assert_eq!(geometries(&h), before); diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 42554651f..dc22d1e2d 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -27,7 +27,9 @@ World Atlas sample, a template or a project dropped on the window would replace Commands don't ask: `project.new`, `project.open` and the rest replace the project as they always have when a script, a task, the AI Assistant, the command line or an attached agent runs them. -The browser app doesn't ask yet; it warns before you leave a page with unsaved changes. +The browser app doesn't ask yet; it warns before you leave a page with unsaved changes. On macOS, +**Quit RustGIS** (⌘Q) and the Dock's **Quit** don't ask yet either: close the window instead, or +save first. Saved layer addresses omit passwords, access keys, tokens and signatures. The open session keeps them, but after restarting RustGIS a protected source may need you to sign in to its From 9a9ce6512113840585b3c500cfed4a695b822bfd Mon Sep 17 00:00:00 2001 From: mavrick Date: Mon, 26 Oct 2026 11:06:00 -0500 Subject: [PATCH 3/4] fix(ui): keep unsaved work when the app is quit without the save prompt On macOS, Quit (Cmd+Q, the Dock) ends the app without asking the window to close, so the save prompt never shows, and the clean exit then removed the recovery copy: the work was lost. Now an exit with unsaved changes the user never chose to discard writes a last copy and keeps it, marked as quit, and the next start offers it back ("RustGIS was quit last time without saving your changes."). With autosave off nothing is kept. The autosave tests that quit with unsaved work now answer Don't Save in the dialog, the only way to that exit elsewhere. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/autosave.rs | 42 ++++++++++++++++++--------- crates/ui-egui/src/saveprompt.rs | 7 +++++ crates/ui-egui/tests/autosave.rs | 50 ++++++++++++++++++++++++++++++-- docs/user-guide/workspace.md | 11 ++++--- 4 files changed, 90 insertions(+), 20 deletions(-) diff --git a/crates/ui-egui/src/autosave.rs b/crates/ui-egui/src/autosave.rs index 274a730c8..513601a91 100644 --- a/crates/ui-egui/src/autosave.rs +++ b/crates/ui-egui/src/autosave.rs @@ -38,6 +38,11 @@ pub struct Recovery { /// When the copy was made, in seconds since 1970 (UTC). #[serde(default)] pub time: u64, + /// Kept as the app was quit without its question about unsaved changes answered (macOS's + /// Quit, which closes without asking the window), not left by a run that didn't close. + /// Copies written before it was known read as `false`. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub quit: bool, } /// Where a shell keeps the recovery copy. Writing and clearing may happen in the background, but @@ -126,7 +131,7 @@ fn now_secs() -> u64 { /// a lossy spelling of the name. fn about(app: &RustGisApp) -> Recovery { let path = app.session.path.as_ref().and_then(|p| std::path::absolute(p).unwrap_or_else(|_| p.clone()).to_str().map(str::to_string)); - Recovery { name: app.session.project.name.clone(), path, time: now_secs() } + Recovery { name: app.session.project.name.clone(), path, time: now_secs(), quit: false } } /// Reads the interval preference and the copy an earlier run left (from [`RustGisApp::with_services`]). @@ -215,9 +220,18 @@ pub(crate) fn tick(app: &mut RustGisApp, ctx: &egui::Context) { /// A clean exit: this run's copy goes, and what was asked of the store is done before the /// process ends. The copy an earlier run left stays until it is recovered or discarded. +/// +/// Unless the work in it would be lost unasked: the app was quit with unsaved changes and the +/// save prompt never answered (macOS's Quit, which ends the app without asking the window to +/// close). Then a last copy is written and kept, and the next start offers it back. With +/// autosave off there is no copy to keep. pub(crate) fn end(app: &mut RustGisApp) { if let Some(store) = &app.services.recovery { - store.clear(); + if app.autosave.minutes > 0 && crate::saveprompt::unanswered(app) { + store.write(app.session.project.clone(), Recovery { quit: true, ..about(app) }); + } else { + store.clear(); + } store.flush(); } app.autosave.written = None; @@ -287,20 +301,16 @@ pub(crate) fn recover(app: &mut RustGisApp, p: &Value) -> Result Ok(json!({"name": app.session.project.name, "path": offered.path, "waiting": waiting})) } -/// The dialog's first line: what was kept, and how long ago (`then`, seconds since 1970). Whole -/// sentences, one per unit, so each translates as a sentence; only data is filled in. +/// The dialog's second line: what was kept, and how long ago (`then`, seconds since 1970). +/// Whole sentences, one per unit, so each translates as a sentence; only data is filled in. fn kept_line(name: &str, then: u64) -> String { let secs = now_secs().saturating_sub(then); let (template, n) = match secs { - 0..60 => ("RustGIS didn't close properly last time. A copy of {name} with unsaved changes was kept less than a minute ago.", 0), - 60..120 => ("RustGIS didn't close properly last time. A copy of {name} with unsaved changes was kept a minute ago.", 1), - 120..7_200 => { - ("RustGIS didn't close properly last time. A copy of {name} with unsaved changes was kept {n} minutes ago.", secs / 60) - } - 7_200..172_800 => { - ("RustGIS didn't close properly last time. A copy of {name} with unsaved changes was kept {n} hours ago.", secs / 3_600) - } - _ => ("RustGIS didn't close properly last time. A copy of {name} with unsaved changes was kept {n} days ago.", secs / 86_400), + 0..60 => ("A copy of {name} with unsaved changes was kept less than a minute ago.", 0), + 60..120 => ("A copy of {name} with unsaved changes was kept a minute ago.", 1), + 120..7_200 => ("A copy of {name} with unsaved changes was kept {n} minutes ago.", secs / 60), + 7_200..172_800 => ("A copy of {name} with unsaved changes was kept {n} hours ago.", secs / 3_600), + _ => ("A copy of {name} with unsaved changes was kept {n} days ago.", secs / 86_400), }; crate::i18n::fmt(template, &[("name", name), ("n", &n.to_string())]) } @@ -320,6 +330,12 @@ pub(crate) fn offer(app: &mut RustGisApp, ctx: &egui::Context) { ui.set_width(420.0); ui.label(egui::RichText::new(tl!("Recover unsaved work?")).font(semibold(15.0))); ui.add_space(4.0); + // Why there is a copy, then what it is. + ui.label(if r.quit { + tl!("RustGIS was quit last time without saving your changes.") + } else { + tl!("RustGIS didn't close properly last time.") + }); ui.label(kept_line(&name, r.time)); ui.add_space(4.0); match &file { diff --git a/crates/ui-egui/src/saveprompt.rs b/crates/ui-egui/src/saveprompt.rs index 89cc2ea15..ebf6640f7 100644 --- a/crates/ui-egui/src/saveprompt.rs +++ b/crates/ui-egui/src/saveprompt.rs @@ -53,6 +53,13 @@ pub(crate) fn waiting(app: &RustGisApp) -> bool { app.save_prompt.asking.is_some() } +/// Whether the app ends with unsaved changes the user hasn't agreed to lose: the project has +/// some, this shell asks, and the dialog wasn't answered with Don't Save (or Save, which leaves +/// none). Only an end that skipped the close request gets here: macOS's Quit. +pub(crate) fn unanswered(app: &RustGisApp) -> bool { + app.session.dirty() && app.opens_files() && !app.save_prompt.closing +} + /// Each frame, before anything draws: a request to close the window (its close button, the /// system's Quit, or `ui.quit`) waits while the project has unsaved changes, and the dialog asks. pub(crate) fn close_requested(app: &mut RustGisApp, ctx: &egui::Context) { diff --git a/crates/ui-egui/tests/autosave.rs b/crates/ui-egui/tests/autosave.rs index d4d325d22..1115841bf 100644 --- a/crates/ui-egui/tests/autosave.rs +++ b/crates/ui-egui/tests/autosave.rs @@ -83,6 +83,17 @@ fn file(dir: &Path, name: &str) -> PathBuf { dir.join("recovery").join(name) } +/// Quits as the user does, with unsaved work: the window's close is asked for, the save prompt +/// answered Don't Save, and the app ends cleanly (`on_exit`). +fn quit_without_saving(h: &mut egui_kittest::Harness<'_, RustGisApp>) { + h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + frames(h, 3); + h.get_by_role_and_label(Role::Button, "Don't Save").click(); + frames(h, 3); + assert!(h.state().quit_requested); + h.state_mut().end_autosave(); +} + /// An edit to the project: a new layer. fn edit(h: &mut egui_kittest::Harness<'_, RustGisApp>, name: &str) { h.state_mut().run("layer.template", json!({"template": "points", "name": name})).unwrap(); @@ -143,7 +154,7 @@ fn a_clean_exit_leaves_nothing_to_recover() { edit(&mut h, "Unsaved"); settle(&mut h); assert!(file(&dir.0, LIVE).is_file()); - h.state_mut().end_autosave(); + quit_without_saving(&mut h); assert!(!file(&dir.0, LIVE).exists()); drop(h); let mut h = launch(&dir.0, &prefs); @@ -152,6 +163,39 @@ fn a_clean_exit_leaves_nothing_to_recover() { assert!(h.state_mut().run("ui.recover", json!({})).is_err(), "nothing to recover"); } +/// The app ended with unsaved work and the save prompt never answered (macOS's Quit, which +/// skips the window's close request): the work is kept, though the exit was clean, and the next +/// start offers it back, saying it was quit. With autosave off nothing is kept. +#[test] +fn quitting_unasked_keeps_the_work() { + let dir = TempDir::new("unasked"); + let prefs = Prefs::default(); + let mut h = launch(&dir.0, &prefs); + frames(&mut h, 2); + // Edited after the last copy: the one kept is the work as it was at the end. + edit(&mut h, "Early"); + settle(&mut h); + edit(&mut h, "Last"); + h.state_mut().end_autosave(); + drop(h); + let mut h = launch(&dir.0, &prefs); + h.step(); + h.get_by_label("RustGIS was quit last time without saving your changes."); + assert!(h.query_by_label("RustGIS didn't close properly last time.").is_none()); + h.get_by_role_and_label(Role::Button, "Recover").click(); + settle(&mut h); + assert!(h.state().session.map().layer_by_name("Last").is_some() && h.state().session.dirty()); + // Autosave off: no copy to keep. + h.state_mut().run("project.new", json!({})).unwrap(); + h.state_mut().run("ui.autosave", json!({"minutes": 0})).unwrap(); + edit(&mut h, "Gone"); + h.state_mut().end_autosave(); + drop(h); + let mut h = launch(&dir.0, &prefs); + h.step(); + assert!(h.query_by_label("Recover unsaved work?").is_none()); +} + #[test] fn later_offers_it_again_and_discard_deletes_it() { let dir = TempDir::new("later"); @@ -170,7 +214,7 @@ fn later_offers_it_again_and_discard_deletes_it() { assert!(h.state().session.map().layer_by_name("Kept").is_none()); edit(&mut h, "Other"); settle(&mut h); - h.state_mut().end_autosave(); + quit_without_saving(&mut h); drop(h); let mut h = launch(&dir.0, &prefs); assert!(std::fs::read_to_string(file(&dir.0, PENDING)).unwrap().contains("Kept")); @@ -279,7 +323,7 @@ fn copies_from_two_crashes_are_both_offered_in_turn() { settle(&mut h); assert!(h.state().session.map().layer_by_name("Second").is_some(), "the newest first"); assert!(h.query_by_label("Recover unsaved work?").is_none(), "the other waits for the next start"); - h.state_mut().end_autosave(); + quit_without_saving(&mut h); drop(h); let mut h = launch(&dir.0, &prefs); diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index dc22d1e2d..2046322e8 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -28,8 +28,9 @@ World Atlas sample, a template or a project dropped on the window would replace Commands don't ask: `project.new`, `project.open` and the rest replace the project as they always have when a script, a task, the AI Assistant, the command line or an attached agent runs them. The browser app doesn't ask yet; it warns before you leave a page with unsaved changes. On macOS, -**Quit RustGIS** (⌘Q) and the Dock's **Quit** don't ask yet either: close the window instead, or -save first. +**Quit RustGIS** (⌘Q) and the Dock's **Quit** don't ask yet either. Your changes aren't lost: +RustGIS keeps them as a [recovery copy](#autosave-and-recovery) and offers them back the next time +it starts. Saved layer addresses omit passwords, access keys, tokens and signatures. The open session keeps them, but after restarting RustGIS a protected source may need you to sign in to its @@ -62,8 +63,10 @@ that has unsaved changes of its own: save it first. The same choices are commands: `ui.recover` opens the copy, and `ui.recover={"discard": true}` deletes it. `ui.autosave` reports the interval and any copy waiting. -Quitting asks about unsaved changes first (see [Projects](#projects)); once you quit, their copy -is removed. A second copy of RustGIS running alongside the first doesn't autosave, so it can't replace the first +Quitting asks about unsaved changes first (see [Projects](#projects)); once you choose +**Don't Save**, their copy is removed. If RustGIS is quit without that answer (on macOS, **Quit** +doesn't ask yet), it keeps a last copy of the work instead, and the next launch offers it back, +saying RustGIS was quit without saving. With autosave off, nothing is kept. A second copy of RustGIS running alongside the first doesn't autosave, so it can't replace the first one's copy; it says so as it starts. Nor does RustGIS autosave when its settings folder can't lock files (some network or synced folders), where it can't tell a second copy apart. The browser app doesn't autosave yet; it warns before you leave a page with unsaved changes. From 863e984f18f5e95b77851a9188aa6f80db10f66a Mon Sep 17 00:00:00 2001 From: mavrick Date: Mon, 26 Oct 2026 12:35:32 -0500 Subject: [PATCH 4/4] fix(ui): ask about the project as it is when answered; hold the rest of a drop Review round 3 on the save prompt: - Don't Save no longer discards changes made while the dialog showed (an agent, the AI Assistant): the dialog says the project changed and asks again. Session::state exposes which state the project is in. - A drop opens its project files first, so data dropped with a project goes into that project; while the prompt asks, the rest of the drop waits unopened and a notice says how many. - Dropped project files open through the prompt now that drops go through layerchooser::add_file (#952). - Command Search closes on its own Escape: the shortcut branch that closed it under a modal could no longer run. - Docs: macOS Quit keeps the work only with autosave on; the recovery copy goes on Don't Save, not on every quit. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lib.rs | 6 +++ crates/ui-egui/src/backstage.rs | 2 +- crates/ui-egui/src/chrome.rs | 4 -- crates/ui-egui/src/lib.rs | 22 +++++++++- crates/ui-egui/src/saveprompt.rs | 74 +++++++++++++++++++++++++++++--- docs/user-guide/workspace.md | 22 +++++----- 6 files changed, 106 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 68ff58caf..c71bae0f4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1268,6 +1268,12 @@ impl Session { self.saved != Some(self.state) } + /// Which state the project is in: another after each change, an earlier one after undo. A + /// front end compares it to tell whether the project changed since it last looked. + pub fn state(&self) -> u64 { + self.state + } + /// The project as it is now is saved (written to its file, or downloaded in the browser). pub fn mark_saved(&mut self) { self.saved = Some(self.state); diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 2c3f9dbc5..7c4bd0dff 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -147,7 +147,7 @@ fn start(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { }); } -/// A New Project card: a thumbnail with its title and line beneath, a button named by its title. +/// A start card: a thumbnail with its title and line beneath, a button named by its title. /// True when clicked. fn card(ui: &mut Ui, t: &Tokens, title: &str, sub: &str, icon: &str) -> bool { let (r, resp) = ui.allocate_exact_size(vec2(200.0, 160.0), Sense::click()); diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 6d7be286e..99c83758d 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -554,10 +554,6 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { if id == "tool.set" { app.map.cancel_sketch(); app.layout.cancel_tool(); - if app.ui.command_search.is_some() { - app.ui.command_search = None; - continue; - } } // An arrow key nudged the selected layout element (unless it's locked): a held key, or // taps close together, are one step to undo. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 55c78f10a..8996bbfe7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3620,12 +3620,20 @@ impl RustGisApp { // Files dropped on the desktop window arrive as paths. (The web shell reads browser drops // asynchronously and passes them through the inbox instead.) if !cfg!(target_arch = "wasm32") { - let dropped = ctx.input_mut(|i| std::mem::take(&mut i.raw.dropped_files)); + let mut dropped = ctx.input_mut(|i| std::mem::take(&mut i.raw.dropped_files)); + // A project first, so the data dropped with it goes into that project, not the one + // it replaces. While the save prompt asks about replacing this one, the rest wait. + dropped.sort_by_key(|f| rustgis_engine::project::open_command(&f.path().display().to_string()) != "project.open"); + let mut held = 0usize; for f in dropped { let p = f.path().display().to_string(); if p.is_empty() { continue; } + if saveprompt::waiting(self) { + held += 1; + continue; + } if p.to_ascii_lowercase().ends_with(".rgplugin") { match std::fs::read(f.path()) { Ok(bytes) => pluginsui::received(self, bytes, None), @@ -3636,6 +3644,18 @@ impl RustGisApp { let _ = layerchooser::add_file(self, &p); self.map.invalidate(); } + if held == 1 { + self.session.notify( + tl!("Another dropped file wasn't opened: drop it again once you've answered about the unsaved changes."), + false, + ); + } else if held > 1 { + let text = crate::i18n::fmt( + "{n} other dropped files weren't opened: drop them again once you've answered about the unsaved changes.", + &[("n", &held.to_string())], + ); + self.session.notify(text, false); + } } if self.fonts_frames == 0 { self.add_cjk_fallback(); diff --git a/crates/ui-egui/src/saveprompt.rs b/crates/ui-egui/src/saveprompt.rs index ebf6640f7..bf8ae6921 100644 --- a/crates/ui-egui/src/saveprompt.rs +++ b/crates/ui-egui/src/saveprompt.rs @@ -31,19 +31,32 @@ pub(crate) struct SavePromptState { pub asking: Option, /// The user chose to quit: the next close request goes through. pub closing: bool, + /// The project's state when asked ([`rustgis_engine::Session::state`]): Don't Save discards + /// the changes the user was asked about, not ones an agent made while the dialog showed. + pub asked: u64, + /// The project changed while the dialog showed: it asks again, and says why. + pub changed: bool, } /// Whether running command `id` from the interface asks first: it replaces the project, which /// has unsaved changes, in the desktop app. pub(crate) fn asks(app: &RustGisApp, id: &str) -> bool { - REPLACES.contains(&id) && app.session.dirty() && app.opens_files() + REPLACES.contains(&id) && unsaved_here(app) } -/// Shows the dialog for `what`. One already showing keeps its question: a file dropped on the -/// window meanwhile doesn't replace it. Quitting does, as the user asked for it last. +/// Whether the project has unsaved changes in a shell that asks about them (the desktop app). +fn unsaved_here(app: &RustGisApp) -> bool { + app.session.dirty() && app.opens_files() +} + +/// Shows the dialog for `what`, about the project as it is now. One already showing keeps its +/// question (the dialog takes the pointer and the keys; a drop that opens a project waits with +/// the rest of its files). Quitting does replace it, as the user asked for it last. pub(crate) fn ask(app: &mut RustGisApp, what: Discard) { if app.save_prompt.asking.is_none() || what == Discard::Quit { app.save_prompt.asking = Some(what); + app.save_prompt.asked = app.session.state(); + app.save_prompt.changed = false; } } @@ -57,7 +70,7 @@ pub(crate) fn waiting(app: &RustGisApp) -> bool { /// some, this shell asks, and the dialog wasn't answered with Don't Save (or Save, which leaves /// none). Only an end that skipped the close request gets here: macOS's Quit. pub(crate) fn unanswered(app: &RustGisApp) -> bool { - app.session.dirty() && app.opens_files() && !app.save_prompt.closing + unsaved_here(app) && !app.save_prompt.closing } /// Each frame, before anything draws: a request to close the window (its close button, the @@ -66,7 +79,7 @@ pub(crate) fn close_requested(app: &mut RustGisApp, ctx: &egui::Context) { if !ctx.input(|i| i.viewport().close_requested()) || app.save_prompt.closing { return; } - if !(app.session.dirty() && app.opens_files()) { + if !unsaved_here(app) { return; } ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); @@ -87,6 +100,12 @@ pub(crate) fn show(app: &mut RustGisApp, ctx: &egui::Context) { ui.label(egui::RichText::new(crate::i18n::fmt("Save changes to {name}?", &[("name", &name)])).font(semibold(15.0))); ui.add_space(4.0); ui.label(tl!("Your changes will be lost if you don't save them.")); + if app.save_prompt.changed { + ui.add_space(4.0); + ui.label( + egui::RichText::new(tl!("The project changed while this was open: check the changes, then answer again.")).color(t.red), + ); + } ui.add_space(10.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Min), |ui| { if crate::widgets::dialog_button(ui, tl!("Save"), Some(t.accent), vec2(84.0, 28.0)).clicked() { @@ -103,6 +122,13 @@ pub(crate) fn show(app: &mut RustGisApp, ctx: &egui::Context) { }); }); let Some(answer) = answer else { return }; + // Changed since the question (an agent, the AI Assistant): Don't Save would discard changes + // the user hasn't seen. The dialog stays and asks about the project as it is now. + if matches!(answer, Answer::DontSave) && app.session.state() != app.save_prompt.asked { + app.save_prompt.asked = app.session.state(); + app.save_prompt.changed = true; + return; + } app.save_prompt.asking = None; // The shell closes the window, or the replaced project draws, on the next frame. ctx.request_repaint(); @@ -350,6 +376,12 @@ mod tests { h.key_press(egui::Key::Escape); frames(&mut h, 3); assert!(!asking(&h) && h.state().session.tool == rustgis_engine::Tool::Measure); + // Command Search closes on its own Escape too, leaving the tool. + h.state_mut().run("ui.commandSearch", Value::Null).unwrap(); + frames(&mut h, 3); + h.key_press(egui::Key::Escape); + frames(&mut h, 3); + assert!(h.state().ui.command_search.is_none() && h.state().session.tool == rustgis_engine::Tool::Measure); h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::N); frames(&mut h, 3); click(&mut h, "Don't Save"); @@ -386,14 +418,42 @@ mod tests { assert!(asking(&h) && h.state().session.path.is_none()); click(&mut h, "Cancel"); assert!(h.state().session.dirty() && h.state().session.path.is_none()); - h.input_mut().dropped_files.push(std::sync::Arc::new(Dropped(other.clone()))); + // Data dropped before a project, and a second project: the project is asked about first; + // the rest wait, unopened (the data isn't added to the project Don't Save discards), and + // a notice says so. + let data = dir.join("sites.geojson"); + std::fs::write(&data, r#"{"type":"FeatureCollection","features":[]}"#).unwrap(); + let said = h.state().session.notices.len(); + for f in [data.clone(), other.clone(), other.clone()] { + h.input_mut().dropped_files.push(std::sync::Arc::new(Dropped(f))); + } frames(&mut h, 2); - assert!(asking(&h)); + assert!(asking(&h) && h.state().session.map().layer_by_name("sites").is_none()); + assert!(h.state().session.notices[said..].iter().any(|n| n.text.starts_with("2 other dropped files weren't opened"))); click(&mut h, "Don't Save"); assert!(!h.state().session.dirty() && h.state().session.path.as_deref() == Some(other.as_path())); let _ = std::fs::remove_dir_all(&dir); } + /// The project changed while the dialog showed (an agent's edit): Don't Save doesn't discard + /// what the user wasn't asked about. The dialog says so and asks again; the next answer goes. + #[test] + fn a_change_while_asking_is_asked_about_again() { + let mut h = launch(); + frames(&mut h, 2); + edit(&mut h, "Edited"); + h.state_mut().run_by_user("project.new", Value::Null).unwrap(); + frames(&mut h, 2); + h.state_mut().session.actor = Some("agent 1".into()); + h.state_mut().session.run("map.rename", &json!({"name": "By the agent"})).unwrap(); + h.state_mut().session.actor = None; + click(&mut h, "Don't Save"); + assert!(asking(&h) && h.state().session.map().name == "By the agent"); + assert!(h.query_by_label_contains("The project changed while this was open").is_some()); + click(&mut h, "Don't Save"); + assert!(!asking(&h) && h.state().session.project.name == "Untitled"); + } + /// Commands run directly never ask: scripts, the AI Assistant and attached agents replace the /// project as they always have. #[test] diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 2046322e8..a3bb11aba 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -28,9 +28,9 @@ World Atlas sample, a template or a project dropped on the window would replace Commands don't ask: `project.new`, `project.open` and the rest replace the project as they always have when a script, a task, the AI Assistant, the command line or an attached agent runs them. The browser app doesn't ask yet; it warns before you leave a page with unsaved changes. On macOS, -**Quit RustGIS** (⌘Q) and the Dock's **Quit** don't ask yet either. Your changes aren't lost: -RustGIS keeps them as a [recovery copy](#autosave-and-recovery) and offers them back the next time -it starts. +**Quit RustGIS** (⌘Q) and the Dock's **Quit** don't ask yet either. With autosave on (the +default), RustGIS keeps your changes as a [recovery copy](#autosave-and-recovery) and offers them +back the next time it starts; with autosave off, they are lost, so save before you quit. Saved layer addresses omit passwords, access keys, tokens and signatures. The open session keeps them, but after restarting RustGIS a protected source may need you to sign in to its @@ -45,7 +45,7 @@ the last one. Choose how often, or turn it off, in **Settings › Autosave** project doesn't hold up the app. It leaves out layer passwords, keys and tokens, as a saved project does. It is kept in the `recovery` folder of the app's [data folder](../installation.md#settings-data-and-cache-folders), not beside your project. -Saving, opening or starting another project, and quitting RustGIS remove it. +Saving, opening or starting another project, and choosing **Don't Save** when you quit remove it. If RustGIS crashes, is closed by the system or loses power, the next launch asks **Recover unsaved work?** and says how old the copy is. @@ -63,13 +63,13 @@ that has unsaved changes of its own: save it first. The same choices are commands: `ui.recover` opens the copy, and `ui.recover={"discard": true}` deletes it. `ui.autosave` reports the interval and any copy waiting. -Quitting asks about unsaved changes first (see [Projects](#projects)); once you choose -**Don't Save**, their copy is removed. If RustGIS is quit without that answer (on macOS, **Quit** -doesn't ask yet), it keeps a last copy of the work instead, and the next launch offers it back, -saying RustGIS was quit without saving. With autosave off, nothing is kept. A second copy of RustGIS running alongside the first doesn't autosave, so it can't replace the first -one's copy; it says so as it starts. Nor does RustGIS autosave when its settings folder can't lock -files (some network or synced folders), where it can't tell a second copy apart. The browser app -doesn't autosave yet; it warns before you leave a page with unsaved changes. +Quitting asks about unsaved changes first (see [Projects](#projects)). If RustGIS is quit without +that answer (on macOS, **Quit** doesn't ask yet), it keeps a last copy of the work instead, and +the next launch offers it back, saying RustGIS was quit without saving. With autosave off, nothing +is kept. A second copy of RustGIS running alongside the first doesn't autosave, so it can't +replace the first one's copy; it says so as it starts. Nor does RustGIS autosave when its settings +folder can't lock files (some network or synced folders), where it can't tell a second copy apart. +The browser app doesn't autosave yet; it warns before you leave a page with unsaved changes. ### QGIS projects and styles