diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 4efb052c71..fbb877796c 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9570,6 +9570,74 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) { } } +#[gpui::test] +async fn test_redo_after_noop_format(cx: &mut TestAppContext) { + init_test(cx, |settings| { + settings.defaults.ensure_final_newline_on_save = Some(false); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.txt"), "foo".into()).await; + + let project = Project::test(fs, [path!("/file.txt").as_ref()], cx).await; + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.txt"), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([0..0]) + }); + }); + assert!(!cx.read(|cx| editor.is_dirty(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.handle_input("\n", window, cx) + }); + cx.run_until_parked(); + save(&editor, &project, cx).await; + assert_eq!("\nfoo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.undo(&Default::default(), window, cx); + }); + save(&editor, &project, cx).await; + assert_eq!("foo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.redo(&Default::default(), window, cx); + }); + cx.run_until_parked(); + assert_eq!("\nfoo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + async fn save(editor: &Entity, project: &Entity, cx: &mut VisualTestContext) { + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + cx.executor().start_waiting(); + save.await; + assert!(!cx.read(|cx| editor.is_dirty(cx))); + } +} + #[gpui::test] async fn test_multibuffer_format_during_save(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -22708,7 +22776,7 @@ pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsC workspace::init_settings(cx); crate::init(cx); }); - + zlog::init_test(); update_test_language_settings(cx, f); } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 59aa63ff38..83517accc2 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -2072,6 +2072,21 @@ impl Buffer { self.text.push_transaction(transaction, now); } + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + pub fn push_empty_transaction(&mut self, now: Instant) -> TransactionId { + self.text.push_empty_transaction(now) + } + /// Prevent the last transaction from being grouped with any subsequent transactions, /// even if they occur with the buffer's undo grouping duration. pub fn finalize_last_transaction(&mut self) -> Option<&Transaction> { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 28cbfcdd18..0cd375e0c5 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1274,15 +1274,11 @@ impl LocalLspStore { // grouped with the previous transaction in the history // based on the transaction group interval buffer.finalize_last_transaction(); - let transaction_id = buffer + buffer .start_transaction() .context("transaction already open")?; - let transaction = buffer - .get_transaction(transaction_id) - .expect("transaction started") - .clone(); buffer.end_transaction(cx); - buffer.push_transaction(transaction, cx.background_executor().now()); + let transaction_id = buffer.push_empty_transaction(cx.background_executor().now()); buffer.finalize_last_transaction(); anyhow::Ok(transaction_id) })??; diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index a2742081f4..aa9682029e 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -320,7 +320,39 @@ impl History { last_edit_at: now, suppress_grouping: false, }); - self.redo_stack.clear(); + } + + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + fn push_empty_transaction( + &mut self, + start: clock::Global, + now: Instant, + clock: &mut clock::Lamport, + ) -> TransactionId { + assert_eq!(self.transaction_depth, 0); + let id = clock.tick(); + let transaction = Transaction { + id, + start, + edit_ids: Vec::new(), + }; + self.undo_stack.push(HistoryEntry { + transaction, + first_edit_at: now, + last_edit_at: now, + suppress_grouping: false, + }); + id } fn push_undo(&mut self, op_id: clock::Lamport) { @@ -1495,6 +1527,24 @@ impl Buffer { self.history.push_transaction(transaction, now); } + /// Differs from `push_transaction` in that it does not clear the redo stack. + /// The caller responsible for + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + pub fn push_empty_transaction(&mut self, now: Instant) -> TransactionId { + self.history + .push_empty_transaction(self.version.clone(), now, &mut self.lamport_clock) + } + pub fn edited_ranges_for_transaction_id( &self, transaction_id: TransactionId,