clarify name handling

tauri/src/commands/disk.rs

@@ -14,7 +14,6 @@ use tauri::{AppHandle, Runtime, State};
 use tauri_plugin_dialog::{DialogExt, FilePath};
 use uuid::Uuid;
 
-use crate::commands::edit;
 use crate::state::AppState;
 use crate::workspace::{Entry, Unsaved, WorkspaceState, free_path};
 
@@ -386,28 +385,28 @@ pub fn create_folder(
     state.core(move |core| core.workspace.create_folder(&parent, &name))
 }
 
-/// Renames a file or a folder of the open folder. An open file is renamed like in its info (it is
-/// selected), which can be undone; the rest is renamed on the disk, and the open files that are in
-/// a folder that is renamed follow.
+/// Renames a file or a folder of the open folder, on the disk only: an open file keeps its name,
+/// and follows. Not undoable.
 #[tauri::command(async)]
-pub fn rename_path(
-    state: State<AppState>,
-    path: PathBuf,
-    name: String,
-) -> Result<api::Outcome, String> {
+pub fn rename_path(state: State<AppState>, path: PathBuf, name: String) -> Result<(), String> {
     state.core(move |core| {
-        if let Some(id) = core.workspace.linked(&path) {
-            let selected = api::select(&mut core.engine, &[id], gpx_engine::SelectMode::Replace);
-            let desc = core
-                .engine
-                .file_structure(&FileId(id))
-                .and_then(|structure| structure.desc.clone())
-                .unwrap_or_default();
-            return Ok(selected.merge(edit::rename_selection(core, name, desc)));
-        }
         core.workspace.rename_path(&path, &name)?;
         core.commit(None);
-        Ok(api::Outcome::failed_none())
+        Ok(())
+    })
+}
+
+/// Renames the file on the disk of an open file, next to where it is (see `Workspace::rename_file`).
+/// Not undoable, the name of the file in the engine is not changed. Says whether it was renamed:
+/// not for a file that is not on the disk.
+#[tauri::command(async)]
+pub fn rename_file(state: State<AppState>, file_id: Uuid, name: String) -> Result<bool, String> {
+    state.core(move |core| {
+        let renamed = core.workspace.rename_file(file_id, &name)?;
+        if renamed {
+            core.commit(None);
+        }
+        Ok(renamed)
     })
 }
 

tauri/src/commands/edit.rs

@@ -1,31 +1,17 @@
 //! 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, FileId, MergeType, Selection, SplitType, api};
+use gpx_engine::{CleanType, MergeType, SplitType, api};
 use tauri::State;
 use uuid::Uuid;
 
-use crate::state::{AppState, Core};
+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.
+/// Names and describes the selected files or tracks. The files on the disk keep their name: see
+/// `disk::rename_file`.
 #[tauri::command(async)]
 pub fn metadata(state: State<AppState>, name: String, desc: String) -> api::Outcome {
-    state.core(move |core| rename_selection(core, name, desc))
-}
-
-/// See `metadata`.
-pub fn rename_selection(core: &mut Core, name: String, desc: String) -> api::Outcome {
-    let mut name = name;
-    if let Selection::File { file_ids } = core.engine.selection() {
-        for FileId(id) in file_ids.clone() {
-            match core.workspace.name_for(id, &name) {
-                Ok(valid) => name = valid,
-                Err(error) => return api::Outcome::failed(error),
-            }
-        }
-    }
-    core.edit(move |e| api::metadata(e, &name, &desc))
+    state.run(move |e| api::metadata(e, &name, &desc))
 }
 
 #[tauri::command(async)]

tauri/src/commands/files.rs

@@ -115,10 +115,10 @@ pub fn move_elements(
 
 #[tauri::command(async)]
 pub fn undo(state: State<AppState>) -> api::Outcome {
-    state.edit(api::undo)
+    state.run(api::undo)
 }
 
 #[tauri::command(async)]
 pub fn redo(state: State<AppState>) -> api::Outcome {
-    state.edit(api::redo)
+    state.run(api::redo)
 }

tauri/src/lib.rs

@@ -226,6 +226,7 @@ fn register_commands<R: tauri::Runtime>(builder: tauri::Builder<R>) -> tauri::Bu
         disk::list_folder,
         disk::create_folder,
         disk::rename_path,
+        disk::rename_file,
         disk::move_paths,
         disk::delete_paths,
         // the system

tauri/src/state.rs

@@ -94,11 +94,8 @@ impl Core {
         self.commit(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<T>(&mut self, f: impl FnOnce(&mut Engine) -> T) -> (T, Vec<String>) {
-        let names = self.workspace.names(&self.engine);
+    /// Runs `f` on the engine, which can change: what changed is kept.
+    fn editing<T>(&mut self, f: impl FnOnce(&mut Engine) -> T) -> T {
         let preview = self.workspace.preview;
         let preview_before = preview.and_then(|id| signature(&self.engine, id));
         let result = f(&mut self.engine);
@@ -107,32 +104,18 @@ impl Core {
         if let Some(id) = preview.filter(|_| kept) {
             self.workspace.keep(&[id]);
         }
-        let (renamed, errors) = self.workspace.follow_names(&mut self.engine, &names);
         // and it goes when something else is selected
         let dropped = self.workspace.drop_preview(&mut self.engine);
-        if kept || renamed || dropped.is_some() {
+        if kept || dropped.is_some() {
             self.commit(dropped.as_ref());
         } else {
             self.persist();
         }
-        (result, errors)
+        result
     }
 
-    /// 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.
+    /// After files were written (`result`): what changed in the engine is kept and told, with where
+    /// the files are. Says whether they were written.
     pub fn wrote(&mut self, result: Result<api::Outcome, String>) -> Result<bool, String> {
         self.commit(result.as_ref().ok().filter(|outcome| outcome.changed));
         result.map(|_| true)
@@ -325,21 +308,7 @@ impl AppState {
 
     /// Runs `f` on the engine, which can change: what changed is kept (see `Core::editing`).
     pub fn run<T: Send + 'static>(&self, f: impl FnOnce(&mut Engine) -> T + Send + 'static) -> T {
-        self.core(move |core| {
-            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))
+        self.core(move |core| core.editing(f))
     }
 
     /// Reads from the engine.

tauri/src/tests.rs

@@ -50,6 +50,20 @@ impl App {
         }
     }
 
+    /// The error of a command that fails.
+    fn error(&self, cmd: &str, args: Value) -> Value {
+        let request = InvokeRequest {
+            cmd: cmd.into(),
+            callback: CallbackFn(0),
+            error: CallbackFn(1),
+            url: "tauri://localhost".parse().unwrap(),
+            body: InvokeBody::Json(args),
+            headers: Default::default(),
+            invoke_key: INVOKE_KEY.to_string(),
+        };
+        get_ipc_response(&self.0, request).expect_err(&format!("{cmd} did not fail"))
+    }
+
     fn bytes(&self, cmd: &str, args: Value) -> Vec<u8> {
         match self.request(cmd, InvokeBody::Json(args), Default::default()) {
             InvokeResponseBody::Raw(bytes) => bytes,
@@ -500,9 +514,9 @@ fn files_made_from_a_file_of_the_open_folder_are_written_next_to_it_at_once() {
         .values()
         .map(|file| file.path.clone().unwrap())
         .collect();
-    // the copy is in the subfolder, named after the file
+    // the copy is in the subfolder, named after its name (the one in the file, not of the file)
     assert!(
-        paths.iter().any(|path| path.ends_with("sub/track (2).gpx")),
+        paths.iter().any(|path| path.ends_with("sub/simple.gpx")),
         "{paths:?}"
     );
     // the new file went where the selected file is
@@ -536,8 +550,8 @@ fn the_parts_of_a_split_are_numbered_from_one() {
         .map(|entry| entry.file_name().to_string_lossy().into_owned())
         .collect();
     names.sort();
-    // the file stays, its two parts are numbered from 1
-    assert_eq!(names, ["track (1).gpx", "track (2).gpx", "track.gpx"]);
+    // the file stays, its two parts are numbered from 1, under their name (the one in the file)
+    assert_eq!(names, ["simple (1).gpx", "simple (2).gpx", "track.gpx"]);
     std::fs::remove_dir_all(folder).unwrap();
 }
 
@@ -581,8 +595,18 @@ fn name_of(app: &App, id: &str) -> String {
         .to_string()
 }
 
+/// The name of the file on the disk of an open file, without its extension.
+fn file_stem_of(app: &App, id: &str) -> String {
+    let path = app.call("workspace_state", json!({}))["files"][id]["path"].clone();
+    std::path::Path::new(path.as_str().unwrap())
+        .file_stem()
+        .unwrap()
+        .to_string_lossy()
+        .into_owned()
+}
+
 #[test]
-fn a_file_is_called_like_its_file_and_renaming_it_renames_the_file() {
+fn a_file_keeps_its_name_and_its_file_is_renamed_apart() {
     let app = App::new();
     let folder = folder_with(&["outside", "taken"]);
     let outcome = app.call(
@@ -590,76 +614,62 @@ fn a_file_is_called_like_its_file_and_renaming_it_renames_the_file() {
         json!({ "paths": [folder.join("outside.gpx")] }),
     );
     let id = outcome["added"][0].as_str().unwrap().to_string();
+    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();
 
-    // 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
-    );
+    // the name in the file is its name, not the one of the file
+    assert_eq!(name_of(&app, &id), "Inside");
+    assert_eq!(file_stem_of(&app, &id), "outside");
+    assert_eq!(dirty(&app), false);
 
-    // renaming renames the file on the disk
+    // renaming it is an edit of the file only, which is undone
     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("<name>renamed</name>"));
-
-    // undone and redone, on the disk too, still with nothing unsaved
+    assert_eq!(name_of(&app, &id), "renamed");
+    assert_eq!(file_stem_of(&app, &id), "outside");
+    assert_eq!(dirty(&app), true);
     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("<name>outside</name>"));
-    app.call("redo", json!({}));
-    assert_eq!(name_of(&app, &id), "renamed");
-    assert!(folder.join("renamed.gpx").exists() && !folder.join("outside.gpx").exists());
+    assert_eq!(name_of(&app, &id), "Inside");
     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());
+    // the file on the disk is renamed apart: not an edit, the file is as saved as it was
+    let renamed = app.call("rename_file", json!({ "fileId": id, "name": "moved" }));
+    assert_eq!(renamed, true);
+    assert!(folder.join("moved.gpx").exists() && !folder.join("outside.gpx").exists());
+    assert_eq!(file_stem_of(&app, &id), "moved");
+    assert_eq!(name_of(&app, &id), "Inside");
+    assert_eq!(dirty(&app), false);
+    assert_eq!(app.call("can_undo", json!({})), false);
+    // saving writes to the new place, under the name of the file
+    app.call("metadata", json!({ "name": "saved", "desc": "" }));
+    app.call("save_files", json!({ "fileIds": [id] }));
+    assert!(inside("moved").contains("<name>saved</name>"));
+    assert_eq!(name_of(&app, &id), "saved");
+
+    // the name of another file is not taken
+    let error = app.error("rename_file", json!({ "fileId": id, "name": "taken" }));
+    assert_eq!(error, crate::workspace::NAME_TAKEN);
+    assert!(folder.join("moved.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");
+    app.call("rename_file", json!({ "fileId": id, "name": "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");
+    app.call("rename_file", json!({ "fileId": id, "name": "A_B" }));
     let names: Vec<String> = 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);
+    assert_eq!(name_of(&app, &id), "saved");
     std::fs::remove_dir_all(folder).unwrap();
 }
 
 #[test]
-fn a_new_file_written_under_another_name_takes_it() {
+fn a_new_file_written_under_another_name_keeps_its_own() {
     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
@@ -669,7 +679,8 @@ fn a_new_file_written_under_another_name_takes_it() {
     );
     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)");
+    // the file keeps its name, whatever the name of the file it was written to
+    assert_eq!(name_of(&app, &id), "route");
     assert_eq!(
         app.call("workspace_state", json!({}))["files"][&id]["dirty"],
         false
@@ -698,7 +709,7 @@ fn a_file_open_as_a_preview_is_replaced_until_it_is_kept_or_edited() {
             .as_array()
             .unwrap()
             .iter()
-            .map(|id| name_of(app, id.as_str().unwrap()))
+            .map(|id| file_stem_of(app, id.as_str().unwrap()))
             .collect()
     };
 

tauri/src/workspace.rs

@@ -695,8 +695,8 @@ 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), which the file then
-    /// has (see `write_file`). 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). Stops at the first
+    /// error, the files that were written stay written.
     pub fn save(
         &mut self,
         engine: &mut Engine,
@@ -725,8 +725,7 @@ impl Workspace {
 
     /// Writes the new files among `ids` in their folder, under their name. 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)`). The files are called like the file
-    /// they are written to.
+    /// of a split, are numbered from 1 (`name (1)`, `name (2)`). The files keep their name.
     pub fn save_new(&mut self, engine: &mut Engine, ids: &[Uuid]) -> Result<api::Outcome, String> {
         let names: Vec<Option<(Uuid, PathBuf, String)>> = ids
             .iter()
@@ -760,30 +759,6 @@ impl Workspace {
         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<Uuid, (String, bool)> {
-        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 can be given. A file on the disk is called like its file: the name is
-    /// one that a file can have (see `file_name`), and not the one of another file next to it
-    /// (`NAME_TAKEN`). The other files get theirs when they are written.
-    pub fn name_for(&self, id: Uuid, name: &str) -> Result<String, String> {
-        if !self.links.contains_key(&id) {
-            return Ok(name.to_owned());
-        }
-        let name = file_name(name);
-        self.renamed_path(id, &name)?;
-        Ok(name)
-    }
-
     /// 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.
     fn renamed_path(&self, id: Uuid, name: &str) -> Result<Option<PathBuf>, String> {
@@ -813,49 +788,24 @@ impl Workspace {
         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<Uuid, (String, bool)>,
-    ) -> (bool, Vec<String>) {
-        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)
-            {
+    /// Renames the file on the disk of an open file, next to where it is: the name is one that a
+    /// file can have (see `file_name`), and not the one of another file (`NAME_TAKEN`). It is not
+    /// an edit, nothing is undone; the file has the same content, so it is as saved as it was.
+    /// Nothing happens to a file that is not on the disk. Says whether it was renamed.
+    pub fn rename_file(&mut self, id: Uuid, name: &str) -> Result<bool, String> {
+        let Some(path) = self.renamed_path(id, name)? else {
+            return Ok(false);
+        };
+        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 {
+            fs::rename(&link.path, &path).map_err(|error| {
                 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);
-            }
+                RENAME_FAILED.to_owned()
+            })?;
         }
-        (renamed, errors)
+        link.path = path;
+        Ok(true)
     }
 
     /// Writes a file to a path of the user's choice, which is the place of the file from then on.
@@ -873,30 +823,21 @@ impl Workspace {
         self.write_file(engine, id, path)
     }
 
-    /// 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.
+    /// Writes a file to `path`, which it is linked to from then on. Says what changed in the engine
+    /// (nothing: its name is its own, whatever the name of the file it is written to).
     fn write_file(
         &mut self,
         engine: &mut Engine,
         id: Uuid,
         path: &Path,
     ) -> Result<api::Outcome, String> {
-        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 unchanged = api::Outcome::failed_none();
         let Some(bytes) = api::export_file(engine, id, ExportOptions::ALL) else {
-            return Ok(renamed);
+            return Ok(unchanged);
         };
         write_atomically(path, &bytes).map_err(|error| format!("{}: {error}", path.display()))?;
         let Some(signature) = signature(engine, id) else {
-            return Ok(renamed);
+            return Ok(unchanged);
         };
         // the folder, if there is one, may watch what was just written: the hash tells it is known
         self.links.insert(
@@ -911,7 +852,7 @@ impl Workspace {
             },
         );
         self.dirs.remove(&id);
-        Ok(renamed)
+        Ok(unchanged)
     }
 
     /// The folders and the GPX files that are in a folder of the open folder (or in it), the
@@ -1026,8 +967,8 @@ impl Workspace {
         Ok(path)
     }
 
-    /// Renames a file or a folder of the tree that is not an open file (which is renamed in the
-    /// engine, see `follow_names`). A file keeps its extension. Gives the new path.
+    /// Renames a file or a folder of the tree. A file keeps its extension, and an open file keeps its
+    /// name (see `rename_file`). Gives the new path.
     pub fn rename_path(&mut self, path: &Path, name: &str) -> Result<PathBuf, String> {
         let path = self.tree_item(path)?;
         let name = file_name(name);