Workspaces: Save, Save As, Rename and Delete fail when the file isn't written (#696)

crates/ui-egui/src/lib.rs

@@ -428,9 +428,16 @@ impl EffectcraftApp {
     }
 
     /// Keep the saved workspaces for the next session (see [`EffectcraftApp::new`]): After
-    /// Effects' saved workspaces last until they are deleted (#320).
-    pub(crate) fn store_saved_workspaces(&mut self) {
-        self.save_workspace_state();
+    /// Effects' saved workspaces last until they are deleted (#320). `Err` when they weren't
+    /// written (#696): the layouts stay usable and a later save retries, but a restart loses them.
+    pub(crate) fn store_saved_workspaces(&mut self) -> Result<(), String> {
+        let Err(error) = self.workspace_store.save_now(self.session.config.as_deref(), &self.ui) else { return Ok(()) };
+        let message = format!("Workspaces are kept for this session only and won't be there after a restart: {error}");
+        log::warn!("{message}");
+        self.ui.status = message.clone();
+        // The raw error, so the background retry doesn't report the same failure again.
+        self.workspace_store.error = Some(error);
+        Err(message)
     }
 
     /// Offer the frontend's commands (tools, timeline navigation…) for keyboard shortcuts.

crates/ui-egui/src/menus.rs

@@ -1017,7 +1017,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
             let name = app.ui.workspace.clone();
             app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone());
             app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone());
-            app.store_saved_workspaces();
+            app.store_saved_workspaces()?;
             json!({"workspace": name})
         }
         "window.saveWorkspaceAs" => {
@@ -1034,7 +1034,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
             app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone());
             app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone());
             app.ui.workspace = name.clone();
-            app.store_saved_workspaces();
+            app.store_saved_workspaces()?;
             json!({"workspace": name})
         }
         "window.editWorkspaces" => {
@@ -1063,7 +1063,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
                 if app.ui.workspace == name {
                     app.set_workspace("Default");
                 }
-                app.store_saved_workspaces();
+                app.store_saved_workspaces()?;
                 return Ok(json!({"deleted": name}));
             }
             if let Some(new) = p.get("rename").and_then(Value::as_str).map(str::trim).filter(|n| !n.is_empty() && *n != name) {
@@ -1078,7 +1078,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
                 if app.ui.workspace == name {
                     app.ui.workspace = new.to_string();
                 }
-                app.store_saved_workspaces();
+                app.store_saved_workspaces()?;
                 return Ok(json!({"renamed": new}));
             }
             json!({"workspaces": app.workspace_names()})

crates/ui-egui/src/native_menu.rs

@@ -429,6 +429,7 @@ mod tests {
     #[test]
     fn saved_workspaces_are_in_the_native_workspace_menu() {
         let mut app = app();
+        app.session.config = Some(std::sync::Arc::new(effectcraft_engine::config::MemoryConfig::default()));
         let ctx = egui::Context::default();
         crate::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap();
         let entry = |app: &EffectcraftApp| build(app).items().into_iter().find(|i| i.label == "My Layout").cloned();

crates/ui-egui/src/workspace_state.rs

@@ -263,6 +263,19 @@ impl WorkspaceStore {
         self.error = None;
         Ok(true)
     }
+
+    /// An explicit save (Save Workspace, Save as New Workspace, Edit Workspaces): unlike
+    /// [`Self::save`], no settings folder or a file blocked at load is an error, not "nothing to do".
+    pub(crate) fn save_now(&mut self, config: Option<&dyn ConfigStore>, ui: &UiState) -> Result<(), String> {
+        let Some(config) = config else {
+            return Err("there is no settings folder to save them in".into());
+        };
+        if !self.writable {
+            let path = config.dir().map_or_else(|| FILE.to_string(), |dir| dir.join(FILE).display().to_string());
+            return Err(format!("{path} couldn't be loaded at startup, so it is left untouched; fix or move it and restart"));
+        }
+        self.save(Some(config), ui).map(|_| ())
+    }
 }
 
 #[cfg(test)]
@@ -283,10 +296,31 @@ mod tests {
             ui.focused = PanelKind::Project;
             ui.dock = crate::dock::workspace("Minimal");
             assert!(!store.save(Some(&config), &ui).unwrap());
+            assert!(store.save_now(Some(&config), &ui).unwrap_err().contains("couldn't be loaded"));
             assert_eq!(config.read(FILE).as_deref(), Some(text.as_str()));
         }
     }
 
+    #[test]
+    fn explicit_save_without_a_settings_folder_is_an_error() {
+        let mut ui = UiState::default();
+        let mut store = WorkspaceStore::load(None, &mut ui);
+        ui.focused = PanelKind::Project;
+        assert!(!store.save(None, &ui).unwrap());
+        assert!(store.save_now(None, &ui).is_err());
+    }
+
+    #[test]
+    fn explicit_save_with_nothing_new_succeeds() {
+        let config = MemoryConfig::default();
+        let mut ui = UiState::default();
+        let mut store = WorkspaceStore::load(Some(&config), &mut ui);
+        ui.focused = PanelKind::Project;
+        store.save_now(Some(&config), &ui).unwrap();
+        store.save_now(Some(&config), &ui).unwrap();
+        assert!(config.read(FILE).is_some());
+    }
+
     #[test]
     fn invalid_tabs_split_and_deep_tree_are_rejected() {
         let mut snapshot = Snapshot::capture(&UiState::default());

crates/ui-egui/tests/ui_panels.rs

@@ -172,6 +172,8 @@ fn saved_workspaces_persist_across_sessions() {
 #[test]
 fn saved_workspace_is_listed_in_the_workspace_menu() {
     let mut h = harness();
+    // Saving needs a settings folder.
+    h.state_mut().session.config = Some(Arc::new(MemoryConfig::default()));
     let ctx = h.ctx.clone();
     effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "Minimal"})).unwrap();
     effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap();

crates/ui-egui/tests/ui_workspace_focus.rs

@@ -85,7 +85,11 @@ fn unreadable_native_workspace_is_preserved_after_later_layout_changes() {
         assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_string());
         app.set_workspace("Minimal");
         app.ui.focused = PanelKind::Project;
-        effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name":"Must not overwrite"})).unwrap();
+        let error = effectcraft_ui_egui::menus::invoke(&mut app, &ctx, "window.saveWorkspaceAs", json!({"name":"Must not overwrite"})).unwrap_err();
+        assert!(error.contains("won't be there after a restart") && error.contains("workspaces.json"), "{error}");
+        assert_eq!(app.ui.status, error, "the failure is in the status bar");
+        assert_eq!(app.ui.workspace, "Must not overwrite", "the layout stays usable for this session");
+        assert!(app.ui.saved_workspaces.contains_key("Must not overwrite"));
         eframe::App::on_exit(&mut app);
         assert_eq!(std::fs::read(&path).unwrap(), bytes);
         assert!(effectcraft_ui_egui::control::inspect(&app, &ctx)["workspaceStorageError"].is_string());