diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 96049fe..865068c 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -227,11 +227,26 @@ cmd { name = "buffer.save", description = "Save the current buffer to its backin ed.set_status("save vetoed by buffer.before-save") return end + -- `ed.save()` refuses when the file changed on disk since this + -- buffer read it, rather than clobbering the other writer. It + -- reports how to override; `buffer.after-save` must not fire. if ed.save() then pmacs.hook.run("buffer.after-save") end end } +cmd { name = "buffer.save-anyway", + description = "Save, overwriting a file that changed on disk since it was read.", + fn = function() + if not pmacs.hook.run("buffer.before-save") then + ed.set_status("save vetoed by buffer.before-save") + return + end + if ed.save_ignoring_disk_changes() then + pmacs.hook.run("buffer.after-save") + end + end } + -- Editor session ------------------------------------------------------------- cmd { name = "editor.quit", description = "Exit the editor.", diff --git a/src/editor_core.rs b/src/editor_core.rs index 0874bef..1b14c44 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -1229,11 +1229,61 @@ impl EditorCore { /// `buffer.save` Lua command) use the return value to gate /// `buffer.after-save` firing. pub fn save(&mut self) -> bool { + self.save_inner(false) + } + + /// [`save`](Self::save), overwriting the file even though it changed on + /// disk since this buffer read it. The escape hatch for when the user + /// has looked and decided their buffer wins. + pub fn save_ignoring_disk_changes(&mut self) -> bool { + self.save_inner(true) + } + + /// True when writing this buffer to `path` would destroy content the + /// buffer has never seen — i.e. a file exists there whose identity + /// differs from the [`FileMeta`] recorded when the buffer last read or + /// wrote it. + /// + /// Two cases count as "changed": + /// + /// * the buffer recorded a meta and the on-disk meta differs — someone + /// else edited the file (another editor, a `git checkout`); + /// * the buffer recorded **no** meta (a `[new file]`, or a buffer whose + /// path was set without reading) yet a file now exists — it was + /// created underneath us, and we have never seen its contents. + /// + /// A **missing** file is not a clobber: there is nothing there to + /// destroy, so recreating a deleted file saves normally. An unstattable + /// path likewise falls through, and `save_atomic` reports the real + /// error. + #[must_use] + pub fn save_would_clobber(&self, id: BufferId, path: &Path) -> bool { + let Ok(current) = crate::file_io::current_meta(path) else { + return false; // absent, or we cannot stat it + }; + let reg = self.registry.borrow(); + let Ok(buffer) = reg.get(id) else { + return false; + }; + buffer.file_meta() != Some(¤t) + } + + fn save_inner(&mut self, force: bool) -> bool { let id = self.active_buffer_id(); let Some(path) = self.active_buffer_path() else { self.status = "no file (M1: open a file from argv)".into(); return false; }; + // Refuse to silently overwrite a file that changed underneath us. + // Without this, pmacs clobbers another editor's (or a `git + // checkout`'s) writes with a buffer that never saw them. + if !force && self.save_would_clobber(id, &path) { + self.status = format!( + "{} changed on disk since it was read --- M-x buffer.save-anyway to overwrite", + path.display() + ); + return false; + } let len_and_bytes = { let reg = self.registry.borrow(); let buffer = match reg.get(id) { diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 9625fdd..2830bed 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -10870,6 +10870,16 @@ fn install_session(editor: &Table, lua: &Lua, core: &SharedCore) -> mlua::Result lua.create_function(move |_, ()| Ok(cc.borrow_mut().save()))?, )?; } + { + // Overwrite even though the file changed on disk since this buffer + // read it. `save()` refuses that case rather than silently + // clobbering another writer; this is the deliberate override. + let cc = core.clone(); + editor.set( + "save_ignoring_disk_changes", + lua.create_function(move |_, ()| Ok(cc.borrow_mut().save_ignoring_disk_changes()))?, + )?; + } register(editor, lua, core, "quit", |c| c.quit = true)?; register(editor, lua, core, "cancel", |c| { c.status = "Quit".into(); diff --git a/tests/save_clobber_guard_acceptance.rs b/tests/save_clobber_guard_acceptance.rs new file mode 100644 index 0000000..f1700c7 --- /dev/null +++ b/tests/save_clobber_guard_acceptance.rs @@ -0,0 +1,202 @@ +//! `EditorCore::save()` must not silently overwrite a file that changed on +//! disk since the buffer read it. +//! +//! Before this guard, pmacs wrote unconditionally and only *then* recorded +//! the new `FileMeta` — so another editor's (or a `git checkout`'s) writes +//! were destroyed without a word. The comparison seam +//! (`FileMeta: PartialEq`, `file_io::current_meta`) already existed and no +//! caller used it. + +use pmacs::editor::EditorState; +use std::path::PathBuf; +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn tempdir() -> PathBuf { + static SEQ: AtomicUsize = AtomicUsize::new(0); + let d = std::env::temp_dir().join(format!( + "pmacs-clobber-{}-{}", + std::process::id(), + SEQ.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir_all(&d).unwrap(); + d +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn write(p: &std::path::Path, body: &str) { + std::fs::write(p, body).unwrap(); +} + +fn read(p: &std::path::Path) -> String { + std::fs::read_to_string(p).unwrap() +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +/// Open `path` and dirty the buffer. +fn open_and_dirty(s: &EditorState, path: &str) { + exec( + s, + &format!("pmacs.buffer.find_or_open({path:?}); pmacs.window.buffer():insert(0, 'mine ')"), + ); +} + +#[test] +fn save_refuses_to_clobber_a_file_changed_on_disk() { + let dir = tempdir(); + let f = dir.join("a.txt"); + write(&f, "original\n"); + let fs = f.display().to_string(); + + let s = EditorState::new(); + open_and_dirty(&s, &fs); + + // Another writer lands between our read and our save. + write(&f, "THEIRS -- do not destroy\n"); + + let saved: bool = eval(&s, "return pmacs.editor.save()"); + assert!(!saved, "save must refuse"); + assert_eq!( + read(&f), + "THEIRS -- do not destroy\n", + "their content is intact" + ); + assert!( + status(&s).contains("changed on disk") && status(&s).contains("save-anyway"), + "the refusal says what happened and how to override: {:?}", + status(&s) + ); + // The buffer keeps its unsaved edits — nothing was lost on our side. + let modified: bool = eval(&s, "return pmacs.window.buffer():is_modified()"); + assert!(modified); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn the_save_command_does_not_fire_after_save_when_it_refuses() { + let dir = tempdir(); + let f = dir.join("a.txt"); + write(&f, "original\n"); + let fs = f.display().to_string(); + + let s = EditorState::new(); + open_and_dirty(&s, &fs); + exec( + &s, + "_G.after = 0; pmacs.hook.add('buffer.after-save', function() _G.after = _G.after + 1 end)", + ); + write(&f, "theirs\n"); + + exec(&s, "pmacs.command.invoke('buffer.save')"); + let after: i64 = eval(&s, "return _G.after"); + assert_eq!(after, 0, "after-save must not fire on a refused save"); + assert_eq!(read(&f), "theirs\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn save_anyway_overwrites_deliberately_and_resyncs_meta() { + let dir = tempdir(); + let f = dir.join("a.txt"); + write(&f, "original\n"); + let fs = f.display().to_string(); + + let s = EditorState::new(); + open_and_dirty(&s, &fs); + write(&f, "theirs\n"); + assert!(!eval::(&s, "return pmacs.editor.save()")); + + // The user looked and decided their buffer wins. + exec(&s, "pmacs.command.invoke('buffer.save-anyway')"); + assert_eq!(read(&f), "mine original\n", "overwritten on purpose"); + + // The buffer re-syncs to the file it just wrote, so an immediate + // ordinary save is allowed again. + exec(&s, "pmacs.window.buffer():insert(0, 'more ')"); + assert!( + eval::(&s, "return pmacs.editor.save()"), + "meta was refreshed by the forced save" + ); + assert_eq!(read(&f), "more mine original\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn an_unchanged_file_saves_normally_and_repeatedly() { + let dir = tempdir(); + let f = dir.join("a.txt"); + write(&f, "original\n"); + let fs = f.display().to_string(); + + let s = EditorState::new(); + open_and_dirty(&s, &fs); + assert!(eval::(&s, "return pmacs.editor.save()")); + assert_eq!(read(&f), "mine original\n"); + + // A save updates our recorded meta, so the next one is not a false + // positive — the guard must not trip on our own writes. + exec(&s, "pmacs.window.buffer():insert(0, 'again ')"); + assert!(eval::(&s, "return pmacs.editor.save()")); + assert_eq!(read(&f), "again mine original\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn a_deleted_file_is_recreated_not_refused() { + let dir = tempdir(); + let f = dir.join("a.txt"); + write(&f, "original\n"); + let fs = f.display().to_string(); + + let s = EditorState::new(); + open_and_dirty(&s, &fs); + // Nothing on disk to clobber, so recreating it is not data loss. + std::fs::remove_file(&f).unwrap(); + assert!( + eval::(&s, "return pmacs.editor.save()"), + "a vanished file is recreated, not refused" + ); + assert_eq!(read(&f), "mine original\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn a_new_file_buffer_refuses_once_someone_else_creates_the_file() { + let dir = tempdir(); + let missing = dir.join("draft.txt"); + + let s = EditorState::new(); + // The argv `[new file]` shape: a path with nothing on disk, no meta. + exec( + &s, + "_G.nb = pmacs.buffer.create('draft.txt'); pmacs.window.switch_buffer(_G.nb)", + ); + { + let id = s.core.borrow().active_buffer_id(); + s.core + .borrow_mut() + .set_buffer_path(id, Some(missing.clone())); + } + exec(&s, "pmacs.window.buffer():insert(0, 'my draft')"); + + // While we drafted, someone created the file. We have never seen its + // contents, so writing over them is exactly the clobber we refuse. + write(&missing, "theirs\n"); + assert!(!eval::(&s, "return pmacs.editor.save()")); + assert_eq!(read(&missing), "theirs\n"); + + // With the file still absent it would have saved cleanly. + std::fs::remove_file(&missing).unwrap(); + assert!(eval::(&s, "return pmacs.editor.save()")); + assert_eq!(read(&missing), "my draft"); + std::fs::remove_dir_all(&dir).ok(); +}