Merge pull request #13331 from gitbutlerapp/dp-tui-replace-amend-with-reverse-rub

Replace amend with reverse-rub in the TUI

Author: davidpdrsn

crates/but/src/command/legacy/status/tui/details/mod.rs

@@ -201,7 +201,6 @@ impl Details {
             | Message::ShrinkDetails
             | Message::RegisterMessageOnDrop(_)
             | Message::WithOneFrameDelay(_)
-            | Message::Amend
             | Message::EnterNormalMode => false,
 
             Message::MoveCursorUp
@@ -220,7 +219,9 @@ impl Details {
                 | CommitMessage::ToggleEmptyMessage => false,
             },
             Message::Rub(rub_message) => match rub_message {
-                RubMessage::Start | RubMessage::StartWithSource { .. } => false,
+                RubMessage::Start
+                | RubMessage::StartReverse
+                | RubMessage::StartWithSource { .. } => false,
                 RubMessage::Confirm => true,
             },
             Message::Reword(reword_message) => match reword_message {

crates/but/src/command/legacy/status/tui/key_bind.rs

@@ -307,6 +307,15 @@ fn register_normal_mode_key_binds(key_binds: &mut KeyBinds) {
         hide_from_hotbar: false,
     });
 
+    key_binds.register(StaticKeyBind {
+        short_description: "reverse rub",
+        chord_display: "shift+r",
+        key_matcher: press().shift().code(KeyCode::Char('R')),
+        modes: Vec::from([ModeDiscriminant::Normal]),
+        message: Message::Rub(RubMessage::StartReverse),
+        hide_from_hotbar: false,
+    });
+
     key_binds.register(StaticKeyBind {
         short_description: "commit",
         chord_display: "c",
@@ -370,15 +379,6 @@ fn register_normal_mode_key_binds(key_binds: &mut KeyBinds) {
         hide_from_hotbar: false,
     });
 
-    key_binds.register(StaticKeyBind {
-        short_description: "amend",
-        chord_display: "a",
-        key_matcher: press().code(KeyCode::Char('a')),
-        modes: Vec::from([ModeDiscriminant::Normal]),
-        message: Message::Amend,
-        hide_from_hotbar: false,
-    });
-
     key_binds.register(StaticKeyBind {
         short_description: "files",
         chord_display: "f",

crates/but/src/command/legacy/status/tui/mod.rs

@@ -33,10 +33,7 @@ use crate::{
     CliId,
     command::legacy::{
         reword::get_branch_name_from_editor,
-        rub::{
-            RubOperation, RubOperationDiscriminants, StackToCommitOperation,
-            UnassignedToCommitOperation,
-        },
+        rub::RubOperationDiscriminants,
         status::{
             CommitLineContent, FileLineContent, StatusFlags, StatusOutputLine, TuiLaunchOptions,
             output::BranchLineContent,
@@ -604,6 +601,9 @@ impl App {
                 } => {
                     self.handle_start_rub_with_source(source, unlock_details);
                 }
+                RubMessage::StartReverse => {
+                    self.handle_rub_start_reverse(ctx)?;
+                }
                 RubMessage::Confirm => self.handle_confirm_rub(ctx, messages)?,
             },
             Message::EnterNormalMode => {
@@ -727,9 +727,6 @@ impl App {
                     terminal_area,
                 );
             }
-            Message::Amend => {
-                self.handle_amend(ctx)?;
-            }
         }
 
         self.ensure_cursor_visible(visible_height);
@@ -813,6 +810,23 @@ impl App {
         self.handle_start_rub_with_source(RubSource::CliId(Arc::clone(cli_id)), None);
     }
 
+    fn available_targets_for_rub_mode(&self, source: &RubSource) -> Vec<Arc<CliId>> {
+        self.status_lines
+            .iter()
+            .filter_map(|line| line.data.cli_id())
+            .filter(|target| {
+                *source == ***target
+                    || match &source {
+                        RubSource::CliId(source) => rub::route_operation(source, target).is_some(),
+                        RubSource::CommittedHunk(hunk) => {
+                            rub_from_detail_view::route_operation(hunk, target).is_some()
+                        }
+                    }
+            })
+            .cloned()
+            .collect::<Vec<_>>()
+    }
+
     fn handle_start_rub_with_source(
         &mut self,
         source: RubSource,
@@ -827,21 +841,7 @@ impl App {
             RubSource::CommittedHunk(..) => {}
         }
 
-        let available_targets = self
-            .status_lines
-            .iter()
-            .filter_map(|line| line.data.cli_id())
-            .filter(|target| {
-                source == ***target
-                    || match &source {
-                        RubSource::CliId(source) => rub::route_operation(source, target).is_some(),
-                        RubSource::CommittedHunk(hunk) => {
-                            rub_from_detail_view::route_operation(hunk, target).is_some()
-                        }
-                    }
-            })
-            .cloned()
-            .collect::<Vec<_>>();
+        let available_targets = self.available_targets_for_rub_mode(&source);
 
         self.mode = Mode::Rub(RubMode {
             source,
@@ -872,6 +872,61 @@ impl App {
         }
     }
 
+    fn handle_rub_start_reverse(&mut self, ctx: &mut Context) -> anyhow::Result<()> {
+        let Some(selection) = self
+            .cursor
+            .selected_line(&self.status_lines)
+            .and_then(|line| line.data.cli_id())
+        else {
+            return Ok(());
+        };
+
+        let CliId::Commit { commit_id, .. } = &**selection else {
+            return Ok(());
+        };
+
+        let stack_id = {
+            let (_guard, _, ws, _) = ctx.workspace_and_db()?;
+            ws.find_commit_and_containers(*commit_id)
+                .and_then(|(stack, _, _)| stack.id)
+        };
+
+        let source = if let Some(stack_id) = stack_id
+            && operations::stack_has_assigned_changes(ctx, stack_id)?
+            && let Some(id) = self
+                .status_lines
+                .iter()
+                .filter_map(|line| line.data.cli_id())
+                .find_map(|id| {
+                    if let CliId::Stack { id, stack_id: sid } = &**id
+                        && *sid == stack_id
+                    {
+                        Some(id)
+                    } else {
+                        None
+                    }
+                }) {
+            RubSource::CliId(Arc::new(CliId::Stack {
+                id: id.to_owned(),
+                stack_id,
+            }))
+        } else {
+            RubSource::CliId(Arc::new(CliId::Unassigned {
+                id: UNASSIGNED.to_owned(),
+            }))
+        };
+
+        let available_targets = self.available_targets_for_rub_mode(&source);
+
+        self.mode = Mode::Rub(RubMode {
+            source,
+            available_targets,
+            _unlock_details: None,
+        });
+
+        Ok(())
+    }
+
     /// Handles toggling file visibility and requests a status reload.
     fn handle_toggle_global_files_list(&mut self, messages: &mut Vec<Message>) {
         self.flags.show_files = match self.flags.show_files {
@@ -2695,57 +2750,6 @@ impl App {
         let details_viewport = self.details_viewport(terminal_area);
         self.details.ensure_selection_visible(details_viewport);
     }
-
-    fn handle_amend(&mut self, ctx: &mut Context) -> anyhow::Result<()> {
-        let Some(selection) = self
-            .cursor
-            .selected_line(&self.status_lines)
-            .and_then(|line| line.data.cli_id())
-        else {
-            return Ok(());
-        };
-
-        let CliId::Commit { commit_id, .. } = &**selection else {
-            return Ok(());
-        };
-
-        let stack_id = {
-            let (_guard, _, ws, _) = ctx.workspace_and_db()?;
-            ws.find_commit_and_containers(*commit_id)
-                .and_then(|(stack, _, _)| stack.id)
-        };
-
-        let (operation, confirm_message) = if let Some(stack_id) = stack_id
-            && operations::stack_has_assigned_changes(ctx, stack_id)?
-        {
-            (
-                RubOperation::StackToCommit(StackToCommitOperation {
-                    from: stack_id,
-                    to: *commit_id,
-                }),
-                format!(
-                    "Amend changes assigned to stack into {}?",
-                    commit_id.to_hex_with_len(7)
-                ),
-            )
-        } else {
-            (
-                RubOperation::UnassignedToCommit(UnassignedToCommitOperation { oid: *commit_id }),
-                format!("Amend unassigned into {}?", commit_id.to_hex_with_len(7)),
-            )
-        };
-
-        self.confirm = Some(Confirm::new(
-            confirm_message,
-            run_after_confirmation_msg(move |_, ctx, messages| {
-                let what_to_select = operations::rub(ctx, &operation)?;
-                messages.push(Message::Reload(what_to_select));
-                Ok(())
-            }),
-        ));
-
-        Ok(())
-    }
 }
 
 fn event_to_messages(ev: Event, key_binds: &KeyBinds, mode: &Mode, messages: &mut Vec<Message>) {
@@ -2832,7 +2836,6 @@ enum Message {
     EnterDetailsMode,
     LeaveDetailsMode,
     NewBranch,
-    Amend,
 
     // Utilities
     CopySelection,
@@ -2887,6 +2890,7 @@ enum RubMessage {
         source: RubSource,
         unlock_details: Option<MessageOnDrop>,
     },
+    StartReverse,
     Confirm,
 }
 

crates/but/src/command/legacy/status/tui/tests/mod.rs

@@ -564,46 +564,6 @@ fn key_b_creates_new_branch_from_selected_branch() {
         .assert_current_line_eq(str!["┊╭┄br [c-branch-1] (no commits)"]);
 }
 
-#[test]
-fn key_a_amends_selected_commit() {
-    let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();
-    env.setup_metadata(&["A"]).unwrap();
-
-    let mut tui = test_tui(env);
-
-    tui.env().file("test.txt", "content");
-
-    tui.input_then_render(None)
-        .assert_current_line_eq(str!["╭┄zz [unassigned changes]"]);
-
-    tui.input_then_render([KeyCode::Down, KeyCode::Down, KeyCode::Down])
-        .assert_current_line_eq(str!["┊●   [..] add A"]);
-
-    tui.input_then_render('a')
-        .assert_current_line_eq(str!["┊●   [..] add A"])
-        .assert_rendered_contains("Amend unassigned into");
-
-    tui.input_then_render('y')
-        .assert_current_line_eq(str!["┊●   [..] add A"]);
-
-    let status = tui.env().invoke_git("status --porcelain");
-    assert_eq!(status, "", "expected amend to consume all worktree changes");
-}
-
-#[test]
-fn key_a_on_non_commit_selection_is_noop() {
-    let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();
-    env.setup_metadata(&["A"]).unwrap();
-
-    let mut tui = test_tui(env);
-
-    tui.input_then_render(None)
-        .assert_current_line_eq(str!["╭┄zz [unassigned changes] (no changes)"]);
-
-    tui.input_then_render('a')
-        .assert_current_line_eq(str!["╭┄zz [unassigned changes] (no changes)"]);
-}
-
 #[test]
 fn rubbing() {
     let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();

crates/but/src/command/legacy/status/tui/tests/rub_tests.rs

@@ -111,6 +111,71 @@ fn rub_api_cannot_rub_into_branches() {
         ]);
 }
 
+// Tests RubMessage::StartReverse on a commit when unassigned has changes.
+#[test]
+fn rub_api_reverse_rub_uses_unassigned_source_when_unassigned_has_changes() {
+    let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();
+    env.setup_metadata(&["A"]).unwrap();
+
+    let mut tui = test_tui(env);
+
+    tui.env().file("test.txt", "content");
+    tui.env().invoke_git("add test.txt");
+
+    tui.input_then_render(None)
+        .assert_current_line_eq(str!["╭┄zz [unassigned changes]"]);
+
+    tui.input_then_render((KeyModifiers::SHIFT, KeyCode::Char('J')))
+        .assert_current_line_eq(str!["┊╭┄g0 [A]"]);
+
+    tui.input_then_render(KeyCode::Down)
+        .assert_current_line_eq(str!["┊●   [..] add A"]);
+
+    tui.input_then_render((KeyModifiers::SHIFT, KeyCode::Char('R')))
+        .assert_current_line_eq(str!["┊●   << amend >> [..] add A"]);
+
+    tui.input_then_render([KeyCode::Up, KeyCode::Up])
+        .assert_current_line_eq(str!["╭┄<< source >> << noop >> zz [unassigned changes]"]);
+}
+
+// Tests RubMessage::StartReverse with unassigned source when stack has no assigned changes.
+#[test]
+fn rub_api_reverse_rub_uses_unassigned_source_when_stack_has_no_assigned_changes() {
+    let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();
+    env.setup_metadata(&["A"]).unwrap();
+
+    let mut tui = test_tui(env);
+
+    tui.input_then_render(None)
+        .assert_current_line_eq(str!["╭┄zz [unassigned changes] (no changes)"]);
+
+    tui.input_then_render([KeyCode::Down, KeyCode::Down])
+        .assert_current_line_eq(str!["┊●   [..] add A"]);
+
+    tui.input_then_render((KeyModifiers::SHIFT, KeyCode::Char('R')))
+        .assert_current_line_eq(str!["┊●   << amend >> [..] add A"]);
+
+    tui.input_then_render([KeyCode::Up, KeyCode::Up])
+        .assert_current_line_eq(str![
+            "╭┄<< source >> << noop >> zz [unassigned changes] (no changes)"
+        ]);
+}
+
+// Tests RubMessage::StartReverse is a no-op when not selecting a commit.
+#[test]
+fn rub_api_reverse_rub_is_noop_on_non_commit_selection() {
+    let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack").unwrap();
+    env.setup_metadata(&["A"]).unwrap();
+
+    let mut tui = test_tui(env);
+
+    tui.input_then_render(None)
+        .assert_current_line_eq(str!["╭┄zz [unassigned changes] (no changes)"]);
+
+    tui.input_then_render((KeyModifiers::SHIFT, KeyCode::Char('R')))
+        .assert_current_line_eq(str!["╭┄zz [unassigned changes] (no changes)"]);
+}
+
 // Tests RubOperation::UndoCommit.
 #[test]
 fn rub_api_undo_commit_operation() {