Fix redo after noop format (#34898)
Closes #31917 Previously, as of #28457 we used a hack, creating an empty transaction in the history that we then merged formatting changes into in order to correctly identify concurrent edits to the buffer while formatting was happening. This caused issues with noop formatting however, as using the normal API of the buffer history (in an albeit weird way) resulted in the redo stack being cleared, regardless of whether the formatting transaction included edits or not, which is the correct behavior in all other contexts. This PR fixes the redo issue by codifying the behavior formatting wants, that being the ability to push an empty transaction to the history with no other side-effects (i.e. clearing the redo stack) to detect concurrent edits, with the tradeoff being that it must then manually remove the transaction later if no changes occurred from the formatting. The redo stack is still cleared when there are formatting edits, as the individual format steps use the normal `{start,end}_transaction` methods which clear the redo stack if the finished transaction isn't empty. Release Notes: - Fixed an issue where redo would not work after buffer formatting (including formatting on save) when the formatting did not result in any changes
This commit is contained in:
parent
96f9942791
commit
2b888e1d30
4 changed files with 137 additions and 8 deletions
|
@ -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<D>(
|
||||
&self,
|
||||
transaction_id: TransactionId,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue