Merge pull request #13313 from gitbutlerapp/davids-rescue-mission

A grab bag of TUI UX changes

Author: davidpdrsn

crates/but/src/command/legacy/rub/mod.rs

@@ -99,6 +99,14 @@ pub(crate) struct StackToBranchOperation<'a> {
     pub(crate) to: &'a str,
 }
 
+/// Represents squashing all assignments from a stack to a commit.
+#[derive(Debug)]
+pub(crate) struct StackToCommitOperation {
+    /// The source stack id.
+    pub(crate) from: StackId,
+    pub(crate) to: gix::ObjectId,
+}
+
 /// Represents amending all unassigned hunks into a commit.
 #[derive(Debug)]
 pub(crate) struct UnassignedToCommitOperation {
@@ -122,11 +130,20 @@ pub(crate) struct UnassignedToStackOperation {
 
 /// Represents undoing a commit.
 #[derive(Debug)]
-pub(crate) struct UndoCommitOperation {
+pub(crate) struct CommitToUnassignedOperation {
     /// The commit id to undo.
     pub(crate) oid: gix::ObjectId,
 }
 
+/// Represents undoing a commit to a stack.
+#[derive(Debug)]
+pub(crate) struct CommitToStackOperation {
+    /// The commit id to undo.
+    pub(crate) oid: gix::ObjectId,
+    /// The stack to assign the changes to.
+    pub(crate) stack: StackId,
+}
+
 /// Represents squashing one commit into another.
 #[derive(Debug)]
 pub(crate) struct SquashCommitsOperation {
@@ -221,10 +238,12 @@ pub(crate) enum RubOperation<'a> {
     StackToUnassigned(StackToUnassignedOperation),
     StackToStack(StackToStackOperation),
     StackToBranch(StackToBranchOperation<'a>),
+    StackToCommit(StackToCommitOperation),
     UnassignedToCommit(UnassignedToCommitOperation),
     UnassignedToBranch(UnassignedToBranchOperation<'a>),
     UnassignedToStack(UnassignedToStackOperation),
-    UndoCommit(UndoCommitOperation),
+    CommitToUnassigned(CommitToUnassignedOperation),
+    CommitToStack(CommitToStackOperation),
     SquashCommits(SquashCommitsOperation),
     MoveCommitToBranch(MoveCommitToBranchOperation<'a>),
     BranchToUnassigned(BranchToUnassignedOperation<'a>),
@@ -425,6 +444,44 @@ impl<'a> StackToBranchOperation<'a> {
     }
 }
 
+impl StackToCommitOperation {
+    /// Executes this operation.
+    pub(crate) fn execute(self, ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Result<()> {
+        let result = self.execute_inner(ctx)?;
+        if let Some(out) = out.for_human() {
+            let repo = ctx.repo.get()?;
+            let new_commit = result
+                .new_commit
+                .map(|c| {
+                    let short = shorten_object_id(&repo, c);
+                    let (lead, rest) = split_short_id(&short, 2);
+                    format!("{}{}", lead.blue().bold(), rest.blue())
+                })
+                .unwrap_or_default();
+            writeln!(
+                out,
+                "Amended files assigned to {} → {}",
+                stack_id_to_branch_name(ctx, self.from)
+                    .map(|b| format!("[{b}]").green())
+                    .unwrap_or_else(|| "stack".to_string().bold()),
+                new_commit,
+            )?;
+        } else if let Some(out) = out.for_json() {
+            out.write_value(serde_json::json!({
+                "ok": true,
+                "new_commit_id": result.new_commit.map(|c| c.to_string()),
+            }))?;
+        }
+        Ok(())
+    }
+
+    /// Executes `StackToCommit` by squashing all hunks from the source stack to the target commit.
+    pub(crate) fn execute_inner(&self, ctx: &mut Context) -> anyhow::Result<CommitCreateResult> {
+        let changes = changes_for_stack_assignment(ctx, Some(self.from))?;
+        but_api::commit::amend::commit_amend(ctx, self.to, changes, DryRun::No)
+    }
+}
+
 impl UnassignedToCommitOperation {
     /// Executes this operation.
     pub(crate) fn execute(self, ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Result<()> {
@@ -505,7 +562,7 @@ impl UnassignedToStackOperation {
     }
 }
 
-impl UndoCommitOperation {
+impl CommitToUnassignedOperation {
     /// Executes this operation.
     pub(crate) fn execute(self, ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Result<()> {
         self.execute_inner(ctx)?;
@@ -524,11 +581,36 @@ impl UndoCommitOperation {
 
     /// Executes `UndoCommit` by uncommitting all changes from the selected commit.
     pub(crate) fn execute_inner(&self, ctx: &mut Context) -> anyhow::Result<CommitUndoResult> {
-        // TODO(David): Have fun. - Love, Caleb
         but_api::commit::undo::commit_undo(ctx, self.oid, None, DryRun::No)
     }
 }
 
+impl CommitToStackOperation {
+    /// Executes this operation.
+    pub(crate) fn execute(self, ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Result<()> {
+        self.execute_inner(ctx)?;
+        if let Some(out) = out.for_human() {
+            let repo = ctx.repo.get()?;
+            writeln!(
+                out,
+                "Uncommitted {} to {}",
+                shorten_object_id(&repo, self.oid).blue(),
+                stack_id_to_branch_name(ctx, self.stack)
+                    .map(|b| format!("[{b}]").green())
+                    .unwrap_or_else(|| "stack".bold()),
+            )?;
+        } else if let Some(out) = out.for_json() {
+            out.write_value(serde_json::json!({"ok": true}))?;
+        }
+        Ok(())
+    }
+
+    /// Executes `UndoCommit` by uncommitting all changes from the selected commit.
+    pub(crate) fn execute_inner(&self, ctx: &mut Context) -> anyhow::Result<CommitUndoResult> {
+        but_api::commit::undo::commit_undo(ctx, self.oid, Some(self.stack), DryRun::No)
+    }
+}
+
 impl SquashCommitsOperation {
     /// Executes this operation.
     pub(crate) fn execute(self, ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Result<()> {
@@ -795,7 +877,8 @@ impl<'a> RubOperation<'a> {
             RubOperation::UnassignedToCommit(operation) => operation.execute(ctx, out),
             RubOperation::UnassignedToBranch(operation) => operation.execute(ctx, out),
             RubOperation::UnassignedToStack(operation) => operation.execute(ctx, out),
-            RubOperation::UndoCommit(operation) => operation.execute(ctx, out),
+            RubOperation::CommitToUnassigned(operation) => operation.execute(ctx, out),
+            RubOperation::CommitToStack(operation) => operation.execute(ctx, out),
             RubOperation::SquashCommits(operation) => operation.execute(ctx, out),
             RubOperation::MoveCommitToBranch(operation) => operation.execute(ctx, out),
             RubOperation::BranchToUnassigned(operation) => operation.execute(ctx, out),
@@ -805,6 +888,7 @@ impl<'a> RubOperation<'a> {
             RubOperation::CommittedFileToBranch(operation) => operation.execute(ctx, out),
             RubOperation::CommittedFileToCommit(operation) => operation.execute(ctx, out),
             RubOperation::CommittedFileToUnassigned(operation) => operation.execute(ctx, out),
+            RubOperation::StackToCommit(operation) => operation.execute(ctx, out),
         }
     }
 }
@@ -951,6 +1035,12 @@ pub(crate) fn route_operation<'a>(
                 to,
             }))
         }
+        (Stack { stack_id, .. }, Commit { commit_id, .. }) => {
+            Some(RubOperation::StackToCommit(StackToCommitOperation {
+                from: *stack_id,
+                to: *commit_id,
+            }))
+        }
         // Unassigned -> *
         (Unassigned { .. }, Commit { commit_id, .. }) => Some(RubOperation::UnassignedToCommit(
             UnassignedToCommitOperation { oid: *commit_id },
@@ -962,11 +1052,9 @@ pub(crate) fn route_operation<'a>(
             UnassignedToStackOperation { to: *stack_id },
         )),
         // Commit -> *
-        (Commit { commit_id, .. }, Unassigned { .. }) => {
-            Some(RubOperation::UndoCommit(UndoCommitOperation {
-                oid: *commit_id,
-            }))
-        }
+        (Commit { commit_id, .. }, Unassigned { .. }) => Some(RubOperation::CommitToUnassigned(
+            CommitToUnassignedOperation { oid: *commit_id },
+        )),
         (
             Commit {
                 commit_id: source, ..
@@ -985,6 +1073,12 @@ pub(crate) fn route_operation<'a>(
                 name,
             },
         )),
+        (Commit { commit_id, .. }, Stack { stack_id, .. }) => {
+            Some(RubOperation::CommitToStack(CommitToStackOperation {
+                oid: *commit_id,
+                stack: *stack_id,
+            }))
+        }
         // Branch -> *
         (Branch { name, .. }, Unassigned { .. }) => Some(RubOperation::BranchToUnassigned(
             BranchToUnassignedOperation { from: name },
@@ -1748,12 +1842,12 @@ mod tests {
         // Valid: Commit -> Branch
         assert!(route_operation(&commit, &branch_id()).is_some());
 
+        // Valid: Commit -> Stack
+        assert!(route_operation(&commit, &stack_id()).is_some());
+
         // Invalid: Commit -> Uncommitted
         assert!(route_operation(&commit, &uncommitted_id()).is_none());
 
-        // Invalid: Commit -> Stack
-        assert!(route_operation(&commit, &stack_id()).is_none());
-
         // Invalid: Commit -> CommittedFile
         assert!(route_operation(&commit, &committed_file_id()).is_none());
     }
@@ -1794,12 +1888,12 @@ mod tests {
         // Valid: Stack -> Branch
         assert!(route_operation(&stack, &branch_id()).is_some());
 
+        // Valid: Stack -> Commit
+        assert!(route_operation(&stack, &commit_id()).is_some());
+
         // Invalid: Stack -> Uncommitted
         assert!(route_operation(&stack, &uncommitted_id()).is_none());
 
-        // Invalid: Stack -> Commit
-        assert!(route_operation(&stack, &commit_id()).is_none());
-
         // Invalid: Stack -> CommittedFile
         assert!(route_operation(&stack, &committed_file_id()).is_none());
     }
@@ -1882,10 +1976,16 @@ mod tests {
             _ => panic!("Expected SquashCommits variant"),
         }
 
-        // Commit -> Unassigned should be UndoCommit
+        // Commit -> Unassigned should be CommitToUnassigned
         match route_operation(&commit, &unassigned) {
-            Some(RubOperation::UndoCommit(..)) => {}
-            _ => panic!("Expected UndoCommit variant"),
+            Some(RubOperation::CommitToUnassigned(..)) => {}
+            _ => panic!("Expected CommitToUnassigned variant"),
+        }
+
+        // Commit -> Stack should be CommitToStack
+        match route_operation(&commit, &stack) {
+            Some(RubOperation::CommitToStack(..)) => {}
+            _ => panic!("Expected CommitToStack variant"),
         }
 
         // Branch -> Stack should be BranchToStack
@@ -1900,6 +2000,12 @@ mod tests {
             _ => panic!("Expected StackToBranch variant"),
         }
 
+        // Stack -> Commit should be StackToCommit
+        match route_operation(&stack, &commit) {
+            Some(RubOperation::StackToCommit(..)) => {}
+            _ => panic!("Expected StackToCommit variant"),
+        }
+
         // CommittedFile -> Commit should be CommittedFileToCommit
         match route_operation(&committed_file, &commit) {
             Some(RubOperation::CommittedFileToCommit(..)) => {}

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

@@ -5,6 +5,7 @@
 
 use std::collections::BTreeMap;
 
+use anyhow::Context as _;
 use assignment::FileAssignment;
 use bstr::{BStr, BString, ByteSlice};
 use but_api::diff::ComputeLineStats;
@@ -66,6 +67,16 @@ impl StatusFlags {
             hint: false,
         }
     }
+
+    pub fn for_tui() -> Self {
+        Self {
+            show_files: FilesStatusFlag::None,
+            verbose: false,
+            refresh_prs: false,
+            show_upstream: false,
+            hint: false,
+        }
+    }
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -708,28 +719,25 @@ fn print_assignments(
     unstaged: bool,
     output: &mut StatusOutput<'_>,
 ) -> anyhow::Result<()> {
-    // if there are no assignments and we're in the unstaged section, print "(no changes)" and return
-    if assignments.is_empty() && unstaged {
-        output.no_assignments_unstaged(
-            Vec::from([Span::raw("┊     ")]),
-            Vec::from([Span::styled("no changes", Style::default().dim().italic())]),
-        )?;
-        return Ok(());
-    }
-
     let id = stack
         .and_then(|s| status_ctx.id_map.resolve_stack(s))
         .map(|s| Span::styled(s.to_short_string(), Style::default().bold().blue()))
         .unwrap_or_default();
 
-    if !unstaged && !assignments.is_empty() {
-        let staged_changes_cli_id = stack
-            .and_then(|stack_id| status_ctx.id_map.resolve_stack(stack_id).cloned())
-            .ok_or_else(|| anyhow::anyhow!("Could not resolve stack CLI id for staged changes"))?;
+    if let Some(stack) = stack
+        && (!unstaged && !assignments.is_empty())
+    {
+        let staged_changes_cli_id = status_ctx
+            .id_map
+            .resolve_stack(stack)
+            .cloned()
+            .with_context(|| {
+                format!("Could not resolve stack CLI id for staged changes. stack_id={stack:?}")
+            })?;
 
         output.staged_changes(
             Vec::from([Span::raw("┊  ╭┄")]),
-            Vec::from([
+            [
                 id,
                 Span::raw(" ["),
                 Span::styled(
@@ -740,7 +748,21 @@ fn print_assignments(
                     Style::default().cyan().bold(),
                 ),
                 Span::raw("]"),
-            ]),
+            ]
+            .into_iter()
+            .chain(
+                assignments
+                    .is_empty()
+                    .then(|| {
+                        [
+                            Span::raw(" "),
+                            Span::styled("(no changes)", Style::default().dim().italic()),
+                        ]
+                    })
+                    .into_iter()
+                    .flatten(),
+            )
+            .collect(),
             staged_changes_cli_id,
         )?;
     }
@@ -1015,15 +1037,22 @@ fn print_group(
                 Style::default().bold().blue(),
             ),
             Span::raw(" ["),
-            Span::styled("unstaged changes", Style::default().bold().cyan()),
+            Span::styled("unassigned changes", Style::default().bold().cyan()),
             Span::raw("]"),
         ]);
+        if assignments.is_empty() {
+            line.extend([
+                Span::raw(" "),
+                Span::styled("(no changes)", Style::default().dim().italic()),
+            ]);
+        }
         if let Some(stack_mark) = stack_mark {
-            line.push(Span::raw(" "));
-            line.push(stack_mark.clone());
+            line.extend([Span::raw(" "), stack_mark.clone()]);
         }
         output.unstaged_changes(Vec::from([Span::raw("╭┄")]), line, cli_id.clone())?;
-        print_assignments(&repo, status_ctx, None, None, assignments, true, output)?;
+        if !assignments.is_empty() {
+            print_assignments(&repo, status_ctx, None, None, assignments, true, output)?;
+        }
     }
     if !first {
         output.connector(Vec::from([Span::raw("├╯")]))?;

crates/but/src/command/legacy/status/output.rs

@@ -210,6 +210,7 @@ impl StatusOutput<'_> {
         )
     }
 
+    #[expect(dead_code)]
     pub(super) fn no_assignments_unstaged(
         &mut self,
         connector: Vec<Span<'static>>,
@@ -299,7 +300,7 @@ pub(super) struct StatusOutputLine {
     ///
     /// Example:
     ///
-    /// ╭┄zz [unstaged changes]                                         | Some("╭┄")
+    /// ╭┄zz [unassigned changes]                                       | Some("╭┄")
     /// ┊   ur M flake.nix                                              | Some("┊   ")
     /// ┊                                                               | Some("┊ ")
     /// ┊╭┄dp [dp-branch-4]                                             | Some("┊╭┄")