From 0ecbc6b35d0647c7719f96a5c486797d677dc869 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:36:24 +0800 Subject: [PATCH] fix(workspace): keep a previewed edit valid when its file is saved An edit's preconditions hashed disk contents plus unsaved buffers. Saving a buffer moved its text from one to the other and changed the hash, so a quick fix, rename or refactoring listed just before a save (autosave of a freshly typed scratch, for one) failed with "changed; request the refactoring again", and only the next attempt worked. Unsaved buffers now stand in for their files' bytes in the hash, so saving doesn't change it; edits, external writes and file operations still do. --- .../src/documents/workspace_edit.rs | 67 ++++++++++--------- crates/ide-workspace/tests/workspace_edits.rs | 32 +++++++++ 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/crates/ide-workspace/src/documents/workspace_edit.rs b/crates/ide-workspace/src/documents/workspace_edit.rs index 466987087..10543d2da 100644 --- a/crates/ide-workspace/src/documents/workspace_edit.rs +++ b/crates/ide-workspace/src/documents/workspace_edit.rs @@ -4,7 +4,7 @@ use super::{DocState, Documents, RegEntry, disk_len, keys_for}; use ide_core::error::ErrorKind; use ide_core::{DocId, FileEdit, IdeError, IdeEvent, IdeResult, NullSink, WorkspaceEdit, WorkspaceEditPrecondition}; use parking_lot::Mutex; -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use std::fs; use std::io::Read; use std::path::{Path, PathBuf}; @@ -259,7 +259,9 @@ fn hash_bytes(hash: &mut blake3::Hasher, bytes: &[u8]) { hash.update(bytes); } -fn hash_disk(hash: &mut blake3::Hasher, path: &Path) -> IdeResult<()> { +/// Hashes `path` (recursively for a directory). A file in `unsaved` contributes the bytes its +/// buffer would save, not its disk bytes, so saving a buffer leaves the hash unchanged. +fn hash_disk(hash: &mut blake3::Hasher, path: &Path, unsaved: &HashMap>) -> IdeResult<()> { let meta = match fs::symlink_metadata(path) { Ok(meta) => meta, Err(e) if e.kind() == std::io::ErrorKind::NotFound => { @@ -277,6 +279,10 @@ fn hash_disk(hash: &mut blake3::Hasher, path: &Path) -> IdeResult<()> { if meta.file_type().is_symlink() { hash.update(b"link"); hash_bytes(hash, fs::read_link(path)?.as_os_str().as_encoded_bytes()); + } else if let Some(bytes) = unsaved.get(path).filter(|_| meta.is_file()) { + hash.update(b"file"); + hash.update(&(bytes.len() as u64).to_le_bytes()); + hash.update(bytes); } else if meta.is_file() { hash.update(b"file"); hash.update(&meta.len().to_le_bytes()); @@ -295,7 +301,7 @@ fn hash_disk(hash: &mut blake3::Hasher, path: &Path) -> IdeResult<()> { entries.sort_by_key(|e| e.file_name()); for entry in entries { hash_bytes(hash, entry.file_name().as_encoded_bytes()); - hash_disk(hash, &entry.path())?; + hash_disk(hash, &entry.path(), unsaved)?; } } else { return Err(IdeError::invalid(format!("{} is not a regular file or directory", path.display()))); @@ -303,14 +309,19 @@ fn hash_disk(hash: &mut blake3::Hasher, path: &Path) -> IdeResult<()> { Ok(()) } -/// A reverted edit may leave the dirty flag set even when all saved bytes match. -fn differs_from_disk(state: &DocState) -> IdeResult { +/// The bytes saving `state` would write. +fn saved_bytes(state: &DocState) -> IdeResult> { let text = state.doc.text_for_save(); - let bytes = if state.native_to_ascii { + Ok(if state.native_to_ascii { ide_vfs::encode_properties_ascii(&text).into_bytes() } else { ide_vfs::encode(&text, &state.encoding, state.has_bom)? - }; + }) +} + +/// A reverted edit may leave the dirty flag set even when all saved bytes match. +fn differs_from_disk(state: &DocState) -> IdeResult { + let bytes = saved_bytes(state)?; match fs::read(&state.path) { Ok(disk) => Ok(disk != bytes), Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(true), @@ -329,35 +340,31 @@ impl Documents { Ok(()) } + /// What each path holds: disk contents, with unsaved buffers in place of their files. Saving + /// a buffer doesn't change it; edits, external writes and file operations do. fn fingerprints(&self, paths: &[PathBuf]) -> IdeResult> { + let mut unsaved = HashMap::new(); + for id in self.open_documents() { + self.with_doc(id, |state| { + let file = text_path(&state.path)?; + if state.dirty && paths.iter().any(|path| file.starts_with(path)) && differs_from_disk(state)? { + unsaved.insert(file, saved_bytes(state)?); + } + Ok(()) + })?; + } paths .iter() .map(|path| { let mut hash = blake3::Hasher::new(); - hash_disk(&mut hash, path)?; - let mut dirty = Vec::new(); - for id in self.open_documents() { - self.with_doc(id, |state| { - let file = text_path(&state.path)?; - if state.dirty && file.starts_with(path) && differs_from_disk(state)? { - dirty.push(( - file, - state.doc.text(), - state.encoding.clone(), - state.doc.line_ending(), - state.has_bom, - )); - } - Ok(()) - })?; - } - dirty.sort_by(|a, b| a.0.cmp(&b.0)); - for (file, text, encoding, ending, bom) in dirty { + hash_disk(&mut hash, path, &unsaved)?; + // Unsaved files not on disk (yet) or outside what the walk above reached. + let mut extra: Vec<_> = + unsaved.iter().filter(|(file, _)| file.starts_with(path) && !file.exists()).collect(); + extra.sort_by(|a, b| a.0.cmp(b.0)); + for (file, bytes) in extra { hash_bytes(&mut hash, file.as_os_str().as_encoded_bytes()); - hash_bytes(&mut hash, text.as_bytes()); - hash_bytes(&mut hash, encoding.as_bytes()); - hash_bytes(&mut hash, ending.as_str().as_bytes()); - hash.update(&[u8::from(bom)]); + hash_bytes(&mut hash, bytes); } Ok(WorkspaceEditPrecondition { path: path.clone().into(), hash: hash.finalize().to_hex().to_string() }) }) diff --git a/crates/ide-workspace/tests/workspace_edits.rs b/crates/ide-workspace/tests/workspace_edits.rs index 78bd905bd..792e8344a 100644 --- a/crates/ide-workspace/tests/workspace_edits.rs +++ b/crates/ide-workspace/tests/workspace_edits.rs @@ -144,6 +144,38 @@ fn unversioned_preview_rejects_changed_disk_and_dirty_buffers() { } } +#[test] +fn saving_between_preview_and_apply_keeps_the_edit_valid() { + let tmp = tempfile::tempdir().unwrap(); + // A new scratch (empty on disk) and an edited file, saved after the edit was computed. + for (name, disk, typed) in [("scratch.c", "", "int\n"), ("a.txt", "old\n", "own")] { + let path = tmp.path().join(name); + std::fs::write(&path, disk).unwrap(); + let docs = documents(); + let info = docs.open(&path).unwrap(); + let replace = if disk.is_empty() { 0 } else { 3 }; + docs.edit( + info.id, + info.version, + ChangeSet { changes: vec![TextChange { from: 0, to: replace, insert: typed.into() }] }, + ) + .unwrap(); + let mut change = WorkspaceEdit { files: vec![edit(&path, "new")], ..Default::default() }; + docs.prepare_workspace_edit(&mut change).unwrap(); + docs.save(info.id).unwrap(); + docs.apply_workspace_edit(&change).unwrap_or_else(|e| panic!("{name}: {e}")); + assert!(docs.snapshot(info.id).unwrap().text.starts_with("new"), "{name}"); + // A real change after the preview still invalidates it. + let mut change = WorkspaceEdit { files: vec![edit(&path, "xyz")], ..Default::default() }; + docs.prepare_workspace_edit(&mut change).unwrap(); + let version = docs.snapshot(info.id).unwrap().version; + docs.edit(info.id, version, ChangeSet { changes: vec![TextChange { from: 0, to: 0, insert: "!".into() }] }) + .unwrap(); + docs.save(info.id).unwrap(); + assert!(docs.apply_workspace_edit(&change).is_err(), "{name}"); + } +} + #[test] fn grouped_undo_restores_open_closed_and_unsaved_files() { let tmp = tempfile::tempdir().unwrap();