dino created
Refactor how `project_panel::undo::UndoManager` supports redo
operations. The initial plan before this commit was to have a single
`VecDeque` complemented by a cursor, where undoing an operation meant
decreasing the cursor, and redoing meant increasing the cursor.
That works fine for almost all cases but, in the particular case of a
`Create` or `Trash` operation, we'll eventually need to keep track of
the trashed entry when we wish to redo a `Create` operation, or undo a
`Trash` operation.
As such, this commit re-introduces the idea of using two different
stacks, one for the list of operations to undo and one for the list of
operations to redo, namely
`project_panel::undo::UndoManager::undo_stack` and
`project_panel::undo::UndoManager::redo_stack`.
It also updates the overall approach such that:
1. When recording an operation, we invert it and push that to the undo
stack
2. When undoing, we execute the most recent operation in the undo stack
and push its inverse to the redo stack
3. When redoing, we execute the most recent operation in the redo stack
and push its inverse to the undo stack
This makes it so that `UndoManager` more or less only needs to know
which operation to execute and where to push the resulting operation,
allowing us to get rid of `UndoManager::undo_operation` and
`UndoManager::redo_operation` specific methods.
Unfortunately, one thing that was lost in this refactor is that, right
now, we early-return in the `Batch` operation, as soon as a single
operation fails, so we'll always only show one message, we might want to
revisit this later.crates/project_panel/src/project_panel.rs | 2
crates/project_panel/src/project_panel_tests.rs | 2
crates/project_panel/src/undo.rs | 726 +++++++++++-------
3 files changed, 423 insertions(+), 307 deletions(-)
@@ -902,7 +902,7 @@ impl ProjectPanel {
unfolded_dir_ids: Default::default(),
},
update_visible_entries_task: Default::default(),
- undo_manager: UndoManager::new(workspace.weak_handle()),
+ undo_manager: UndoManager::new(workspace.weak_handle(), cx.weak_entity()),
};
this.update_visible_entries(None, false, false, window, cx);
@@ -9637,7 +9637,7 @@ async fn test_hide_hidden_entries(cx: &mut gpui::TestAppContext) {
);
}
-fn select_path(panel: &Entity<ProjectPanel>, path: &str, cx: &mut VisualTestContext) {
+pub(crate) fn select_path(panel: &Entity<ProjectPanel>, path: &str, cx: &mut VisualTestContext) {
let path = rel_path(path);
panel.update_in(cx, |panel, window, cx| {
for worktree in panel.project.read(cx).worktrees(cx).collect::<Vec<_>>() {
@@ -1,8 +1,9 @@
-use anyhow::anyhow;
+use crate::ProjectPanel;
+use anyhow::{Result, anyhow};
use gpui::{AppContext, SharedString, Task, WeakEntity};
use project::ProjectPath;
use std::collections::VecDeque;
-use ui::{App, IntoElement, Label, ParentElement, Styled, v_flex};
+use ui::App;
use workspace::{
Workspace,
notifications::{NotificationId, simple_message_notification::MessageNotification},
@@ -14,169 +15,143 @@ const MAX_UNDO_OPERATIONS: usize = 10_000;
pub enum ProjectPanelOperation {
Batch(Vec<ProjectPanelOperation>),
Create { project_path: ProjectPath },
+ Trash { project_path: ProjectPath },
Rename { from: ProjectPath, to: ProjectPath },
}
+impl ProjectPanelOperation {
+ fn inverse(&self) -> Self {
+ match self {
+ Self::Create { project_path } => Self::Trash {
+ project_path: project_path.clone(),
+ },
+ Self::Trash { project_path } => Self::Create {
+ project_path: project_path.clone(),
+ },
+ Self::Rename { from, to } => Self::Rename {
+ from: to.clone(),
+ to: from.clone(),
+ },
+ // When inverting a batch of operations, we reverse the order of
+ // operations to handle dependencies between them. For example, if a
+ // batch contains the following order of operations:
+ //
+ // 1. Create `src/`
+ // 2. Create `src/main.rs`
+ //
+ // If we first tried to revert the directory creation, it would fail
+ // because there's still files inside the directory.
+ Self::Batch(operations) => Self::Batch(
+ operations
+ .iter()
+ .rev()
+ .map(|operation| operation.inverse())
+ .collect(),
+ ),
+ }
+ }
+}
+
pub struct UndoManager {
workspace: WeakEntity<Workspace>,
- history: VecDeque<ProjectPanelOperation>,
- /// Keeps track of the cursor position in the undo stack so we can easily
- /// undo by picking the current operation in the stack and decreasing the
- /// cursor, as well as redoing, by picking the next operation in the stack
- /// and increasing the cursor.
- cursor: usize,
+ panel: WeakEntity<ProjectPanel>,
+ undo_stack: VecDeque<ProjectPanelOperation>,
+ redo_stack: Vec<ProjectPanelOperation>,
/// Maximum number of operations to keep on the undo history.
limit: usize,
}
impl UndoManager {
- pub fn new(workspace: WeakEntity<Workspace>) -> Self {
- Self::new_with_limit(workspace, MAX_UNDO_OPERATIONS)
+ pub fn new(workspace: WeakEntity<Workspace>, panel: WeakEntity<ProjectPanel>) -> Self {
+ Self::new_with_limit(workspace, panel, MAX_UNDO_OPERATIONS)
}
- pub fn new_with_limit(workspace: WeakEntity<Workspace>, limit: usize) -> Self {
+ pub fn new_with_limit(
+ workspace: WeakEntity<Workspace>,
+ panel: WeakEntity<ProjectPanel>,
+ limit: usize,
+ ) -> Self {
Self {
workspace,
+ panel,
limit,
- cursor: 0,
- history: VecDeque::new(),
+ undo_stack: VecDeque::new(),
+ redo_stack: Vec::new(),
}
}
pub fn can_undo(&self) -> bool {
- self.cursor > 0
+ !self.undo_stack.is_empty()
}
pub fn can_redo(&self) -> bool {
- self.cursor < self.history.len()
+ !self.redo_stack.is_empty()
}
pub fn undo(&mut self, cx: &mut App) {
- if self.cursor == 0 {
- return;
- }
-
- // We don't currently care whether the undo operation failed or
- // succeeded, so the cursor can always be updated, as we just assume
- // we'll be attempting to undo the next operation, even if undoing
- // the previous one failed.
- self.cursor -= 1;
-
- if let Some(operation) = self.history.get(self.cursor) {
- let task = self.undo_operation(operation, cx);
+ if let Some(operation) = self.undo_stack.pop_back() {
+ let task = self.execute_operation(&operation, cx);
+ let panel = self.panel.clone();
let workspace = self.workspace.clone();
- cx.spawn(async move |cx| {
- let errors = task.await;
- if !errors.is_empty() {
- cx.update(|cx| {
- let messages = errors
- .iter()
- .map(|err| SharedString::from(err.to_string()))
- .collect();
-
- Self::show_errors(
- "Failed to undo Project Panel Operation(s)",
- workspace,
- messages,
- cx,
- )
- })
- }
+ cx.spawn(async move |cx| match task.await {
+ Ok(operation) => panel.update(cx, |panel, _cx| {
+ panel.undo_manager.redo_stack.push(operation)
+ }),
+ Err(err) => cx.update(|cx| {
+ Self::show_error(
+ "Failed to undo Project Panel Operation(s)",
+ workspace,
+ err.to_string().into(),
+ cx,
+ );
+
+ Ok(())
+ }),
})
.detach();
}
}
pub fn redo(&mut self, cx: &mut App) {
- if self.cursor >= self.history.len() {
- return;
- }
-
- if let Some(operation) = self.history.get(self.cursor) {
- let task = self.redo_operation(operation, cx);
+ if let Some(operation) = self.redo_stack.pop() {
+ let task = self.execute_operation(&operation, cx);
+ let panel = self.panel.clone();
let workspace = self.workspace.clone();
- cx.spawn(async move |cx| {
- let errors = task.await;
- if !errors.is_empty() {
- cx.update(|cx| {
- let messages = errors
- .iter()
- .map(|err| SharedString::from(err.to_string()))
- .collect();
-
- Self::show_errors(
- "Failed to redo Project Panel Operation(s)",
- workspace,
- messages,
- cx,
- )
- })
- }
+ cx.spawn(async move |cx| match task.await {
+ Ok(operation) => panel.update(cx, |panel, _cx| {
+ panel.undo_manager.undo_stack.push_back(operation)
+ }),
+ Err(err) => cx.update(|cx| {
+ Self::show_error(
+ "Failed to redo Project Panel Operation(s)",
+ workspace,
+ err.to_string().into(),
+ cx,
+ );
+
+ Ok(())
+ }),
})
.detach();
}
-
- self.cursor += 1;
- }
-
- fn redo_operation(
- &self,
- operation: &ProjectPanelOperation,
- cx: &mut App,
- ) -> Task<Vec<anyhow::Error>> {
- match operation {
- ProjectPanelOperation::Rename {
- from: old_path,
- to: new_path,
- } => self.rename(old_path, new_path, cx),
- ProjectPanelOperation::Batch(operations) => {
- // Ensure that, when redoing a batch of operations, we do these
- // in the same order as they were passed to the batch, as there
- // might be dependencies between them.
- //
- // For example, imagine the following batch of operations:
- //
- // 1. Create `src/`
- // 2. Create `src/main.rs`
- //
- // If these are not done in order, there's no guarantee that the
- // second one succeeds, as the command to redo the `src/main.rs`
- // file creation might fail unless it also forces the parent
- // directory to be created first.
- let tasks: Vec<_> = operations
- .into_iter()
- .map(|operation| self.redo_operation(operation, cx))
- .collect();
-
- Self::run_sequentially(tasks, cx)
- }
- ProjectPanelOperation::Create { .. } => Task::ready(vec![anyhow!(
- "Redoing create operations is currently not supported."
- )]),
- }
}
pub fn record(&mut self, operation: ProjectPanelOperation) {
- // Recording a new operation while the cursor is not at the end of the
- // undo history should remove all operations from the cursor position to
- // the end instead of inserting an operation in the middle of the undo
- // history.
- if self.cursor < self.history.len() {
- self.history.drain(self.cursor..);
+ // Recording a new operation while there's still operations in the
+ // `redo_stack` should clear all operations from the `redo_stack`, as we
+ // might end up in a situation where the state diverges and the
+ // `redo_stack` operations can no longer be done.
+ if !self.redo_stack.is_empty() {
+ self.redo_stack.clear();
}
- // The `cursor` is only increased in the case where the history's length
- // is not yet at the limit, because when it is, the `cursor` value
- // should already match `limit`.
- if self.history.len() >= self.limit {
- self.history.pop_front();
- self.history.push_back(operation);
- } else {
- self.history.push_back(operation);
- self.cursor += 1;
+ if self.undo_stack.len() >= self.limit {
+ self.undo_stack.pop_front();
}
+
+ self.undo_stack.push_back(operation.inverse());
}
pub fn record_batch(&mut self, operations: impl IntoIterator<Item = ProjectPanelOperation>) {
@@ -190,69 +165,18 @@ impl UndoManager {
self.record(operation);
}
- /// Attempts to revert the provided `operation`, returning a vector of errors
- /// in case there was any failure while reverting the operation.
- ///
- /// For all operations other than [`crate::undo::ProjectPanelOperation::Batch`], a maximum
- /// of one error is returned.
- fn undo_operation(
- &self,
+ /// Attempts to execute the provided operation, returning the inverse of the
+ /// provided `operation` as a result.
+ fn execute_operation(
+ &mut self,
operation: &ProjectPanelOperation,
cx: &mut App,
- ) -> Task<Vec<anyhow::Error>> {
+ ) -> Task<Result<ProjectPanelOperation>> {
match operation {
- ProjectPanelOperation::Create { project_path } => {
- let Some(workspace) = self.workspace.upgrade() else {
- return Task::ready(vec![anyhow!("Failed to obtain workspace.")]);
- };
-
- let result = workspace.update(cx, |workspace, cx| {
- workspace.project().update(cx, |project, cx| {
- let entry_id = project
- .entry_for_path(&project_path, cx)
- .map(|entry| entry.id)
- .ok_or_else(|| anyhow!("No entry for path."))?;
-
- project
- .delete_entry(entry_id, true, cx)
- .ok_or_else(|| anyhow!("Failed to trash entry."))
- })
- });
-
- let task = match result {
- Ok(task) => task,
- Err(err) => return Task::ready(vec![err]),
- };
-
- cx.spawn(async move |_| match task.await {
- Ok(_) => vec![],
- Err(err) => vec![err],
- })
- }
- ProjectPanelOperation::Rename {
- from: old_path,
- to: new_path,
- } => self.rename(new_path, old_path, cx),
- ProjectPanelOperation::Batch(operations) => {
- // When reverting operations in a batch, we reverse the order of
- // operations to handle dependencies between them. For example,
- // if a batch contains the following order of operations:
- //
- // 1. Create `src/`
- // 2. Create `src/main.rs`
- //
- // If we first try to revert the directory creation, it would
- // fail because there's still files inside the directory.
- // Operations are also reverted sequentially in order to avoid
- // this same problem.
- let tasks: Vec<_> = operations
- .into_iter()
- .rev()
- .map(|operation| self.undo_operation(operation, cx))
- .collect();
-
- Self::run_sequentially(tasks, cx)
- }
+ ProjectPanelOperation::Rename { from, to } => self.rename(from, to, cx),
+ ProjectPanelOperation::Trash { project_path } => self.trash(project_path, cx),
+ ProjectPanelOperation::Create { project_path } => self.create(project_path, cx),
+ ProjectPanelOperation::Batch(operations) => self.batch(operations, cx),
}
}
@@ -261,9 +185,9 @@ impl UndoManager {
from: &ProjectPath,
to: &ProjectPath,
cx: &mut App,
- ) -> Task<Vec<anyhow::Error>> {
+ ) -> Task<Result<ProjectPanelOperation>> {
let Some(workspace) = self.workspace.upgrade() else {
- return Task::ready(vec![anyhow!("Failed to obtain workspace.")]);
+ return Task::ready(Err(anyhow!("Failed to obtain workspace.")));
};
let result = workspace.update(cx, |workspace, cx| {
@@ -279,44 +203,118 @@ impl UndoManager {
let task = match result {
Ok(task) => task,
- Err(err) => return Task::ready(vec![err]),
+ Err(err) => return Task::ready(Err(err)),
};
+ let from = from.clone();
+ let to = to.clone();
cx.spawn(async move |_| match task.await {
- Ok(_) => vec![],
- Err(err) => vec![err],
+ Err(err) => Err(err),
+ Ok(_) => Ok(ProjectPanelOperation::Rename {
+ from: to.clone(),
+ to: from.clone(),
+ }),
})
}
- /// Awaits each task in `tasks` sequentially, collecting any errors.
- ///
- /// Sequential execution is important for [`ProjectPanelOperation::Batch`]
- /// operations, where there may be ordering dependencies between tasks. For
- /// example, a directory must be created before files can be placed inside
- /// it.
- fn run_sequentially(
- tasks: Vec<Task<Vec<anyhow::Error>>>,
+ fn create(
+ &self,
+ project_path: &ProjectPath,
cx: &mut App,
- ) -> Task<Vec<anyhow::Error>> {
+ ) -> Task<Result<ProjectPanelOperation>> {
+ let Some(workspace) = self.workspace.upgrade() else {
+ return Task::ready(Err(anyhow!("Failed to obtain workspace.")));
+ };
+
+ let task = workspace.update(cx, |workspace, cx| {
+ workspace.project().update(cx, |project, cx| {
+ // This should not be hardcoded to `false`, as it can genuinely
+ // be a directory and it misses all the nuances and details from
+ // `ProjectPanel::confirm_edit`. However, we expect this to be a
+ // short-lived solution as we add support for restoring trashed
+ // files, at which point we'll no longer need to `Create` new
+ // files, any redoing of a trash operation should be a restore.
+ let is_directory = false;
+ project.create_entry(project_path.clone(), is_directory, cx)
+ })
+ });
+
+ let project_path = project_path.clone();
+ cx.spawn(async move |_| match task.await {
+ Ok(_) => Ok(ProjectPanelOperation::Trash { project_path }),
+ Err(err) => Err(err),
+ })
+ }
+
+ fn trash(
+ &self,
+ project_path: &ProjectPath,
+ cx: &mut App,
+ ) -> Task<Result<ProjectPanelOperation>> {
+ let Some(workspace) = self.workspace.upgrade() else {
+ return Task::ready(Err(anyhow!("Failed to obtain workspace.")));
+ };
+
+ let result = workspace.update(cx, |workspace, cx| {
+ workspace.project().update(cx, |project, cx| {
+ let entry_id = project
+ .entry_for_path(&project_path, cx)
+ .map(|entry| entry.id)
+ .ok_or_else(|| anyhow!("No entry for path."))?;
+
+ project
+ .delete_entry(entry_id, true, cx)
+ .ok_or_else(|| anyhow!("Failed to trash entry."))
+ })
+ });
+
+ let task = match result {
+ Ok(task) => task,
+ Err(err) => return Task::ready(Err(err)),
+ };
+
+ let project_path = project_path.clone();
+ cx.spawn(async move |_| match task.await {
+ // We'll want this to eventually be a `Restore` operation, once
+ // we've added support, in `fs` to track and restore a trashed file.
+ Ok(_) => Ok(ProjectPanelOperation::Create { project_path }),
+ Err(err) => Err(err),
+ })
+ }
+
+ fn batch(
+ &mut self,
+ operations: &[ProjectPanelOperation],
+ cx: &mut App,
+ ) -> Task<Result<ProjectPanelOperation>> {
+ let tasks: Vec<_> = operations
+ .into_iter()
+ .map(|operation| self.execute_operation(operation, cx))
+ .collect();
+
cx.spawn(async move |_| {
- let mut errors = Vec::new();
+ let mut operations = Vec::new();
for task in tasks {
- errors.extend(task.await);
+ match task.await {
+ Ok(operation) => operations.push(operation),
+ Err(err) => return Err(err),
+ }
}
- errors
+ // Return the `ProjectPanelOperation::Batch` that reverses all of
+ // the provided operations. The order of operations should be reversed
+ // so that dependencies are handled correctly.
+ operations.reverse();
+ Ok(ProjectPanelOperation::Batch(operations))
})
}
- /// Displays a notification with the list of provided errors ensuring that,
- /// when more than one error is provided, which can be the case when dealing
- /// with undoing a [`crate::undo::ProjectPanelOperation::Batch`], a list is
- /// displayed with each of the errors, instead of a single message.
- fn show_errors(
+ /// Displays a notification with the provided `title` and `error`.
+ fn show_error(
title: impl Into<SharedString>,
workspace: WeakEntity<Workspace>,
- messages: Vec<SharedString>,
+ error: SharedString,
cx: &mut App,
) {
workspace
@@ -325,23 +323,7 @@ impl UndoManager {
NotificationId::Named(SharedString::new_static("project_panel_undo"));
workspace.show_notification(notification_id, cx, move |cx| {
- cx.new(|cx| {
- if let [err] = messages.as_slice() {
- MessageNotification::new(err.to_string(), cx).with_title(title)
- } else {
- MessageNotification::new_from_builder(cx, move |_, _| {
- v_flex()
- .gap_1()
- .children(
- messages
- .iter()
- .map(|message| Label::new(format!("- {message}"))),
- )
- .into_any_element()
- })
- .with_title(title)
- }
- })
+ cx.new(|cx| MessageNotification::new(error.to_string(), cx).with_title(title))
})
})
.ok();
@@ -354,8 +336,9 @@ pub(crate) mod test {
ProjectPanel, project_panel_tests,
undo::{ProjectPanelOperation, UndoManager},
};
- use gpui::{Entity, TestAppContext, VisualTestContext};
+ use gpui::{Entity, TestAppContext, VisualTestContext, WindowHandle};
use project::{FakeFs, Project, ProjectPath, WorktreeId};
+ use serde_json::{Value, json};
use std::sync::Arc;
use util::rel_path::rel_path;
use workspace::MultiWorkspace;
@@ -363,12 +346,16 @@ pub(crate) mod test {
struct TestContext {
project: Entity<Project>,
panel: Entity<ProjectPanel>,
+ window: WindowHandle<MultiWorkspace>,
}
- async fn init_test(cx: &mut TestAppContext) -> TestContext {
+ async fn init_test(cx: &mut TestAppContext, tree: Option<Value>) -> TestContext {
project_panel_tests::init_test(cx);
let fs = FakeFs::new(cx.executor());
+ if let Some(tree) = tree {
+ fs.insert_tree("/root", tree).await;
+ }
let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await;
let window =
cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
@@ -379,7 +366,11 @@ pub(crate) mod test {
let panel = workspace.update_in(cx, ProjectPanel::new);
cx.run_until_parked();
- TestContext { project, panel }
+ TestContext {
+ project,
+ panel,
+ window,
+ }
}
pub(crate) fn build_create_operation(
@@ -394,6 +385,18 @@ pub(crate) mod test {
}
}
+ pub(crate) fn build_trash_operation(
+ worktree_id: WorktreeId,
+ file_name: &str,
+ ) -> ProjectPanelOperation {
+ ProjectPanelOperation::Trash {
+ project_path: ProjectPath {
+ path: Arc::from(rel_path(file_name)),
+ worktree_id,
+ },
+ }
+ }
+
pub(crate) fn build_rename_operation(
worktree_id: WorktreeId,
from: &str,
@@ -414,9 +417,33 @@ pub(crate) mod test {
}
}
+ async fn rename(
+ panel: &Entity<ProjectPanel>,
+ from: &str,
+ to: &str,
+ cx: &mut VisualTestContext,
+ ) {
+ project_panel_tests::select_path(panel, from, cx);
+ panel.update_in(cx, |panel, window, cx| {
+ panel.rename(&Default::default(), window, cx)
+ });
+ cx.run_until_parked();
+
+ panel
+ .update_in(cx, |panel, window, cx| {
+ panel
+ .filename_editor
+ .update(cx, |editor, cx| editor.set_text(to, window, cx));
+ panel.confirm_edit(true, window, cx).unwrap()
+ })
+ .await
+ .unwrap();
+ cx.run_until_parked();
+ }
+
#[gpui::test]
async fn test_limit(cx: &mut TestAppContext) {
- let test_context = init_test(cx).await;
+ let test_context = init_test(cx, None).await;
let worktree_id = test_context.project.update(cx, |project, cx| {
project.visible_worktrees(cx).next().unwrap().read(cx).id()
});
@@ -430,127 +457,216 @@ pub(crate) mod test {
let operation_c = build_create_operation(worktree_id, "file_c.txt");
let operation_d = build_create_operation(worktree_id, "file_d.txt");
- test_context.panel.update(cx, move |panel, _cx| {
- panel.undo_manager = UndoManager::new_with_limit(panel.workspace.clone(), 3);
+ test_context.panel.update(cx, move |panel, cx| {
+ panel.undo_manager =
+ UndoManager::new_with_limit(panel.workspace.clone(), cx.weak_entity(), 3);
panel.undo_manager.record(operation_a);
panel.undo_manager.record(operation_b);
panel.undo_manager.record(operation_c);
panel.undo_manager.record(operation_d);
- assert_eq!(panel.undo_manager.history.len(), 3);
+ assert_eq!(panel.undo_manager.undo_stack.len(), 3);
});
}
-
#[gpui::test]
- async fn test_cursor(cx: &mut TestAppContext) {
- let test_context = init_test(cx).await;
- let worktree_id = test_context.project.update(cx, |project, cx| {
+ async fn test_undo_redo_stacks(cx: &mut TestAppContext) {
+ let TestContext {
+ window,
+ panel,
+ project,
+ ..
+ } = init_test(
+ cx,
+ Some(json!({
+ "a.txt": "",
+ "b.txt": ""
+ })),
+ )
+ .await;
+ let worktree_id = project.update(cx, |project, cx| {
project.visible_worktrees(cx).next().unwrap().read(cx).id()
});
+ let cx = &mut VisualTestContext::from_window(window.into(), cx);
- test_context.panel.update(cx, |panel, _cx| {
- panel.undo_manager = UndoManager::new_with_limit(panel.workspace.clone(), 3);
- panel
- .undo_manager
- .record(build_create_operation(worktree_id, "file_a.txt"));
-
- assert_eq!(panel.undo_manager.cursor, 1);
+ // Start by renaming `src/file_a.txt` to `src/file_1.txt` and asserting
+ // we get the correct inverse operation in the
+ // `UndoManager::undo_stackand asserting we get the correct inverse
+ // operation in the `UndoManager::undo_stack`.
+ rename(&panel, "root/a.txt", "1.txt", cx).await;
+ panel.update(cx, |panel, _cx| {
+ assert_eq!(
+ panel.undo_manager.undo_stack,
+ vec![build_rename_operation(worktree_id, "1.txt", "a.txt")]
+ );
+ assert!(panel.undo_manager.redo_stack.is_empty());
});
- test_context.panel.update(cx, |panel, cx| {
- panel.undo_manager.undo(cx);
+ // After undoing, the operation to be executed should be popped from
+ // `UndoManager::undo_stack` and its inverse operation pushed to
+ // `UndoManager::redo_stack`.
+ panel.update_in(cx, |panel, window, cx| {
+ panel.undo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- // Ensure that only the `UndoManager::cursor` is updated, as the
- // history should remain unchanged, so we can later redo the
- // operation.
- assert_eq!(panel.undo_manager.cursor, 0);
+ panel.update(cx, |panel, _cx| {
+ assert!(panel.undo_manager.undo_stack.is_empty());
assert_eq!(
- panel.undo_manager.history,
- vec![build_create_operation(worktree_id, "file_a.txt")]
+ panel.undo_manager.redo_stack,
+ vec![build_rename_operation(worktree_id, "a.txt", "1.txt")]
);
+ });
- panel.undo_manager.undo(cx);
+ // Redoing should have the same effect as undoing, but in reverse.
+ panel.update_in(cx, |panel, window, cx| {
+ panel.redo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- // Undoing when cursor is already at `0` should have no effect on
- // both the `cursor` and `history`.
- assert_eq!(panel.undo_manager.cursor, 0);
+ panel.update(cx, |panel, _cx| {
assert_eq!(
- panel.undo_manager.history,
- vec![build_create_operation(worktree_id, "file_a.txt")]
+ panel.undo_manager.undo_stack,
+ vec![build_rename_operation(worktree_id, "1.txt", "a.txt")]
);
+ assert!(panel.undo_manager.redo_stack.is_empty());
+ });
+ }
+
+ #[gpui::test]
+ async fn test_undo_redo_trash(cx: &mut TestAppContext) {
+ let TestContext {
+ window,
+ panel,
+ project,
+ ..
+ } = init_test(
+ cx,
+ Some(json!({
+ "a.txt": "",
+ "b.txt": ""
+ })),
+ )
+ .await;
+ let worktree_id = project.update(cx, |project, cx| {
+ project.visible_worktrees(cx).next().unwrap().read(cx).id()
+ });
+ let cx = &mut VisualTestContext::from_window(window.into(), cx);
+
+ // Start by setting up the `UndoManager::undo_stack` such that, undoing
+ // the last user operation will trash `a.txt`.
+ panel.update(cx, |panel, _cx| {
+ panel
+ .undo_manager
+ .undo_stack
+ .push_back(build_trash_operation(worktree_id, "a.txt"));
});
- test_context.panel.update(cx, |panel, cx| {
- panel.undo_manager.redo(cx);
+ // Undoing should now delete the file and update the
+ // `UndoManager::redo_stack` state with a new `Create` operation.
+ panel.update_in(cx, |panel, window, cx| {
+ panel.undo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- // Ensure that only the `UndoManager::cursor` is updated, since
- // we're only re-doing an operation that was already part of the
- // undo history.
- assert_eq!(panel.undo_manager.cursor, 1);
+ panel.update(cx, |panel, _cx| {
+ assert!(panel.undo_manager.undo_stack.is_empty());
assert_eq!(
- panel.undo_manager.history,
- vec![build_create_operation(worktree_id, "file_a.txt")]
+ panel.undo_manager.redo_stack,
+ vec![build_create_operation(worktree_id, "a.txt")]
);
});
- test_context.panel.update(cx, |panel, _cx| {
- panel
- .undo_manager
- .record(build_create_operation(worktree_id, "file_b.txt"));
- panel
- .undo_manager
- .record(build_create_operation(worktree_id, "file_c.txt"));
+ // Redoing should create the file again and pop the operation from
+ // `UndoManager::redo_stack`.
+ panel.update_in(cx, |panel, window, cx| {
+ panel.redo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- assert_eq!(panel.undo_manager.cursor, panel.undo_manager.limit);
+ panel.update(cx, |panel, _cx| {
+ assert_eq!(
+ panel.undo_manager.undo_stack,
+ vec![build_trash_operation(worktree_id, "a.txt")]
+ );
+ assert!(panel.undo_manager.redo_stack.is_empty());
+ });
+ }
- panel
- .undo_manager
- .record(build_create_operation(worktree_id, "file_d.txt"));
+ #[gpui::test]
+ async fn test_undo_redo_batch(cx: &mut TestAppContext) {
+ let TestContext {
+ window,
+ panel,
+ project,
+ ..
+ } = init_test(
+ cx,
+ Some(json!({
+ "a.txt": "",
+ "b.txt": ""
+ })),
+ )
+ .await;
+ let worktree_id = project.update(cx, |project, cx| {
+ project.visible_worktrees(cx).next().unwrap().read(cx).id()
+ });
+ let cx = &mut VisualTestContext::from_window(window.into(), cx);
+
+ // There's currently no way to trigger two file renames in a single
+ // operation using the `ProjectPanel`. As such, we'll directly record
+ // the batch of operations in `UndoManager`, simulating that `1.txt` and
+ // `2.txt` had been renamed to `a.txt` and `b.txt`, respectively.
+ panel.update(cx, |panel, _cx| {
+ panel.undo_manager.record_batch(vec![
+ build_rename_operation(worktree_id, "1.txt", "a.txt"),
+ build_rename_operation(worktree_id, "2.txt", "b.txt"),
+ ]);
- // Ensure that the operation to create `file_a.txt` has been evicted
- // but the cursor has not grown when that new operation was
- // recorded, as the history was already at its limit.
- assert_eq!(panel.undo_manager.cursor, panel.undo_manager.limit);
assert_eq!(
- panel.undo_manager.history,
- vec![
- build_create_operation(worktree_id, "file_b.txt"),
- build_create_operation(worktree_id, "file_c.txt"),
- build_create_operation(worktree_id, "file_d.txt")
- ]
+ panel.undo_manager.undo_stack,
+ vec![ProjectPanelOperation::Batch(vec![
+ build_rename_operation(worktree_id, "b.txt", "2.txt"),
+ build_rename_operation(worktree_id, "a.txt", "1.txt"),
+ ])]
);
+ assert!(panel.undo_manager.redo_stack.is_empty());
});
- // We'll now undo 2 operations, ensuring that the `cursor` is updated
- // accordingly. Afterwards, we'll record a new operation and verify that
- // the `cursor` is incremented but that all operations from the previous
- // cursor position onwards are discarded.
- test_context.panel.update(cx, |panel, cx| {
- panel.undo_manager.undo(cx);
- panel.undo_manager.undo(cx);
+ panel.update_in(cx, |panel, window, cx| {
+ panel.undo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- assert_eq!(panel.undo_manager.cursor, 1);
+ // Since the operations in the `Batch` are meant to be done in order,
+ // the inverse should have the operations in the opposite order to avoid
+ // dependencies. For example, creating a `src/` folder come before
+ // creating the `src/file_a.txt` file, but when undoing, the file should
+ // be trashed first.
+ panel.update(cx, |panel, _cx| {
+ assert!(panel.undo_manager.undo_stack.is_empty());
assert_eq!(
- panel.undo_manager.history,
- vec![
- build_create_operation(worktree_id, "file_b.txt"),
- build_create_operation(worktree_id, "file_c.txt"),
- build_create_operation(worktree_id, "file_d.txt")
- ]
+ panel.undo_manager.redo_stack,
+ vec![ProjectPanelOperation::Batch(vec![
+ build_rename_operation(worktree_id, "1.txt", "a.txt"),
+ build_rename_operation(worktree_id, "2.txt", "b.txt"),
+ ])]
);
+ });
- panel
- .undo_manager
- .record(build_create_operation(worktree_id, "file_e.txt"));
+ panel.update_in(cx, |panel, window, cx| {
+ panel.redo(&Default::default(), window, cx);
+ });
+ cx.run_until_parked();
- assert_eq!(panel.undo_manager.cursor, 2);
+ panel.update(cx, |panel, _cx| {
assert_eq!(
- panel.undo_manager.history,
- vec![
- build_create_operation(worktree_id, "file_b.txt"),
- build_create_operation(worktree_id, "file_e.txt"),
- ]
+ panel.undo_manager.undo_stack,
+ vec![ProjectPanelOperation::Batch(vec![
+ build_rename_operation(worktree_id, "b.txt", "2.txt"),
+ build_rename_operation(worktree_id, "a.txt", "1.txt"),
+ ])]
);
+ assert!(panel.undo_manager.redo_stack.is_empty());
});
}
}