From 11b6eac9585f0be95635da98e6bc0acf94b19831 Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Thu, 15 Aug 2024 16:51:27 +0100 Subject: [PATCH] no-op: Refactor movement utilities into cli/src/movement_utils.rs The code in both cli/src/commands/{next,prev}.rs is identical except for the direction of movement. This no-op PR pull the parts that make sense out into cli/src/movement_util.rs so it's easier to see the differences. Part of #3947 --- cli/src/commands/next.rs | 92 +++------------------ cli/src/commands/prev.rs | 55 +++---------- cli/src/lib.rs | 1 + cli/src/movement_util.rs | 170 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 122 deletions(-) create mode 100644 cli/src/movement_util.rs diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs index 6a78a790b6..b77d294dcf 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -12,15 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::io::Write; - -use itertools::Itertools; -use jj_lib::commit::Commit; -use jj_lib::repo::Repo; -use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; - -use crate::cli_util::{short_commit_hash, CommandHelper, WorkspaceCommandHelper}; +use crate::cli_util::{short_commit_hash, CommandHelper}; use crate::command_error::{user_error, CommandError}; +use crate::movement_util::{get_target_commit, Direction}; use crate::ui::Ui; /// Move the working-copy commit to the child revision @@ -70,38 +64,6 @@ pub(crate) struct NextArgs { conflict: bool, } -pub fn choose_commit<'a>( - ui: &mut Ui, - workspace_command: &WorkspaceCommandHelper, - cmd: &str, - commits: &'a [Commit], -) -> Result<&'a Commit, CommandError> { - writeln!(ui.stdout(), "ambiguous {cmd} commit, choose one to target:")?; - let mut formatter = ui.stdout_formatter(); - let template = workspace_command.commit_summary_template(); - let mut choices: Vec = Default::default(); - for (i, commit) in commits.iter().enumerate() { - write!(formatter, "{}: ", i + 1)?; - template.format(commit, formatter.as_mut())?; - writeln!(formatter)?; - choices.push(format!("{}", i + 1)); - } - writeln!(formatter, "q: quit the prompt")?; - choices.push("q".to_string()); - drop(formatter); - - let choice = ui.prompt_choice( - "enter the index of the commit you want to target", - &choices, - None, - )?; - if choice == "q" { - return Err(user_error("ambiguous target commit")); - } - - Ok(&commits[choice.parse::().unwrap() - 1]) -} - pub(crate) fn cmd_next( ui: &mut Ui, command: &CommandHelper, @@ -117,44 +79,16 @@ pub(crate) fn cmd_next( .view() .heads() .contains(current_wc_id); - let wc_revset = RevsetExpression::commit(current_wc_id.clone()); - // If we're editing, start at the working-copy commit. Otherwise, start from - // its direct parent(s). - let start_revset = if edit { - wc_revset.clone() - } else { - wc_revset.parents() - }; - - let target_revset = if args.conflict { - start_revset - .children() - .descendants() - .filtered(RevsetFilterPredicate::HasConflict) - .roots() - } else { - start_revset.descendants_at(args.offset) - } - .minus(&wc_revset); - - let targets: Vec = target_revset - .evaluate_programmatic(workspace_command.repo().as_ref())? - .iter() - .commits(workspace_command.repo().store()) - .try_collect()?; + let target = get_target_commit( + ui, + &workspace_command, + Direction::Next, + current_wc_id, + edit, + args.conflict, + args.offset, + )?; - let target = match targets.as_slice() { - [target] => target, - [] => { - // We found no descendant. - return Err(user_error(format!( - "No descendant found {} commit{} forward", - args.offset, - if args.offset > 1 { "s" } else { "" } - ))); - } - commits => choose_commit(ui, &workspace_command, "next", commits)?, - }; let current_short = short_commit_hash(current_wc_id); let target_short = short_commit_hash(target.id()); // We're editing, just move to the target commit. @@ -162,7 +96,7 @@ pub(crate) fn cmd_next( // We're editing, the target must be rewritable. workspace_command.check_rewritable([target.id()])?; let mut tx = workspace_command.start_transaction(); - tx.edit(target)?; + tx.edit(&target)?; tx.finish( ui, format!("next: {current_short} -> editing {target_short}"), @@ -171,7 +105,7 @@ pub(crate) fn cmd_next( } let mut tx = workspace_command.start_transaction(); // Move the working-copy commit to the new parent. - tx.check_out(target)?; + tx.check_out(&target)?; tx.finish(ui, format!("next: {current_short} -> {target_short}"))?; Ok(()) } diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index 659f116e6c..266478186c 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -12,13 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use itertools::Itertools; -use jj_lib::repo::Repo; -use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; - use crate::cli_util::{short_commit_hash, CommandHelper}; use crate::command_error::{user_error, CommandError}; -use crate::commands::next::choose_commit; +use crate::movement_util::{get_target_commit, Direction}; use crate::ui::Ui; /// Change the working copy revision relative to the parent revision /// @@ -79,42 +75,15 @@ pub(crate) fn cmd_prev( .view() .heads() .contains(current_wc_id); - let wc_revset = RevsetExpression::commit(current_wc_id.clone()); - // If we're editing, start at the working-copy commit. Otherwise, start from - // its direct parent(s). - let start_revset = if edit { - wc_revset.clone() - } else { - wc_revset.parents() - }; - - let target_revset = if args.conflict { - // If people desire to move to the root conflict, replace the `heads()` below - // with `roots(). But let's wait for feedback. - start_revset - .parents() - .ancestors() - .filtered(RevsetFilterPredicate::HasConflict) - .heads() - } else { - start_revset.ancestors_at(args.offset) - }; - let targets: Vec<_> = target_revset - .evaluate_programmatic(workspace_command.repo().as_ref())? - .iter() - .commits(workspace_command.repo().store()) - .try_collect()?; - let target = match targets.as_slice() { - [target] => target, - [] => { - return Err(user_error(format!( - "No ancestor found {} commit{} back", - args.offset, - if args.offset > 1 { "s" } else { "" } - ))) - } - commits => choose_commit(ui, &workspace_command, "prev", commits)?, - }; + let target = get_target_commit( + ui, + &workspace_command, + Direction::Prev, + current_wc_id, + edit, + args.conflict, + args.offset, + )?; // Generate a short commit hash, to make it readable in the op log. let current_short = short_commit_hash(current_wc_id); @@ -124,7 +93,7 @@ pub(crate) fn cmd_prev( // The target must be rewritable if we're editing. workspace_command.check_rewritable([target.id()])?; let mut tx = workspace_command.start_transaction(); - tx.edit(target)?; + tx.edit(&target)?; tx.finish( ui, format!("prev: {current_short} -> editing {target_short}"), @@ -132,7 +101,7 @@ pub(crate) fn cmd_prev( return Ok(()); } let mut tx = workspace_command.start_transaction(); - tx.check_out(target)?; + tx.check_out(&target)?; tx.finish(ui, format!("prev: {current_short} -> {target_short}"))?; Ok(()) } diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 6e60b293a7..20004e3ff5 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -27,6 +27,7 @@ pub mod generic_templater; pub mod git_util; pub mod graphlog; pub mod merge_tools; +pub mod movement_util; pub mod operation_templater; mod progress; pub mod revset_util; diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs new file mode 100644 index 0000000000..a61def344a --- /dev/null +++ b/cli/src/movement_util.rs @@ -0,0 +1,170 @@ +// Copyright 2020 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::io::Write; +use std::rc::Rc; + +use itertools::Itertools; +use jj_lib::backend::CommitId; +use jj_lib::commit::Commit; +use jj_lib::repo::Repo; +use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; + +use crate::cli_util::WorkspaceCommandHelper; +use crate::command_error::{user_error, CommandError}; +use crate::ui::Ui; + +pub enum Direction { + Next, + Prev, +} + +impl Direction { + fn cmd(&self) -> String { + match self { + Direction::Next => "next".to_string(), + Direction::Prev => "prev".to_string(), + } + } + fn direction(&self) -> String { + match self { + Direction::Next => "forward".to_string(), + Direction::Prev => "back".to_string(), + } + } + + fn next_node_type(&self) -> String { + match self { + Direction::Next => "descendant".to_string(), + Direction::Prev => "ancestor".to_string(), + } + } + + fn get_target_revset( + &self, + working_commit_id: &CommitId, + edit: bool, + has_conflict: bool, + change_offset: u64, + ) -> Result, CommandError> { + let wc_revset = RevsetExpression::commit(working_commit_id.clone()); + // If we're editing, start at the working-copy commit. Otherwise, start from + // its direct parent(s). + let start_revset = if edit { + wc_revset.clone() + } else { + wc_revset.parents() + }; + + let target_revset = match self { + Direction::Next => if has_conflict { + start_revset + .children() + .descendants() + .filtered(RevsetFilterPredicate::HasConflict) + .roots() + } else { + start_revset.descendants_at(change_offset) + } + .minus(&wc_revset), + + Direction::Prev => { + if has_conflict { + // If people desire to move to the root conflict, replace the `heads()` below + // with `roots(). But let's wait for feedback. + start_revset + .parents() + .ancestors() + .filtered(RevsetFilterPredicate::HasConflict) + .heads() + } else { + start_revset.ancestors_at(change_offset) + } + } + }; + + Ok(target_revset) + } +} + +pub fn get_target_commit( + ui: &mut Ui, + workspace_command: &WorkspaceCommandHelper, + direction: Direction, + working_commit_id: &CommitId, + edit: bool, + has_conflict: bool, + change_offset: u64, +) -> Result { + let target_revset = + direction.get_target_revset(working_commit_id, edit, has_conflict, change_offset)?; + let targets: Vec = target_revset + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + + let target = match targets.as_slice() { + [target] => target, + [] => { + // We found no descendant. + return Err(user_error(format!( + "No {} found {} commit{} {}", + direction.next_node_type(), + change_offset, + if change_offset > 1 { "s" } else { "" }, + direction.direction(), + ))); + } + commits => choose_commit(ui, &workspace_command, &direction, commits)?, + }; + + Ok(target.clone()) +} + +pub fn choose_commit<'a>( + ui: &mut Ui, + workspace_command: &WorkspaceCommandHelper, + direction: &Direction, + commits: &'a [Commit], +) -> Result<&'a Commit, CommandError> { + writeln!( + ui.stdout(), + "ambiguous {} commit, choose one to target:", + direction.cmd() + )?; + let mut formatter = ui.stdout_formatter(); + let template = workspace_command.commit_summary_template(); + let mut choices: Vec = Default::default(); + for (i, commit) in commits.iter().enumerate() { + write!(formatter, "{}: ", i + 1)?; + template.format(commit, formatter.as_mut())?; + writeln!(formatter)?; + choices.push(format!("{}", i + 1)); + } + writeln!(formatter, "q: quit the prompt")?; + choices.push("q".to_string()); + drop(formatter); + + let choice = ui.prompt_choice( + "enter the index of the commit you want to target", + &choices, + None, + )?; + if choice == "q" { + return Err(user_error("ambiguous target commit")); + } + + Ok(&commits[choice.parse::().unwrap() - 1]) +}