Rework cherry_pick_no_working_copy_range()

Rework cherry_pick_no_working_copy_range() to use terms root_oid and
leaf_oid to be clearer and make it so that root_oid is exclusive while
leaf_oid is inclusive.

<!-- ps-id: 1eac4222-7412-4c82-af5e-d15b73ade7c3 -->

src/ps/mod.rs

@@ -156,6 +156,8 @@ pub enum AddPsIdError {
     ReferenceNameMissing,
     CommitMessageMissing,
     FailedToFindCommonAncestor(git::CommonAncestorError),
+    FailedToFindCommit(git2::Error),
+    FailedToFindParentCommit(git2::Error),
 }
 
 impl From<git2::Error> for AddPsIdError {
@@ -208,11 +210,17 @@ pub fn add_ps_id(
         .ok_or(AddPsIdError::FailedToGetReferenceName)?;
 
     // cherry pick
+    let commit = repo
+        .find_commit(commit_oid)
+        .map_err(AddPsIdError::FailedToFindCommit)?;
+    let parent_commit = commit
+        .parent(0)
+        .map_err(AddPsIdError::FailedToFindParentCommit)?;
     git::cherry_pick_no_working_copy_range(
-        &repo,
+        repo,
         config,
-        commit_oid,
         upstream_branch_oid,
+        parent_commit.id(),
         add_id_rework_branch_ref_name,
     )?;
 
@@ -225,25 +233,18 @@ pub fn add_ps_id(
         message_amendment.as_str(),
     )?;
 
-    if cur_branch_oid != commit_oid {
-        git::cherry_pick_no_working_copy_range(
-            &repo,
-            config,
-            cur_branch_oid,
-            commit_oid,
-            add_id_rework_branch_ref_name,
-        )?;
-        let cherry_picked_commit_oid = git::cherry_pick_no_working_copy(
-            &repo,
-            config,
-            cur_branch_oid,
-            add_id_rework_branch_ref_name,
-            0,
-        )?;
-        branch_ref.set_target(cherry_picked_commit_oid, "swap branch to add_id_rework")?;
-    } else {
-        branch_ref.set_target(amended_patch_oid, "swap branch to add_id_rework")?;
-    }
+    let cherry_picked_commit_oid = git::cherry_pick_no_working_copy_range(
+        repo,
+        config,
+        commit_oid,
+        cur_branch_oid,
+        add_id_rework_branch_ref_name,
+    )?;
+
+    match cherry_picked_commit_oid {
+        Some(oid) => branch_ref.set_target(oid, "swap branch to add_id_rework")?,
+        None => branch_ref.set_target(amended_patch_oid, "swap branch to add_id_rework")?,
+    };
 
     // delete temporary branch
     let mut tmp_branch_ref = repo.find_reference(add_id_rework_branch_ref_name)?;

src/ps/private/git.rs

@@ -118,42 +118,54 @@ pub fn get_current_branch_shorthand(repo: &git2::Repository) -> Option<String> {
     }
 }
 
-// Cherry pick the range of commits onto the destination.
-//
-// Note: The range of commits is bounded by the `start` and `end` Oids. It is
-// important to recoginze that commits referenced by `start` and `end` are
-// both excluded from the actually cherry picking as they just define the
-// commits surrounding the range of the commits to actually cherry-pick. It is
-// also important to understand that `start` should be the most recent commit
-// in history and the `end` should be the least recent commit in the range.
-// Think you are starting at the top of the tree going down.
+/// Cherry pick the specified range of commits onto the destination ref
+///
+/// The given `repo` is the repository that you want to cherry pick the range of commits within.
+/// The `config` is the config used to facilitate commit creation, providing things like the
+/// author, email, etc.
+///
+/// The `root_oid` specifies the commit to start the ranged cherry picking process from,
+/// exclusively. Meaning this commit won't be included in the cherry picked commits, but its
+/// descendants will be, up to and including the `leaf_oid`. This commit should be an ancestor to
+/// the `leaf_oid`.
+///
+/// The `leaf_oid` specifies the commit to end the ranged cherry picking process on, inclusively.
+/// Meaning this commit will be included in the cherry picked commits. This commit should be a
+/// descendant to the `root_oid`.
+///
+/// The `dest_ref_name` specifies the reference (e.g. branch) to cherry pick the range of commits
+/// into.
+///
+/// It returns an Ok(Option(last_cherry_picked_commit_oid)) result in the case of success and an
+/// error result of GitError in the case of failure.
 pub fn cherry_pick_no_working_copy_range<'a>(
     repo: &'a git2::Repository,
     config: &git2::Config,
-    start: git2::Oid,
-    end: git2::Oid,
+    root_oid: git2::Oid,
+    leaf_oid: git2::Oid,
     dest_ref_name: &str,
-) -> Result<(), GitError> {
+) -> Result<Option<git2::Oid>, GitError> {
     let mut rev_walk = repo.revwalk()?;
-    rev_walk.set_sorting(git2::Sort::REVERSE)?;
-    rev_walk.push(start)?;
-    rev_walk.hide(end)?;
-
-    for rev in rev_walk {
-        if let Ok(r) = rev {
-            if r == start {
-                return Ok(()); // effectively short-circuit doing nothing for this last patch
-            }
+    rev_walk.push(leaf_oid)?; // start traversal from leaf_oid and walk to root_oid
+    rev_walk.hide(root_oid)?; // mark root_oid as where to hide from
+    rev_walk.set_sorting(git2::Sort::REVERSE)?; // reverse traversal order so we walk from child
+                                                // commit of the commit identified by root_oid and
+                                                // then iterate our way to the the commit
+                                                // identified by the leaf_oid
 
-            repo.find_commit(r)
-                .map_err(|e| GitError::GitError(e))
-                .and_then(|commit| {
-                    cherry_pick_no_working_copy(repo, config, commit.id(), dest_ref_name, 0)
-                })?;
-        }
+    let mut last_cherry_picked_oid: Option<git2::Oid> = None;
+
+    for rev in rev_walk.flatten() {
+        last_cherry_picked_oid = Some(cherry_pick_no_working_copy(
+            repo,
+            config,
+            rev,
+            dest_ref_name,
+            0,
+        )?);
     }
 
-    return Ok(());
+    Ok(last_cherry_picked_oid)
 }
 
 #[derive(Debug)]

src/ps/public/branch.rs

@@ -79,13 +79,11 @@ pub fn branch(
         git::cherry_pick_no_working_copy_range(
             &repo,
             &config,
-            end_patch_oid,
             start_patch_parent_oid,
+            end_patch_oid,
             branch_ref_name,
         )
         .map_err(BranchError::CherryPickFailed)?;
-        git::cherry_pick_no_working_copy(&repo, &config, end_patch_oid, branch_ref_name, 0)
-            .map_err(BranchError::CherryPickFailed)?;
 
         // push branch up to remote branch
         if create_remote_branch {