Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Don't autosave unmodified buffers (#32626)

Browse source 
Closes https://github.com/zed-industries/zed/issues/12091

Proper redo of https://github.com/zed-industries/zed/pull/32603

Release Notes:

- Fixed formatting effects not triggered when saving unmodified
singleton buffers

---------

Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
Co-authored-by: Cole Miller <m@cole-miller.net>
This commit is contained in:
Kirill Bulatov 2025-06-13 01:12:14 +03:00 • committed by GitHub
parent cd018da1ad
commit cef0c415f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 453 additions and 171 deletions
Download patch file Download diff file Expand all files Collapse all files

40
crates/editor/src/editor.rs
View file

@ -208,7 +208,7 @@ use workspace::{
CollaboratorId, Item as WorkspaceItem, ItemId, ItemNavHistory, OpenInTerminal, OpenTerminal,
RestoreOnStartupBehavior, SERIALIZATION_THROTTLE_TIME, SplitDirection, TabBarSettings, Toast,
ViewId, Workspace, WorkspaceId, WorkspaceSettings,
item::{ItemHandle, PreviewTabsSettings},
item::{ItemHandle, PreviewTabsSettings, SaveOptions},
notifications::{DetachAndPromptErr, NotificationId, NotifyTaskExt},
searchable::SearchEvent,
};
@ -1498,7 +1498,7 @@ impl InlayHintRefreshReason {
}
pub enum FormatTarget {
Buffers,
Buffers(HashSet<Entity<Buffer>>),
Ranges(Vec<Range<MultiBufferPoint>>),
}
@ -15601,7 +15601,7 @@ impl Editor {
Some(self.perform_format(
project,
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(self.buffer.read(cx).all_buffers()),
window,
cx,
))
@ -15636,10 +15636,6 @@ impl Editor {
))
}
fn save_non_dirty_buffers(&self, cx: &App) -> bool {
self.is_singleton(cx) && EditorSettings::get_global(cx).save_non_dirty_buffers
}
fn perform_format(
&mut self,
project: Entity<Project>,
@ -15650,13 +15646,7 @@ impl Editor {
) -> Task<Result<()>> {
let buffer = self.buffer.clone();
let (buffers, target) = match target {
FormatTarget::Buffers => {
let mut buffers = buffer.read(cx).all_buffers();
if trigger == FormatTrigger::Save && !self.save_non_dirty_buffers(cx) {
buffers.retain(|buffer| buffer.read(cx).is_dirty());
}
(buffers, LspFormatTarget::Buffers)
}
FormatTarget::Buffers(buffers) => (buffers, LspFormatTarget::Buffers),
FormatTarget::Ranges(selection_ranges) => {
let multi_buffer = buffer.read(cx);
let snapshot = multi_buffer.read(cx);
@ -17101,7 +17091,16 @@ impl Editor {
}
if let Some(project) = self.project.clone() {
self.save(true, project, window, cx).detach_and_log_err(cx);
self.save(
SaveOptions {
format: true,
autosave: false,
},
project,
window,
cx,
)
.detach_and_log_err(cx);
}
}
@ -17133,7 +17132,16 @@ impl Editor {
});
if let Some(project) = self.project.clone() {
self.save(true, project, window, cx).detach_and_log_err(cx);
self.save(
SaveOptions {
format: true,
autosave: false,
},
project,
window,
cx,
)
.detach_and_log_err(cx);
}
}

7
crates/editor/src/editor_settings.rs
View file

@ -50,7 +50,6 @@ pub struct EditorSettings {
pub diagnostics_max_severity: Option<DiagnosticSeverity>,
pub inline_code_actions: bool,
pub drag_and_drop_selection: bool,
pub save_non_dirty_buffers: bool,
}
#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
@ -503,12 +502,6 @@ pub struct EditorSettingsContent {
///
/// Default: true
pub drag_and_drop_selection: Option<bool>,
/// Whether to save singleton buffers that are not dirty.
/// This will "touch" the file and related tools enabled, e.g. formatters.
///
/// Default: true
pub save_non_dirty_buffers: Option<bool>,
}
// Toolbar related settings

316
crates/editor/src/editor_tests.rs
View file

@ -56,7 +56,7 @@ use util::{
};
use workspace::{
CloseActiveItem, CloseAllItems, CloseInactiveItems, NavigationEntry, OpenOptions, ViewId,
item::{FollowEvent, FollowableItem, Item, ItemHandle},
item::{FollowEvent, FollowableItem, Item, ItemHandle, SaveOptions},
};
#[gpui::test]
@ -9041,7 +9041,15 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
);
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
cx.executor().start_waiting();
@ -9073,7 +9081,15 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
);
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
cx.executor().advance_clock(super::FORMAT_TIMEOUT);
@ -9085,38 +9101,6 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
);
}
// For non-dirty buffer and the corresponding settings, no formatting request should be sent
{
assert!(!cx.read(|cx| editor.is_dirty(cx)));
cx.update_global::<SettingsStore, _>(|settings, cx| {
settings.update_user_settings::<EditorSettings>(cx, |settings| {
settings.save_non_dirty_buffers = Some(false);
});
});
fake_server.set_request_handler::<lsp::request::Formatting, _, _>(move |_, _| async move {
panic!("Should not be invoked on non-dirty buffer when configured so");
});
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
})
.unwrap();
cx.executor().start_waiting();
save.await;
assert_eq!(
editor.update(cx, |editor, cx| editor.text(cx)),
"one\ntwo\nthree\n"
);
assert!(!cx.read(|cx| editor.is_dirty(cx)));
}
cx.update_global::<SettingsStore, _>(|settings, cx| {
settings.update_user_settings::<EditorSettings>(cx, |settings| {
settings.save_non_dirty_buffers = Some(false);
});
});
// Set rust language override and assert overridden tabsize is sent to language server
update_test_language_settings(cx, |settings| {
settings.languages.insert(
@ -9144,7 +9128,15 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
});
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
cx.executor().start_waiting();
@ -9312,7 +9304,15 @@ async fn test_multibuffer_format_during_save(cx: &mut TestAppContext) {
cx.executor().start_waiting();
let save = multi_buffer_editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
@ -9356,6 +9356,170 @@ async fn test_multibuffer_format_during_save(cx: &mut TestAppContext) {
});
}
#[gpui::test]
async fn test_autosave_with_dirty_buffers(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/dir"),
json!({
"file1.rs": "fn main() { println!(\"hello\"); }",
"file2.rs": "fn test() { println!(\"test\"); }",
"file3.rs": "fn other() { println!(\"other\"); }\n",
}),
)
.await;
let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
let workspace = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*workspace.deref(), cx);
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
language_registry.add(rust_lang());
let worktree = project.update(cx, |project, cx| project.worktrees(cx).next().unwrap());
let worktree_id = worktree.update(cx, |worktree, _| worktree.id());
// Open three buffers
let buffer_1 = project
.update(cx, |project, cx| {
project.open_buffer((worktree_id, "file1.rs"), cx)
})
.await
.unwrap();
let buffer_2 = project
.update(cx, |project, cx| {
project.open_buffer((worktree_id, "file2.rs"), cx)
})
.await
.unwrap();
let buffer_3 = project
.update(cx, |project, cx| {
project.open_buffer((worktree_id, "file3.rs"), cx)
})
.await
.unwrap();
// Create a multi-buffer with all three buffers
let multi_buffer = cx.new(|cx| {
let mut multi_buffer = MultiBuffer::new(ReadWrite);
multi_buffer.push_excerpts(
buffer_1.clone(),
[ExcerptRange::new(Point::new(0, 0)..Point::new(1, 0))],
cx,
);
multi_buffer.push_excerpts(
buffer_2.clone(),
[ExcerptRange::new(Point::new(0, 0)..Point::new(1, 0))],
cx,
);
multi_buffer.push_excerpts(
buffer_3.clone(),
[ExcerptRange::new(Point::new(0, 0)..Point::new(1, 0))],
cx,
);
multi_buffer
});
let editor = cx.new_window_entity(|window, cx| {
Editor::new(
EditorMode::full(),
multi_buffer,
Some(project.clone()),
window,
cx,
)
});
// Edit only the first buffer
editor.update_in(cx, |editor, window, cx| {
editor.change_selections(Some(Autoscroll::Next), window, cx, |s| {
s.select_ranges(Some(10..10))
});
editor.insert("// edited", window, cx);
});
// Verify that only buffer 1 is dirty
buffer_1.update(cx, |buffer, _| assert!(buffer.is_dirty()));
buffer_2.update(cx, |buffer, _| assert!(!buffer.is_dirty()));
buffer_3.update(cx, |buffer, _| assert!(!buffer.is_dirty()));
// Get write counts after file creation (files were created with initial content)
// We expect each file to have been written once during creation
let write_count_after_creation_1 = fs.write_count_for_path(path!("/dir/file1.rs"));
let write_count_after_creation_2 = fs.write_count_for_path(path!("/dir/file2.rs"));
let write_count_after_creation_3 = fs.write_count_for_path(path!("/dir/file3.rs"));
// Perform autosave
let save_task = editor.update_in(cx, |editor, window, cx| {
editor.save(
SaveOptions {
format: true,
autosave: true,
},
project.clone(),
window,
cx,
)
});
save_task.await.unwrap();
// Only the dirty buffer should have been saved
assert_eq!(
fs.write_count_for_path(path!("/dir/file1.rs")) - write_count_after_creation_1,
1,
"Buffer 1 was dirty, so it should have been written once during autosave"
);
assert_eq!(
fs.write_count_for_path(path!("/dir/file2.rs")) - write_count_after_creation_2,
0,
"Buffer 2 was clean, so it should not have been written during autosave"
);
assert_eq!(
fs.write_count_for_path(path!("/dir/file3.rs")) - write_count_after_creation_3,
0,
"Buffer 3 was clean, so it should not have been written during autosave"
);
// Verify buffer states after autosave
buffer_1.update(cx, |buffer, _| assert!(!buffer.is_dirty()));
buffer_2.update(cx, |buffer, _| assert!(!buffer.is_dirty()));
buffer_3.update(cx, |buffer, _| assert!(!buffer.is_dirty()));
// Now perform a manual save (format = true)
let save_task = editor.update_in(cx, |editor, window, cx| {
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
});
save_task.await.unwrap();
// During manual save, clean buffers don't get written to disk
// They just get did_save called for language server notifications
assert_eq!(
fs.write_count_for_path(path!("/dir/file1.rs")) - write_count_after_creation_1,
1,
"Buffer 1 should only have been written once total (during autosave, not manual save)"
);
assert_eq!(
fs.write_count_for_path(path!("/dir/file2.rs")) - write_count_after_creation_2,
0,
"Buffer 2 should not have been written at all"
);
assert_eq!(
fs.write_count_for_path(path!("/dir/file3.rs")) - write_count_after_creation_3,
0,
"Buffer 3 should not have been written at all"
);
}
#[gpui::test]
async fn test_range_format_during_save(cx: &mut TestAppContext) {
init_test(cx, |_| {});
@ -9399,7 +9563,15 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
fake_server
@ -9442,7 +9614,15 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
);
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
cx.executor().advance_clock(super::FORMAT_TIMEOUT);
@ -9454,35 +9634,27 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
);
assert!(!cx.read(|cx| editor.is_dirty(cx)));
// For non-dirty buffer, a formatting request should be sent anyway with the default settings
// where non-dirty singleton buffers are saved and formatted anyway.
// For non-dirty buffer, no formatting request should be sent
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: false,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
fake_server
.set_request_handler::<lsp::request::RangeFormatting, _, _>(move |params, _| async move {
assert_eq!(
params.text_document.uri,
lsp::Url::from_file_path(path!("/file.rs")).unwrap()
);
assert_eq!(params.options.tab_size, 4);
Ok(Some(vec![lsp::TextEdit::new(
lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(1, 0)),
", ".to_string(),
)]))
let _pending_format_request = fake_server
.set_request_handler::<lsp::request::RangeFormatting, _, _>(move |_, _| async move {
panic!("Should not be invoked");
})
.next()
.await;
cx.executor().advance_clock(super::FORMAT_TIMEOUT);
.next();
cx.executor().start_waiting();
save.await;
assert_eq!(
editor.update(cx, |editor, cx| editor.text(cx)),
"one, two\nthree\n"
);
assert!(!cx.read(|cx| editor.is_dirty(cx)));
// Set Rust language override and assert overridden tabsize is sent to language server
update_test_language_settings(cx, |settings| {
@ -9501,7 +9673,15 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
assert!(cx.read(|cx| editor.is_dirty(cx)));
let save = editor
.update_in(cx, |editor, window, cx| {
editor.save(true, project.clone(), window, cx)
editor.save(
SaveOptions {
format: true,
autosave: false,
},
project.clone(),
window,
cx,
)
})
.unwrap();
fake_server
@ -9585,7 +9765,7 @@ async fn test_document_format_manual_trigger(cx: &mut TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)
@ -9631,7 +9811,7 @@ async fn test_document_format_manual_trigger(cx: &mut TestAppContext) {
editor.perform_format(
project,
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)
@ -9809,7 +9989,7 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)
@ -9845,7 +10025,7 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)
@ -15387,7 +15567,7 @@ async fn test_document_format_with_prettier(cx: &mut TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)
@ -15407,7 +15587,7 @@ async fn test_document_format_with_prettier(cx: &mut TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffers,
FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
window,
cx,
)

84
crates/editor/src/items.rs
View file

@ -40,7 +40,7 @@ use ui::{IconDecorationKind, prelude::*};
use util::{ResultExt, TryFutureExt, paths::PathExt};
use workspace::{
CollaboratorId, ItemId, ItemNavHistory, ToolbarItemLocation, ViewId, Workspace, WorkspaceId,
item::{FollowableItem, Item, ItemEvent, ProjectItem},
item::{FollowableItem, Item, ItemEvent, ProjectItem, SaveOptions},
searchable::{Direction, SearchEvent, SearchableItem, SearchableItemHandle},
};
use workspace::{
@ -805,7 +805,7 @@ impl Item for Editor {
fn save(
&mut self,
format: bool,
options: SaveOptions,
project: Entity<Project>,
window: &mut Window,
cx: &mut Context<Self>,
@ -816,48 +816,54 @@ impl Item for Editor {
.into_iter()
.map(|handle| handle.read(cx).base_buffer().unwrap_or(handle.clone()))
.collect::<HashSet<_>>();
let save_non_dirty_buffers = self.save_non_dirty_buffers(cx);
cx.spawn_in(window, async move |editor, cx| {
if format {
editor
.update_in(cx, |editor, window, cx| {
editor.perform_format(
project.clone(),
FormatTrigger::Save,
FormatTarget::Buffers,
window,
cx,
)
// let mut buffers_to_save =
let buffers_to_save = if self.buffer.read(cx).is_singleton() && !options.autosave {
buffers.clone()
} else {
buffers
.iter()
.filter(|buffer| buffer.read(cx).is_dirty())
.cloned()
.collect()
};
cx.spawn_in(window, async move |this, cx| {
if options.format {
this.update_in(cx, |editor, window, cx| {
editor.perform_format(
project.clone(),
FormatTrigger::Save,
FormatTarget::Buffers(buffers_to_save.clone()),
window,
cx,
)
})?
.await?;
}
if !buffers_to_save.is_empty() {
project
.update(cx, |project, cx| {
project.save_buffers(buffers_to_save.clone(), cx)
})?
.await?;
}
if save_non_dirty_buffers {
project
.update(cx, |project, cx| project.save_buffers(buffers, cx))?
.await?;
} else {
// For multi-buffers, only format and save the buffers with changes.
// For clean buffers, we simulate saving by calling `Buffer::did_save`,
// so that language servers or other downstream listeners of save events get notified.
let (dirty_buffers, clean_buffers) = buffers.into_iter().partition(|buffer| {
buffer
.read_with(cx, |buffer, _| buffer.is_dirty() || buffer.has_conflict())
.unwrap_or(false)
});
// Notify about clean buffers for language server events
let buffers_that_were_not_saved: Vec<_> = buffers
.into_iter()
.filter(|b| !buffers_to_save.contains(b))
.collect();
project
.update(cx, |project, cx| project.save_buffers(dirty_buffers, cx))?
.await?;
for buffer in clean_buffers {
buffer
.update(cx, |buffer, cx| {
let version = buffer.saved_version().clone();
let mtime = buffer.saved_mtime();
buffer.did_save(version, mtime, cx);
})
.ok();
}
for buffer in buffers_that_were_not_saved {
buffer
.update(cx, |buffer, cx| {
let version = buffer.saved_version().clone();
let mtime = buffer.saved_mtime();
buffer.did_save(version, mtime, cx);
})
.ok();
}
Ok(())

6
crates/editor/src/proposed_changes_editor.rs
View file

@ -12,7 +12,7 @@ use text::ToOffset;
use ui::{ButtonLike, KeyBinding, prelude::*};
use workspace::{
Item, ItemHandle as _, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace,
searchable::SearchableItemHandle,
item::SaveOptions, searchable::SearchableItemHandle,
};
pub struct ProposedChangesEditor {
@ -351,13 +351,13 @@ impl Item for ProposedChangesEditor {
fn save(
&mut self,
format: bool,
options: SaveOptions,
project: Entity<Project>,
window: &mut Window,
cx: &mut Context<Self>,
) -> Task<anyhow::Result<()>> {
self.editor.update(cx, |editor, cx| {
Item::save(editor, format, project, window, cx)
Item::save(editor, options, project, window, cx)
})
}
}