diff --git a/gpx-rs/src/api/commands.rs b/gpx-rs/src/api/commands.rs index 53a7e1e80..9e3886931 100644 --- a/gpx-rs/src/api/commands.rs +++ b/gpx-rs/src/api/commands.rs @@ -183,8 +183,8 @@ pub fn load_files( } /// Brings changes of the files from the outside in (see [`engine::SyncFiles`]): each file is -/// `(id, data, name)`, and replaces the file of that id, or is added if there is none. The files to -/// `remove` are deleted. The selection is left alone. +/// `(id, data, name)`, and replaces the file of that id, or is added if there is none; it is called +/// `name`, whatever its data says. The files to `remove` are deleted. The selection is left alone. pub fn sync_files( engine: &mut Engine, files: &[(Uuid, &[u8], &str)], @@ -232,6 +232,18 @@ pub fn reorder(engine: &mut Engine, file_ids_: &[Uuid], index: us ) } +/// Names a file in all its states, as if it had always been called so (not undoable): for a file +/// that is written to a file of another name, whose name it takes. +pub fn rename_file(engine: &mut Engine, file_id: Uuid, name: &str) -> Outcome { + execute( + engine, + Action::Rename { + file_id: engine::FileId(file_id), + name: name.to_owned(), + }, + ) +} + /// Puts the selected elements in the clipboard. Nothing happens when nothing is selected. pub fn copy(engine: &mut Engine) -> Outcome { execute(engine, Action::Copy) diff --git a/gpx-rs/src/engine/action.rs b/gpx-rs/src/engine/action.rs index 5e644fb95..75f036b26 100644 --- a/gpx-rs/src/engine/action.rs +++ b/gpx-rs/src/engine/action.rs @@ -36,4 +36,11 @@ pub enum Action<'a> { file_ids: Vec, index: usize, }, + /// Names a file in all the states of the history, as if it had always been called so (see + /// [`crate::History::rename`]). Not undoable: for a file that is given a name from the outside, + /// like the name of the file it is written to. + Rename { + file_id: FileId, + name: String, + }, } diff --git a/gpx-rs/src/engine/command/file/sync.rs b/gpx-rs/src/engine/command/file/sync.rs index 52015bdc3..ee297712e 100644 --- a/gpx-rs/src/engine/command/file/sync.rs +++ b/gpx-rs/src/engine/command/file/sync.rs @@ -7,7 +7,8 @@ use crate::{Apply, CommandError, FileId, Load, State}; pub struct ExternalFile<'a> { pub id: FileId, pub data: &'a [u8], - /// Name of the file when the data has none (see [`Load`]). + /// Name of the file outside (the name of the file on the disk, without its extension), which + /// is its name here whatever the data says: a file has a single name. pub name: &'a str, } @@ -15,7 +16,8 @@ pub struct ExternalFile<'a> { /// that are given replace the ones of the same id (which stay where they are in the order, and keep /// their id), or are added at the end if there are none; the ones to `remove` are deleted. /// -/// Unlike [`crate::LoadFiles`], the selection is left alone (what it holds of a file that was +/// Unlike [`crate::LoadFiles`], the files are named as they are outside (see [`ExternalFile`]), and +/// the selection is left alone (what it holds of a file that was /// replaced or deleted is dropped by the engine afterwards, as with any edit). /// /// The files that cannot be read are skipped, and the files that are here stay as they are. It is an @@ -38,6 +40,9 @@ impl Apply for SyncFiles<'_> { match load.parse(state.categories) { Ok(mut file) => { file.id = external.id; + if !external.name.trim().is_empty() { + file.info.name = external.name.to_owned(); + } if state.files.insert(external.id, Rc::new(file)).is_none() { state.order.0.push(external.id); } @@ -101,7 +106,8 @@ mod tests { .apply(&mut fx.state()) .unwrap(); assert_eq!(fx.order.0, [first, second]); - assert_eq!(fx.files[&first].info.name, "from disk"); + // named as it is outside, not as the data says + assert_eq!(fx.files[&first].info.name, "name"); assert_eq!(fx.files[&first].id, first); // the selection is not changed assert_eq!(fx.selected_files(), [second].into()); diff --git a/gpx-rs/src/engine/mod.rs b/gpx-rs/src/engine/mod.rs index 1c2f462ef..4f310310e 100644 --- a/gpx-rs/src/engine/mod.rs +++ b/gpx-rs/src/engine/mod.rs @@ -248,6 +248,7 @@ impl Engine { self.order.move_files(&file_ids, index); None } + Action::Rename { file_id, name } => self.history.rename(file_id, &name), }; let changed = diff.is_some() || self.selection != selection_before diff --git a/gpx-rs/src/engine/state/history/global.rs b/gpx-rs/src/engine/state/history/global.rs index c56cfdffc..5b7d1264b 100644 --- a/gpx-rs/src/engine/state/history/global.rs +++ b/gpx-rs/src/engine/state/history/global.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use super::{Diff, Files, History, diff_between}; +use super::{Diff, Files, History, Renamer, diff_between}; use crate::{CommandError, FileId}; /// One history for the whole workspace: each entry is a state of all the files. @@ -123,6 +123,22 @@ impl History for GlobalHistory { None => !self.entries.is_empty(), } } + + fn rename(&mut self, id: FileId, name: &str) -> Option { + let before = self.current()?.get(&id)?.clone(); + let mut renamer = Renamer::new(name); + for entry in self.entries.iter_mut() { + if let Some(file) = entry.get(&id) { + let renamed = renamer.rename(file); + entry.insert(id, renamed); + } + } + let after = self.current()?.get(&id)?; + (!Rc::ptr_eq(&before, after)).then(|| Diff { + modified: vec![id], + ..Default::default() + }) + } } #[cfg(test)] diff --git a/gpx-rs/src/engine/state/history/mod.rs b/gpx-rs/src/engine/state/history/mod.rs index d8895664d..9bb37fe11 100644 --- a/gpx-rs/src/engine/state/history/mod.rs +++ b/gpx-rs/src/engine/state/history/mod.rs @@ -57,6 +57,41 @@ pub trait History: Default + Debug { /// Whether [`History::redo`] would do something for these `files`. fn can_redo(&self, files: &[FileId]) -> bool; + + /// Names a file `name` in all its states, as if it had always been called so: there is nothing + /// to undo. Says that the file changed, `None` if there is no such file or it is called so. + fn rename(&mut self, id: FileId, name: &str) -> Option; +} + +/// Names the states of a file: the ones that were shared stay shared (a state that several +/// entries have is renamed once), and the ones that are called so already are kept. +pub(super) struct Renamer<'a> { + name: &'a str, + renamed: HashMap<*const File, Rc>, +} + +impl<'a> Renamer<'a> { + pub(super) fn new(name: &'a str) -> Self { + Self { + name, + renamed: HashMap::new(), + } + } + + pub(super) fn rename(&mut self, file: &Rc) -> Rc { + if file.info.name == self.name { + return file.clone(); + } + let name = self.name; + self.renamed + .entry(Rc::as_ptr(file)) + .or_insert_with(|| { + let mut renamed = (**file).clone(); + renamed.info.name = name.to_owned(); + Rc::new(renamed) + }) + .clone() + } } /// Which files were added, removed or modified by a change of the current files. diff --git a/gpx-rs/src/engine/state/history/per_file.rs b/gpx-rs/src/engine/state/history/per_file.rs index b1f6409e0..b3b43e963 100644 --- a/gpx-rs/src/engine/state/history/per_file.rs +++ b/gpx-rs/src/engine/state/history/per_file.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use super::{Diff, Files, History, diff_between}; +use super::{Diff, Files, History, Renamer, diff_between}; use crate::{CommandError, File, FileId}; use std::collections::HashMap; @@ -139,6 +139,20 @@ impl History for PerFileHistory { self.any(files, FileHistory::can_undo) } + fn rename(&mut self, id: FileId, name: &str) -> Option { + let history = self.stacks.get_mut(&id)?; + let mut renamer = Renamer::new(name); + for entry in history.entries.iter_mut() { + *entry = renamer.rename(entry); + } + let current = history.current().clone(); + let before = self.current.insert(id, current.clone()); + (!before.is_some_and(|before| Rc::ptr_eq(&before, ¤t))).then(|| Diff { + modified: vec![id], + ..Default::default() + }) + } + fn can_redo(&self, files: &[FileId]) -> bool { self.any(files, FileHistory::can_redo) } diff --git a/gpx-rs/src/engine/tests.rs b/gpx-rs/src/engine/tests.rs index dc3015851..9100a4619 100644 --- a/gpx-rs/src/engine/tests.rs +++ b/gpx-rs/src/engine/tests.rs @@ -182,6 +182,52 @@ fn test_metadata_renames_selected_file() { assert_eq!(file.info.desc.as_deref(), Some("about")); } +#[test] +fn test_rename_names_every_state_of_the_file_and_is_not_undoable() { + let mut engine = TestEngine::default(); + new(&mut engine, "first"); + new(&mut engine, "other"); + let id = engine.order()[0]; + let other = engine.order()[1]; + let other_file = engine.history.current().unwrap()[&other].clone(); + engine.run(Action::Select { + selection: Selection::File { + file_ids: [id].into(), + }, + mode: SelectMode::Replace, + }); + assert!(edit( + &mut engine, + Command::Metadata(Metadata { + name: "edited", + desc: "", + }) + )); + assert!(engine.run(Action::Rename { + file_id: id, + name: "written".to_owned(), + })); + assert_eq!(engine.last_diff().unwrap().modified, vec![id]); + assert_eq!(engine.file_structure(&id).unwrap().name, "written"); + // nothing to undo for it: the edits before are undone, and the file keeps its name + engine.run(Action::Undo); + engine.run(Action::Undo); + assert_eq!(engine.file_structure(&id).unwrap().name, "written"); + engine.run(Action::Redo); + engine.run(Action::Redo); + assert_eq!(engine.file_structure(&id).unwrap().name, "written"); + // the other file is still the same object, in every state + assert!(Rc::ptr_eq( + &engine.history.current().unwrap()[&other], + &other_file + )); + // naming it as it is already called changes nothing + assert!(!engine.run(Action::Rename { + file_id: id, + name: "written".to_owned(), + })); +} + #[test] fn test_metadata_without_selection_does_not_modify_files() { let mut engine = TestEngine::default(); @@ -1469,6 +1515,31 @@ mod per_file_history { engine.file_structure(&id).unwrap().name.clone() } + #[test] + fn test_rename_names_every_state_of_the_file() { + let mut engine = Engine::::default(); + let id = new_file(&mut engine, "a"); + select(&mut engine, &[id]); + rename(&mut engine, "b"); + rename(&mut engine, "c"); + let outcome = engine + .execute(Action::Rename { + file_id: id, + name: "written".to_owned(), + }) + .unwrap(); + assert_eq!(outcome.diff.unwrap().modified, vec![id]); + assert_eq!(name_of(&engine, id), "written"); + // the edits are still there to undo, the name stays + engine.execute(Action::Undo).unwrap(); + assert!(engine.can_undo()); + engine.execute(Action::Undo).unwrap(); + assert!(!engine.can_undo()); + assert_eq!(name_of(&engine, id), "written"); + engine.execute(Action::Redo).unwrap(); + assert_eq!(name_of(&engine, id), "written"); + } + #[test] fn test_creating_a_file_is_not_undoable() { let mut engine = Engine::::default(); diff --git a/tauri/src/commands/disk.rs b/tauri/src/commands/disk.rs index 80055373f..16c41533e 100644 --- a/tauri/src/commands/disk.rs +++ b/tauri/src/commands/disk.rs @@ -113,10 +113,8 @@ fn write_unsaved( state.core(move |core| { let result = core .workspace - .save(&core.engine, &ids, &chosen, folder.as_deref()); - core.keep_workspace(); - core.announce(None); - result.map(|()| true) + .save(&mut core.engine, &ids, &chosen, folder.as_deref()); + core.wrote(result) }) } @@ -180,10 +178,8 @@ pub fn save_file_as( return Ok(false); }; state.core(move |core| { - let result = core.workspace.save_as(&core.engine, file_id, &path); - core.keep_workspace(); - core.announce(None); - result.map(|()| true) + let result = core.workspace.save_as(&mut core.engine, file_id, &path); + core.wrote(result) }) } diff --git a/tauri/src/commands/edit.rs b/tauri/src/commands/edit.rs index d15cc13ab..8f2a31fc2 100644 --- a/tauri/src/commands/edit.rs +++ b/tauri/src/commands/edit.rs @@ -1,15 +1,28 @@ //! Editing the metadata, the style, the structure and the waypoints of the files, and the tools: //! reversing, cropping, reducing, timestamps, merging, cleaning... -use gpx_engine::{CleanType, MergeType, SplitType, api}; +use gpx_engine::{CleanType, FileId, MergeType, Selection, SplitType, api}; use tauri::State; use uuid::Uuid; use crate::state::AppState; +/// Names and describes the selected files or tracks. A file on the disk is renamed with it (and back +/// when it is undone): its name is one that a file can have, and not the one of another file. #[tauri::command(async)] pub fn metadata(state: State, name: String, desc: String) -> api::Outcome { - state.run(move |e| api::metadata(e, &name, &desc)) + state.core(move |core| { + let mut name = name; + if let Selection::File { file_ids } = core.engine.selection() { + for FileId(id) in file_ids.clone() { + name = core.workspace.valid_name(id, &name); + if let Err(error) = core.workspace.renamed_path(id, &name) { + return api::Outcome::failed(error); + } + } + } + core.edit(move |e| api::metadata(e, &name, &desc)) + }) } #[tauri::command(async)] diff --git a/tauri/src/commands/files.rs b/tauri/src/commands/files.rs index d7f210942..2634fb4ca 100644 --- a/tauri/src/commands/files.rs +++ b/tauri/src/commands/files.rs @@ -109,10 +109,10 @@ pub fn move_elements( #[tauri::command(async)] pub fn undo(state: State) -> api::Outcome { - state.run(api::undo) + state.edit(api::undo) } #[tauri::command(async)] pub fn redo(state: State) -> api::Outcome { - state.run(api::redo) + state.edit(api::redo) } diff --git a/tauri/src/state.rs b/tauri/src/state.rs index 1b8ca6c3a..e021b0210 100644 --- a/tauri/src/state.rs +++ b/tauri/src/state.rs @@ -96,6 +96,48 @@ impl Core { self.announce(None); } + /// Runs `f` on the engine, which can change: what changed is kept, and the files on the disk are + /// renamed like the files of the engine were (an edit of the name, or its undo). Gives what + /// could not be renamed. + fn editing(&mut self, f: impl FnOnce(&mut Engine) -> T) -> (T, Vec) { + let names = self.workspace.names(&self.engine); + let result = f(&mut self.engine); + let (renamed, errors) = self.workspace.follow_names(&mut self.engine, &names); + if renamed { + self.keep_workspace(); + self.announce(None); + } + self.persist(); + (result, errors) + } + + /// Like `editing`, for an edit that can rename files: the outcome tells the first thing that + /// could not be renamed (a key of the dictionaries of the frontend, which shows it). + pub fn edit(&mut self, f: impl FnOnce(&mut Engine) -> api::Outcome) -> api::Outcome { + let (outcome, errors) = self.editing(f); + match errors.first() { + Some(error) if !outcome.has_error() => { + outcome.merge(api::Outcome::failed(error.clone())) + } + _ => outcome, + } + } + + /// After files were written (`result`): what changed in the engine (the files are called like + /// the files they were written to) is kept and told, with where the files are. Says whether + /// they were written. + pub fn wrote(&mut self, result: Result) -> Result { + self.keep_workspace(); + match &result { + Ok(outcome) if outcome.changed => { + self.persist(); + self.announce(Some(outcome)); + } + _ => self.announce(None), + } + result.map(|_| true) + } + /// Writes the workspace, and watches what it holds (a file may have been opened or closed). pub fn keep_workspace(&mut self) { self.workspace.write(self.storage.root()); @@ -228,10 +270,12 @@ impl Core { Some((id, source.map(|(_, display)| display.clone()))) }) .collect(); - if let Err(error) = self.workspace.save_new(&self.engine, &made) - && !outcome.has_error() - { - outcome = outcome.merge(api::Outcome::failed(error)); + match self.workspace.save_new(&mut self.engine, &made) { + Ok(renamed) => outcome = outcome.merge(renamed), + Err(error) if !outcome.has_error() => { + outcome = outcome.merge(api::Outcome::failed(error)); + } + Err(_) => {} } } self.keep_workspace(); @@ -291,15 +335,25 @@ impl AppState { result.recv().expect("the engine thread stopped") } - /// Runs `f` on the engine, which can change: what changed is kept. + /// Runs `f` on the engine, which can change: what changed is kept (see `Core::editing`). pub fn run(&self, f: impl FnOnce(&mut Engine) -> T + Send + 'static) -> T { self.core(move |core| { - let result = f(&mut core.engine); - core.persist(); + let (result, errors) = core.editing(f); + for error in errors { + eprintln!("{error}"); + } result }) } + /// Like `run`, for an edit that can rename files: the outcome tells what could not be renamed. + pub fn edit( + &self, + f: impl FnOnce(&mut Engine) -> api::Outcome + Send + 'static, + ) -> api::Outcome { + self.core(move |core| core.edit(f)) + } + /// Reads from the engine. pub fn read(&self, f: impl FnOnce(&Engine) -> T + Send + 'static) -> T { self.core(move |core| f(&core.engine)) diff --git a/tauri/src/tests.rs b/tauri/src/tests.rs index 720c442a7..6958b8250 100644 --- a/tauri/src/tests.rs +++ b/tauri/src/tests.rs @@ -396,7 +396,7 @@ fn files_are_opened_closed_and_made_in_a_folder() { // closing: what is saved goes, what has edits stays unless it is discarded app.call("select", json!({ "fileIds": [new_id], "mode": "replace" })); - app.call("metadata", json!({ "name": "edited", "desc": "" })); + app.call("style", json!({ "color": "#ff0000" })); app.call("close_files", json!({ "fileIds": [id, new_id] })); assert_eq!(app.call("file_order", json!({})), json!([new_id])); app.call( @@ -550,3 +550,123 @@ fn files_can_be_reordered() { json!([ids[2], ids[0], ids[1]]) ); } + +/// A folder of its own, with GPX files whose name inside is not the name of the file. +fn folder_with(files: &[&str]) -> std::path::PathBuf { + let folder = std::env::temp_dir().join(format!("gpx-names-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&folder).unwrap(); + for name in files { + std::fs::write( + folder.join(format!("{name}.gpx")), + r#"Inside"#, + ) + .unwrap(); + } + folder +} + +fn name_of(app: &App, id: &str) -> String { + app.call("file_structure", json!({ "fileId": id }))["name"] + .as_str() + .unwrap() + .to_string() +} + +#[test] +fn a_file_is_called_like_its_file_and_renaming_it_renames_the_file() { + let app = App::new(); + let folder = folder_with(&["outside", "taken"]); + let outcome = app.call( + "open_paths", + json!({ "paths": [folder.join("outside.gpx")] }), + ); + let id = outcome["added"][0].as_str().unwrap().to_string(); + + // the name in the file is not the name of the file: the file wins, and nothing is unsaved + assert_eq!(name_of(&app, &id), "outside"); + assert_eq!( + app.call("workspace_state", json!({}))["files"][&id]["dirty"], + false + ); + + // renaming renames the file on the disk + app.call("select", json!({ "fileIds": [id], "mode": "replace" })); + let outcome = app.call("metadata", json!({ "name": "renamed", "desc": "" })); + assert!(outcome.get("error").is_none()); + assert!(folder.join("renamed.gpx").exists()); + assert!(!folder.join("outside.gpx").exists()); + let path = app.call("workspace_state", json!({}))["files"][&id]["path"].clone(); + assert!(path.as_str().unwrap().ends_with("renamed.gpx")); + // the file had nothing unsaved: it is written with its new name, and still has nothing unsaved + let dirty = |app: &App| app.call("workspace_state", json!({}))["files"][&id]["dirty"].clone(); + let inside = |name: &str| std::fs::read_to_string(folder.join(format!("{name}.gpx"))).unwrap(); + assert_eq!(dirty(&app), false); + assert!(inside("renamed").contains("renamed")); + + // undone and redone, on the disk too, still with nothing unsaved + app.call("undo", json!({})); + assert_eq!(name_of(&app, &id), "outside"); + assert!(folder.join("outside.gpx").exists() && !folder.join("renamed.gpx").exists()); + assert_eq!(dirty(&app), false); + assert!(inside("outside").contains("outside")); + app.call("redo", json!({})); + assert_eq!(name_of(&app, &id), "renamed"); + assert!(folder.join("renamed.gpx").exists() && !folder.join("outside.gpx").exists()); + assert_eq!(dirty(&app), false); + + // the name of another file is not taken, nothing changes + let outcome = app.call("metadata", json!({ "name": "taken", "desc": "" })); + assert_eq!(outcome["error"], crate::workspace::NAME_TAKEN); + assert_eq!(name_of(&app, &id), "renamed"); + assert!(folder.join("renamed.gpx").exists()); + + // a name is one that a file can have + app.call("metadata", json!({ "name": "a/b", "desc": "" })); + assert_eq!(name_of(&app, &id), "a_b"); + assert!(folder.join("a_b.gpx").exists()); + + // a file with edits is renamed, but not written: its edits are not saved for it + app.call("style", json!({ "color": "#ff0000" })); + assert_eq!(dirty(&app), true); + app.call("metadata", json!({ "name": "edited", "desc": "" })); + assert!(folder.join("edited.gpx").exists()); + assert_eq!(dirty(&app), true); + assert!(!inside("edited").contains("ff0000")); + app.call("undo", json!({})); + app.call("undo", json!({})); + assert_eq!(name_of(&app, &id), "a_b"); + assert_eq!(dirty(&app), false); + + // only the case changes: the same file + app.call("metadata", json!({ "name": "A_B", "desc": "" })); + assert_eq!(name_of(&app, &id), "A_B"); + let names: Vec = std::fs::read_dir(&folder) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + assert!(names.contains(&"A_B.gpx".to_string())); + assert_eq!(names.len(), 2); + std::fs::remove_dir_all(folder).unwrap(); +} + +#[test] +fn a_new_file_written_under_another_name_takes_it() { + let app = App::new(); + let folder = folder_with(&["route"]); + // a new file made in the folder is written there at once: route.gpx is taken + let created = app.call( + "new_file", + json!({ "name": "route", "lng": 4.4, "lat": 50.8, "folder": folder }), + ); + let id = created["added"][0].as_str().unwrap().to_string(); + assert!(folder.join("route (2).gpx").exists()); + assert_eq!(name_of(&app, &id), "route (2)"); + assert_eq!( + app.call("workspace_state", json!({}))["files"][&id]["dirty"], + false + ); + // which is not an edit to undo + app.call("select", json!({ "fileIds": [id], "mode": "replace" })); + assert_eq!(app.call("can_undo", json!({})), false); + std::fs::remove_dir_all(folder).unwrap(); +} diff --git a/tauri/src/workspace.rs b/tauri/src/workspace.rs index 7fd6c243e..11f259a39 100644 --- a/tauri/src/workspace.rs +++ b/tauri/src/workspace.rs @@ -58,6 +58,12 @@ pub fn signature(engine: &Engine, id: Uuid) -> Option { Some(hash(&serde_json::to_vec(&structure).ok()?)) } +/// A file cannot be renamed: another one has the name. The errors that the frontend shows are keys +/// of its dictionaries (`website/src/locales`). +pub const NAME_TAKEN: &str = "menu.metadata.name_taken"; +/// A file cannot be renamed on the disk. +pub const RENAME_FAILED: &str = "menu.metadata.rename_failed"; + /// What to do with the files that have unsaved edits when they are closed. #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] @@ -211,6 +217,23 @@ fn file_name(name: &str) -> String { } } +/// Whether two paths are the same file (two cases of a name, on a disk that does not tell them +/// apart). +fn same_file(a: &Path, b: &Path) -> bool { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + matches!( + (fs::metadata(a), fs::metadata(b)), + (Ok(a), Ok(b)) if (a.dev(), a.ino()) == (b.dev(), b.ino()) + ) + } + #[cfg(not(unix))] + { + matches!((fs::canonicalize(a), fs::canonicalize(b)), (Ok(a), Ok(b)) if a == b) + } +} + fn stem(path: &Path) -> String { path.file_stem() .map(|stem| stem.to_string_lossy().into_owned()) @@ -617,15 +640,16 @@ impl Workspace { /// Writes files to the disk, replacing what is there: they are the same as their file from /// then on. A file that is not on the disk goes in its folder, or at its path in `paths`, or in - /// `folder`, under its name (changed if that is the one of something else). Stops at the first - /// error, the files that were written stay written. + /// `folder`, under its name (changed if that is the one of something else), which the file then + /// has (see `write_file`). Stops at the first error, the files that were written stay written. pub fn save( &mut self, - engine: &Engine, + engine: &mut Engine, ids: &[Uuid], paths: &BTreeMap, folder: Option<&Path>, - ) -> Result<(), String> { + ) -> Result { + let mut outcome = api::Outcome::failed_none(); for id in ids { let Some(structure) = engine.file_structure(&FileId(*id)) else { continue; @@ -638,20 +662,20 @@ impl Workspace { (None, None, None, Some(dir)) => in_folder(dir), (None, None, None, None) => continue, }; - self.write_file(engine, *id, &path)?; + outcome = outcome.merge(self.write_file(engine, *id, &path)?); } - Ok(()) + Ok(outcome) } /// Writes the new files among `files` in their folder, each under the name that is given (the /// name in the file when there is none). A name that is the one of something else is changed /// (`name (2)`). Files that would be called the same, like the parts of a split, are numbered - /// from 1 (`name (1)`, `name (2)`). + /// from 1 (`name (1)`, `name (2)`). The files are called like the file they are written to. pub fn save_new( &mut self, - engine: &Engine, + engine: &mut Engine, files: &[(Uuid, Option)], - ) -> Result<(), String> { + ) -> Result { let names: Vec> = files .iter() .map(|(id, name)| { @@ -664,6 +688,7 @@ impl Workspace { )) }) .collect(); + let mut outcome = api::Outcome::failed_none(); for entry in &names { let Some((id, dir, name)) = entry else { continue; @@ -681,9 +706,106 @@ impl Workspace { } else { free_path(dir, name, |p| self.is_taken(p)) }; - self.write_file(engine, *id, &path)?; + outcome = outcome.merge(self.write_file(engine, *id, &path)?); } - Ok(()) + Ok(outcome) + } + + /// The names of the files that are on the disk, and whether they are the files on the disk, to + /// tell which ones were renamed by an edit (see `follow_names`). + pub fn names(&self, engine: &Engine) -> BTreeMap { + self.links + .keys() + .filter_map(|id| { + let name = engine.file_structure(&FileId(*id))?.name.clone(); + Some((*id, (name, !self.is_dirty(engine, *id)))) + }) + .collect() + } + + /// The name that a file on the disk can have: the name of a file, without its extension (see + /// `file_name`). Others are given to the files that are not on the disk yet when they are + /// written. + pub fn valid_name(&self, id: Uuid, name: &str) -> String { + if self.links.contains_key(&id) { + file_name(name) + } else { + name.to_owned() + } + } + + /// Where a file on the disk goes when it is called `name`: next to where it is, with the same + /// extension. `None` if it is there already, an error if something else is. + pub fn renamed_path(&self, id: Uuid, name: &str) -> Result, String> { + let Some(link) = self.links.get(&id) else { + return Ok(None); + }; + let extension = link + .path + .extension() + .map(|extension| extension.to_string_lossy().into_owned()) + .unwrap_or_else(|| "gpx".to_owned()); + let path = link + .path + .with_file_name(format!("{}.{extension}", file_name(name))); + if path == link.path { + return Ok(None); + } + // the same file under another case, on a disk that does not tell them apart, is not another + let other = self + .links + .iter() + .any(|(other, l)| *other != id && l.path == path) + || (path.exists() && !same_file(&path, &link.path)); + if other { + return Err(NAME_TAKEN.to_owned()); + } + Ok(Some(path)) + } + + /// Renames the files on the disk whose name changed in the engine since it had `before` (see + /// `names`): an edit of the name, or its undo, renames the file. A file that was the one on the + /// disk is written too, so that it still is: the rename is not something to save. A file with + /// edits is not, its edits are not saved for it. A file that cannot be renamed keeps its place, + /// with the reason in the errors. Says whether a file was renamed. + pub fn follow_names( + &mut self, + engine: &mut Engine, + before: &BTreeMap, + ) -> (bool, Vec) { + let mut renamed = false; + let mut errors = vec![]; + for (id, (old, clean)) in before { + let Some(structure) = engine.file_structure(&FileId(*id)) else { + continue; + }; + if structure.name == *old { + continue; + } + let path = match self.renamed_path(*id, &structure.name) { + Ok(Some(path)) => path, + Ok(None) => continue, + Err(error) => { + errors.push(error); + continue; + } + }; + let link = self.links.get_mut(id).expect("the file is on the disk"); + // a file that is gone is written at its new place when it is saved + if !link.missing + && let Err(error) = fs::rename(&link.path, &path) + { + eprintln!("{} cannot be renamed: {error}", link.path.display()); + errors.push(RENAME_FAILED.to_owned()); + continue; + } + link.path = path.clone(); + renamed = true; + if *clean && let Err(error) = self.write_file(engine, *id, &path) { + errors.push(error); + } + } + (renamed, errors) } /// The name of a file as the user knows it: the name of the file on the disk, without its @@ -696,7 +818,12 @@ impl Workspace { } /// Writes a file to a path of the user's choice, which is the place of the file from then on. - pub fn save_as(&mut self, engine: &Engine, id: Uuid, path: &Path) -> Result<(), String> { + pub fn save_as( + &mut self, + engine: &mut Engine, + id: Uuid, + path: &Path, + ) -> Result { if let Some(other) = self.id_of(&canonical(path)) && other != id { @@ -705,13 +832,30 @@ impl Workspace { self.write_file(engine, id, path) } - fn write_file(&mut self, engine: &Engine, id: Uuid, path: &Path) -> Result<(), String> { + /// Writes a file to `path`, which it is linked to from then on. It is called like the file it is + /// written to (a file has a single name), as if it always had been: that is not an edit to + /// undo. Says what changed in the engine. + fn write_file( + &mut self, + engine: &mut Engine, + id: Uuid, + path: &Path, + ) -> Result { + let Some(structure) = engine.file_structure(&FileId(id)) else { + return Ok(api::Outcome::failed_none()); + }; + let name = stem(path); + let renamed = if structure.name != name { + api::rename_file(engine, id, &name) + } else { + api::Outcome::failed_none() + }; let Some(bytes) = api::export_file(engine, id, ExportOptions::ALL) else { - return Ok(()); + return Ok(renamed); }; write_atomically(path, &bytes).map_err(|error| format!("{}: {error}", path.display()))?; let Some(signature) = signature(engine, id) else { - return Ok(()); + return Ok(renamed); }; // the folder, if there is one, may watch what was just written: the hash tells it is known self.links.insert( @@ -726,7 +870,7 @@ impl Workspace { }, ); self.dirs.remove(&id); - Ok(()) + Ok(renamed) } /// The folders and the GPX files that are in a folder of the open folder (or in it), the @@ -776,7 +920,7 @@ mod tests { fn gpx(name: &str) -> String { format!( - r#"{name}1020"# + r#"{name}{name}1020"# ) } @@ -833,7 +977,7 @@ mod tests { fn save(&mut self, ids: &[Uuid]) { self.workspace - .save(&self.engine, ids, &BTreeMap::new(), None) + .save(&mut self.engine, ids, &BTreeMap::new(), None) .unwrap(); } @@ -856,6 +1000,14 @@ mod tests { .clone() } + /// What the file holds: the name of its track (its own name is the one of the file). + fn content(&self, id: Uuid) -> String { + self.engine.file_structure(&FileId(id)).unwrap().tracks[0] + .name + .clone() + .unwrap_or_default() + } + fn rename(&mut self, id: Uuid, name: &str) { api::select(&mut self.engine, &[id], SelectMode::Replace); assert!(!api::metadata(&mut self.engine, name, "").has_error()); @@ -919,7 +1071,9 @@ mod tests { let path = fx.folder.join("fresh.gpx"); let paths = BTreeMap::from([(id, path.clone())]); - fx.workspace.save(&fx.engine, &[id], &paths, None).unwrap(); + fx.workspace + .save(&mut fx.engine, &[id], &paths, None) + .unwrap(); assert!(path.exists()); assert!(!fx.workspace.is_dirty(&fx.engine, id)); assert!(fx.workspace.unlinked(&fx.engine, &[id]).is_empty()); @@ -935,7 +1089,7 @@ mod tests { let ids = fx.ids(); let folder = fx.folder.clone(); fx.workspace - .save(&fx.engine, &ids, &BTreeMap::new(), Some(&folder)) + .save(&mut fx.engine, &ids, &BTreeMap::new(), Some(&folder)) .unwrap(); for name in ["fresh (2).gpx", "fresh (3).gpx", "a_b_ c.gpx"] { assert!(fx.folder.join(name).exists(), "{name}"); @@ -949,11 +1103,11 @@ mod tests { let path = fx.write("a.gpx", "a"); fx.open_folder(); let id = fx.ids()[0]; - fx.rename(id, "renamed"); + api::select(&mut fx.engine, &[id], SelectMode::Replace); + assert!(!api::style(&mut fx.engine, Some("#ff0000"), None, None).has_error()); fx.save(&[id]); assert!(fx.reconcile(Some(&[path])).is_none()); - assert!(!fx.status(id).replaced); - assert_eq!(fx.name(id), "renamed"); + assert!(!fx.status(id).replaced && !fx.status(id).dirty); } #[test] @@ -1076,7 +1230,7 @@ mod tests { assert!(outcome.changed); // the same file, now the one on the disk assert_eq!(fx.ids(), [id]); - assert_eq!(fx.name(id), "theirs"); + assert_eq!(fx.content(id), "theirs"); let status = fx.status(id); assert!(!status.dirty && !status.replaced); // looking again changes nothing @@ -1119,13 +1273,13 @@ mod tests { assert!(fx.status(id).replaced); fx.workspace.reload(&mut fx.engine, &[id]).unwrap(); - assert_eq!(fx.name(id), "theirs"); + assert_eq!(fx.content(id), "theirs"); let status = fx.status(id); assert!(!status.dirty && !status.replaced); // and it is one undo step api::select(&mut fx.engine, &[id], SelectMode::Replace); api::undo(&mut fx.engine); - assert_eq!(fx.name(id), "mine"); + assert_eq!(fx.content(id), "mine"); } #[test] @@ -1176,7 +1330,7 @@ mod tests { fx.write("a.gpx", "back"); fx.reconcile(None); - assert_eq!(fx.name(id), "back"); + assert_eq!(fx.content(id), "back"); let status = fx.status(id); assert!(!status.dirty && !status.missing); } @@ -1189,7 +1343,7 @@ mod tests { let id = fx.ids()[0]; fx.write("a.gpx", "changed"); assert!(fx.reconcile(None).is_some()); - assert_eq!(fx.name(id), "changed"); + assert_eq!(fx.content(id), "changed"); fs::remove_file(a).unwrap(); assert!(fx.reconcile(None).is_some()); assert!(fx.status(id).missing); @@ -1360,7 +1514,9 @@ mod tests { // the place of the file is taken by a folder fs::remove_file(fx.folder.join("a.gpx")).unwrap(); fs::create_dir(fx.folder.join("a.gpx")).unwrap(); - let result = fx.workspace.save(&fx.engine, &[id], &BTreeMap::new(), None); + let result = fx + .workspace + .save(&mut fx.engine, &[id], &BTreeMap::new(), None); assert!(result.is_err()); assert!(fx.workspace.is_dirty(&fx.engine, id)); } @@ -1376,7 +1532,7 @@ mod tests { fx.rename(id, "edited"); let target = fx.folder.join("copy.gpx"); - fx.workspace.save_as(&fx.engine, id, &target).unwrap(); + fx.workspace.save_as(&mut fx.engine, id, &target).unwrap(); assert!(fs::read_to_string(&target).unwrap().contains("edited")); assert!(fs::read_to_string(a).unwrap().contains(">a<")); let status = fx.status(id); @@ -1385,7 +1541,7 @@ mod tests { // the place of another open file is not available let taken = fx.folder.join("b.gpx"); - assert!(fx.workspace.save_as(&fx.engine, id, &taken).is_err()); + assert!(fx.workspace.save_as(&mut fx.engine, id, &taken).is_err()); assert!(!fx.workspace.is_dirty(&fx.engine, other)); } } diff --git a/website/src/lib/components/file-list/metadata/MetadataDialog.svelte b/website/src/lib/components/file-list/metadata/MetadataDialog.svelte index df4bc0ead..9f40b5c68 100644 --- a/website/src/lib/components/file-list/metadata/MetadataDialog.svelte +++ b/website/src/lib/components/file-list/metadata/MetadataDialog.svelte @@ -9,6 +9,8 @@ import { editMetadata } from '$lib/components/file-list/metadata/utils.svelte'; import { engine, type FileState } from '$lib/engine'; import type { FileTreeNode } from '$lib/engine/selection-helpers'; + import { toast } from 'svelte-sonner'; + import { get } from 'svelte/store'; let { fileState, @@ -53,8 +55,15 @@ // would open the dialog again before the effect below has reset it editMetadata.current = false; open = false; - // the metadata of the selected elements are changed - engine.metadata(name, description); + // the metadata of the selected elements are changed. In the desktop app, a file on + // the disk is renamed with it, which is refused if the name is the one of another + engine.metadata(name, description).then((changed) => { + const error = get(engine.lastError); + if (!changed && error !== undefined && error !== 'nothing to do') { + // a key of the dictionaries, when it comes from the desktop app + toast.error(i18n._(error, error)); + } + }); }} > diff --git a/website/src/locales/en.json b/website/src/locales/en.json index 9e88fe6a5..d7dce2a87 100644 --- a/website/src/locales/en.json +++ b/website/src/locales/en.json @@ -87,7 +87,9 @@ "button": "Info...", "name": "Name", "description": "Description", - "save": "Save" + "save": "Save", + "name_taken": "A file with this name already exists in the folder.", + "rename_failed": "The file could not be renamed." }, "style": { "button": "Appearance...",