Merge pull request #13660 from gitbutlerapp/remove-dead-split-commit

remove unused legacy implementations in but-workspace

Author: krlvi

crates/but-workspace/src/legacy/mod.rs

@@ -25,9 +25,7 @@ pub use stacks::{
 };
 pub use tree_manipulation::{
     MoveChangesResult,
-    remove_changes_from_commit_in_stack::remove_changes_from_commit_in_stack,
     split_branch::{split_branch, split_into_dependent_branch},
-    split_commit::{CommitFiles, CommmitSplitOutcome, split_commit},
 };
 
 /// Various types for the frontend.

crates/but-workspace/src/legacy/tree_manipulation/mod.rs

@@ -11,29 +11,6 @@ pub struct MoveChangesResult {
     pub replaced_commits: Vec<(gix::ObjectId, gix::ObjectId)>,
 }
 
-impl MoveChangesResult {
-    /// Merges the changes from another `MoveChangesResult` into this one.
-    pub fn merge(&mut self, other: MoveChangesResult) {
-        let mut new_replaced_commits = self.replaced_commits.clone();
-
-        for (before, after) in other.replaced_commits {
-            let matching_commit_mapping =
-                new_replaced_commits.iter_mut().find(|(_, a)| *a == before);
-            if let Some(found_matching_mapping) = matching_commit_mapping {
-                // If we found a matching commit mapping, we update the "after" id
-                // to the new "after" id.
-                found_matching_mapping.1 = after;
-            } else {
-                // Otherwise, we add the new mapping.
-                new_replaced_commits.push((before, after));
-            }
-        }
-
-        self.replaced_commits = new_replaced_commits;
-    }
-}
-
 pub(super) mod remove_changes_from_commit_in_stack;
 pub(super) mod split_branch;
-pub(super) mod split_commit;
 mod utils;

crates/but-workspace/src/legacy/tree_manipulation/remove_changes_from_commit_in_stack.rs

@@ -1,77 +1,10 @@
-#![expect(
-    deprecated,
-    reason = "VirtualBranchesHandle should be replaced with ctx.workspace_* helpers"
-)]
-
 use anyhow::{Result, bail};
-use but_core::{DiffSpec, TreeChange, ref_metadata::StackId, sync::RepoExclusive};
+use but_core::{DiffSpec, TreeChange, sync::RepoExclusive};
 use but_ctx::Context;
-use but_rebase::{Rebase, replace_commit_tree};
-use gitbutler_stack::VirtualBranchesHandle;
+use but_rebase::replace_commit_tree;
 use gix::ObjectId;
 
-use super::MoveChangesResult;
-use crate::legacy::{
-    stack_ext::StackExt,
-    tree_manipulation::utils::{
-        ChangesSource, create_tree_without_diff, rebase_mapping_with_overrides,
-        replace_pick_with_commit,
-    },
-};
-
-/// Removes the specified changes from a given commit.
-///
-/// This only updates the specified stack. After calling you may want to call
-/// `update_workspace_commit` such that the workspace commit now contains the
-/// updated head of the stack.
-///
-/// You may want to make use of `update_uncommitted_changes`. Using it will
-/// cause the specified change to be dropped from the working directory. Not
-/// using it will result in the change showing up as an uncommitted change.
-///
-/// ## Assumptions
-///
-/// Currently this function does not take into consideration the possibility
-/// that the commit _might_ be part of two different stacks. As such, the
-/// other stacks may end up referring to stale commits and potentially cause
-/// a merge conflict when combining them in the workspace.
-pub fn remove_changes_from_commit_in_stack(
-    ctx: &mut Context,
-    source_stack_id: StackId,
-    source_commit_id: gix::ObjectId,
-    changes: impl IntoIterator<Item = DiffSpec>,
-    perm: &mut RepoExclusive,
-) -> Result<MoveChangesResult> {
-    let vb_state = VirtualBranchesHandle::new(ctx.project_data_dir());
-    let source_stack = vb_state.get_stack(source_stack_id)?;
-
-    let rewritten_source_commit = remove_changes_from_commit(ctx, source_commit_id, changes, perm)?;
-
-    let mut steps = source_stack.as_rebase_steps(ctx)?;
-    replace_pick_with_commit(&mut steps, source_commit_id, rewritten_source_commit)?;
-    let base = source_stack.merge_base(ctx)?;
-
-    let result = {
-        let repo = ctx.repo.get()?;
-        let mut rebase = Rebase::new(&repo, base, None)?;
-        rebase.steps(steps)?;
-        rebase.rebase_noops(false);
-        rebase.rebase()?
-    };
-    let commit_mapping =
-        rebase_mapping_with_overrides(&result, [(source_commit_id, rewritten_source_commit)]);
-
-    let mut source_stack = source_stack;
-    source_stack.set_heads_from_rebase_output(ctx, result.references)?;
-
-    let meta = ctx.meta()?;
-    let (repo, mut ws, _) = ctx.workspace_mut_and_db_with_perm(perm)?;
-    ws.refresh_from_head(&repo, &meta)?;
-
-    Ok(MoveChangesResult {
-        replaced_commits: commit_mapping.into_iter().collect(),
-    })
-}
+use crate::legacy::tree_manipulation::utils::{ChangesSource, create_tree_without_diff};
 
 /// Removes the specified changes from a commit.
 ///

crates/but-workspace/src/legacy/tree_manipulation/split_commit.rs

@@ -1,105 +1,4 @@
 //! Utility types related to discarding changes in the worktree.
 
-use std::collections::HashMap;
-
-use but_rebase::{RebaseOutput, RebaseStep};
-
 // Re-export from non-legacy location for backward compatibility
 pub use crate::tree_manipulation::{ChangesSource, create_tree_without_diff};
-
-/// Takes a rebase output and returns the commit mapping with any extra
-/// mapping overrides provided.
-///
-/// This will only include commits that have actually changed. If a commit was
-/// mapped to itself it will not be included in the resulting HashMap.
-///
-/// Overrides are used to handle the case where the caller of the rebase engine
-/// has manually replaced a particular commit with a rewritten one. This is
-/// needed because a manually re-written commit that ends up matching the
-/// base when the rebase occurs will end up showing up as a no-op in the
-/// resulting commit_mapping.
-///
-/// Overrides should be provided as a vector that contains tuples of object
-/// ids, where the first item is the before object_id, and the second item is
-/// the after object_id.
-pub(crate) fn rebase_mapping_with_overrides(
-    rebase_output: &RebaseOutput,
-    overrides: impl IntoIterator<Item = (gix::ObjectId, gix::ObjectId)>,
-) -> HashMap<gix::ObjectId, gix::ObjectId> {
-    let mut mapping = rebase_output
-        .commit_mapping
-        .iter()
-        .filter(|(_, old, new)| old != new)
-        .map(|(_, old, new)| (*old, *new))
-        .collect::<HashMap<_, _>>();
-
-    for (old, new) in overrides {
-        if old != new {
-            mapping.insert(old, new);
-        }
-    }
-
-    mapping
-}
-
-pub fn replace_pick_with_commit(
-    steps: &mut Vec<RebaseStep>,
-    target_commit_id: gix::ObjectId,
-    replacement_commit_id: gix::ObjectId,
-) -> anyhow::Result<()> {
-    let mut found = false;
-    for step in steps {
-        if step.commit_id() != Some(&target_commit_id) {
-            continue;
-        }
-        let RebaseStep::Pick { commit_id, .. } = step else {
-            continue;
-        };
-        found = true;
-        *commit_id = replacement_commit_id;
-    }
-
-    if found {
-        Ok(())
-    } else {
-        Err(anyhow::anyhow!(
-            "Failed to replace pick step {target_commit_id} with {replacement_commit_id}"
-        ))
-    }
-}
-
-pub fn replace_pick_with_multiple_commits(
-    steps: &mut Vec<RebaseStep>,
-    target_commit_id: gix::ObjectId,
-    replacement_commit_ids: &[(gix::ObjectId, Option<String>)],
-) -> anyhow::Result<()> {
-    let mut found = false;
-    let mut new_steps =
-        Vec::with_capacity(steps.len() + replacement_commit_ids.len().saturating_sub(1));
-    for step in steps.drain(..) {
-        if step.commit_id() == Some(&target_commit_id) {
-            let RebaseStep::Pick { .. } = step else {
-                new_steps.push(step);
-                continue;
-            };
-            found = true;
-            for (replacement_commit_id, new_message) in replacement_commit_ids {
-                new_steps.push(RebaseStep::Pick {
-                    commit_id: *replacement_commit_id,
-                    new_message: new_message.clone().map(|msg| msg.into()),
-                });
-            }
-        } else {
-            new_steps.push(step);
-        }
-    }
-    *steps = new_steps;
-
-    if found {
-        Ok(())
-    } else {
-        Err(anyhow::anyhow!(
-            "Failed to replace pick step {target_commit_id} with multiple commits"
-        ))
-    }
-}