From 24bbae0de06c81bd16d7cc79d159df784809bf68 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 21 Jun 2024 16:08:12 +0900 Subject: [PATCH 1/4] view: remove has_branch() which is called only from tests Since we've split (local, remotes) branches to (locals, remotes { branches }), .has_branch() API no longer makes much sense. Callers often need to check if a remote branch is tracked. --- lib/src/repo.rs | 6 ------ lib/src/view.rs | 11 ----------- lib/tests/test_git.rs | 33 ++++++++++++++++++++++++--------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 1382dc9224..7a742b6590 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -1428,12 +1428,6 @@ impl MutableRepo { self.view.mark_dirty(); } - /// Returns true if any local or remote branch of the given `name` exists. - #[must_use] - pub fn has_branch(&self, name: &str) -> bool { - self.view.with_ref(|v| v.has_branch(name)) - } - pub fn remove_branch(&mut self, name: &str) { self.view_mut().remove_branch(name); } diff --git a/lib/src/view.rs b/lib/src/view.rs index 1125bd0b2b..8ce37f5299 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -96,17 +96,6 @@ impl View { self.data.head_ids.remove(head_id); } - /// Returns true if any local or remote branch of the given `name` exists. - #[must_use] - pub fn has_branch(&self, name: &str) -> bool { - self.data.local_branches.contains_key(name) - || self - .data - .remote_views - .values() - .any(|remote_view| remote_view.branches.contains_key(name)) - } - // TODO: maybe rename to forget_branch() because this seems unusual operation? pub fn remove_branch(&mut self, name: &str) { self.data.local_branches.remove(name); diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 3f8a98a8cb..5ae15842f0 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -534,7 +534,7 @@ fn test_import_refs_reimport_with_deleted_remote_ref() { state: RemoteRefState::Tracking, }, ); - assert!(view.has_branch("main")); // branch #3 of 3 + assert!(view.get_local_branch("main").is_present()); // branch #3 of 3 // Simulate fetching from a remote where feature-remote-only and // feature-remote-and-local branches were deleted. This leads to the @@ -550,8 +550,11 @@ fn test_import_refs_reimport_with_deleted_remote_ref() { let view = repo.view(); // The local branches were indeed deleted assert_eq!(view.branches().count(), 2); - assert!(view.has_branch("main")); - assert!(!view.has_branch("feature-remote-only")); + assert!(view.get_local_branch("main").is_present()); + assert!(view.get_local_branch("feature-remote-only").is_absent()); + assert!(view + .get_remote_branch("feature-remote-only", "origin") + .is_absent()); assert!(view .get_local_branch("feature-remote-and-local") .is_absent()); @@ -653,7 +656,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() { state: RemoteRefState::Tracking, }, ); - assert!(view.has_branch("main")); // branch #3 of 3 + assert!(view.get_local_branch("main").is_present()); // branch #3 of 3 // Simulate fetching from a remote where feature-remote-only and // feature-remote-and-local branches were moved. This leads to the @@ -711,7 +714,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() { state: RemoteRefState::Tracking, }, ); - assert!(view.has_branch("main")); // branch #3 of 3 + assert!(view.get_local_branch("main").is_present()); // branch #3 of 3 let expected_heads = hashset! { jj_id(&commit_main), jj_id(&new_commit_remote_and_local), @@ -1185,9 +1188,13 @@ fn test_import_some_refs() { view.get_remote_branch("feature4", "origin"), &commit_feat4_remote_ref ); - assert!(!view.has_branch("main")); + assert!(view.get_local_branch("main").is_absent()); + assert!(view.get_remote_branch("main", "git").is_absent()); + assert!(view.get_remote_branch("main", "origin").is_absent()); assert!(!view.heads().contains(&jj_id(&commit_main))); - assert!(!view.has_branch("ignored")); + assert!(view.get_local_branch("ignored").is_absent()); + assert!(view.get_remote_branch("ignored", "git").is_absent()); + assert!(view.get_remote_branch("ignored", "origin").is_absent()); assert!(!view.heads().contains(&jj_id(&commit_ign))); // Delete branch feature1, feature3 and feature4 in git repository and import @@ -2375,7 +2382,11 @@ fn test_fetch_prune_deleted_ref() { ) .unwrap(); // Test the setup - assert!(tx.mut_repo().has_branch("main")); + assert!(tx.mut_repo().get_local_branch("main").is_present()); + assert!(tx + .mut_repo() + .get_remote_branch("main", "origin") + .is_present()); test_data .origin_repo @@ -2394,7 +2405,11 @@ fn test_fetch_prune_deleted_ref() { ) .unwrap(); assert_eq!(stats.import_stats.abandoned_commits, vec![jj_id(&commit)]); - assert!(!tx.mut_repo().has_branch("main")); + assert!(tx.mut_repo().get_local_branch("main").is_absent()); + assert!(tx + .mut_repo() + .get_remote_branch("main", "origin") + .is_absent()); } #[test] From 0801a5665814971e030ddfe2466d2567405a4ebb Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 24 Jun 2024 16:34:23 +0900 Subject: [PATCH 2/4] cli: branch: inline make_branch_term(), use singular form It's used only in transaction descriptions, and I think singular form works at adjective position. --- cli/src/commands/branch/create.rs | 7 +++---- cli/src/commands/branch/delete.rs | 4 ++-- cli/src/commands/branch/forget.rs | 4 ++-- cli/src/commands/branch/mod.rs | 9 --------- cli/src/commands/branch/move.rs | 8 ++++---- cli/src/commands/branch/rename.rs | 10 +--------- cli/src/commands/branch/set.rs | 8 ++++---- cli/src/commands/branch/track.rs | 9 +++++++-- cli/src/commands/branch/untrack.rs | 8 ++++++-- 9 files changed, 29 insertions(+), 38 deletions(-) diff --git a/cli/src/commands/branch/create.rs b/cli/src/commands/branch/create.rs index 42cd8ee955..9c6cbe7157 100644 --- a/cli/src/commands/branch/create.rs +++ b/cli/src/commands/branch/create.rs @@ -16,7 +16,6 @@ use clap::builder::NonEmptyStringValueParser; use jj_lib::object_id::ObjectId as _; use jj_lib::op_store::RefTarget; -use super::make_branch_term; use crate::cli_util::{CommandHelper, RevisionArg}; use crate::command_error::{user_error_with_hint, CommandError}; use crate::ui::Ui; @@ -69,9 +68,9 @@ pub fn cmd_branch_create( tx.finish( ui, format!( - "create {} pointing to commit {}", - make_branch_term(branch_names), - target_commit.id().hex() + "create branch {names} pointing to commit {id}", + names = branch_names.join(", "), + id = target_commit.id().hex() ), )?; Ok(()) diff --git a/cli/src/commands/branch/delete.rs b/cli/src/commands/branch/delete.rs index 2664c9519a..3e8afed456 100644 --- a/cli/src/commands/branch/delete.rs +++ b/cli/src/commands/branch/delete.rs @@ -15,7 +15,7 @@ use jj_lib::op_store::RefTarget; use jj_lib::str_util::StringPattern; -use super::{find_local_branches, make_branch_term}; +use super::find_local_branches; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; use crate::ui::Ui; @@ -46,7 +46,7 @@ pub fn cmd_branch_delete( tx.mut_repo() .set_local_branch_target(branch_name, RefTarget::absent()); } - tx.finish(ui, format!("delete {}", make_branch_term(&names)))?; + tx.finish(ui, format!("delete branch {}", names.join(", ")))?; if names.len() > 1 { writeln!(ui.status(), "Deleted {} branches.", names.len())?; } diff --git a/cli/src/commands/branch/forget.rs b/cli/src/commands/branch/forget.rs index b87cffc6a2..c6a264d1eb 100644 --- a/cli/src/commands/branch/forget.rs +++ b/cli/src/commands/branch/forget.rs @@ -15,7 +15,7 @@ use jj_lib::str_util::StringPattern; use jj_lib::view::View; -use super::{find_branches_with, make_branch_term}; +use super::find_branches_with; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; use crate::ui::Ui; @@ -48,7 +48,7 @@ pub fn cmd_branch_forget( for branch_name in names.iter() { tx.mut_repo().remove_branch(branch_name); } - tx.finish(ui, format!("forget {}", make_branch_term(&names)))?; + tx.finish(ui, format!("forget branch {}", names.join(", ")))?; if names.len() > 1 { writeln!(ui.status(), "Forgot {} branches.", names.len())?; } diff --git a/cli/src/commands/branch/mod.rs b/cli/src/commands/branch/mod.rs index dfd6a29f3b..b61444def5 100644 --- a/cli/src/commands/branch/mod.rs +++ b/cli/src/commands/branch/mod.rs @@ -22,8 +22,6 @@ mod set; mod track; mod untrack; -use std::fmt; - use itertools::Itertools as _; use jj_lib::backend::CommitId; use jj_lib::op_store::{RefTarget, RemoteRef}; @@ -87,13 +85,6 @@ pub fn cmd_branch( } } -fn make_branch_term(branch_names: &[impl fmt::Display]) -> String { - match branch_names { - [branch_name] => format!("branch {}", branch_name), - branch_names => format!("branches {}", branch_names.iter().join(", ")), - } -} - fn find_local_branches( view: &View, name_patterns: &[StringPattern], diff --git a/cli/src/commands/branch/move.rs b/cli/src/commands/branch/move.rs index e0cd00348c..8fc9c5c9ad 100644 --- a/cli/src/commands/branch/move.rs +++ b/cli/src/commands/branch/move.rs @@ -17,7 +17,7 @@ use jj_lib::object_id::ObjectId as _; use jj_lib::op_store::RefTarget; use jj_lib::str_util::StringPattern; -use super::{find_branches_with, is_fast_forward, make_branch_term}; +use super::{find_branches_with, is_fast_forward}; use crate::cli_util::{CommandHelper, RevisionArg}; use crate::command_error::{user_error_with_hint, CommandError}; use crate::ui::Ui; @@ -125,9 +125,9 @@ pub fn cmd_branch_move( tx.finish( ui, format!( - "point {} to commit {}", - make_branch_term(&branch_names), - target_commit.id().hex() + "point branch {names} to commit {id}", + names = branch_names.join(", "), + id = target_commit.id().hex() ), )?; diff --git a/cli/src/commands/branch/rename.rs b/cli/src/commands/branch/rename.rs index b2eb66ab21..a589778575 100644 --- a/cli/src/commands/branch/rename.rs +++ b/cli/src/commands/branch/rename.rs @@ -15,7 +15,6 @@ use jj_lib::op_store::RefTarget; use jj_lib::str_util::StringPattern; -use super::make_branch_term; use crate::cli_util::CommandHelper; use crate::command_error::{user_error, CommandError}; use crate::ui::Ui; @@ -55,14 +54,7 @@ pub fn cmd_branch_rename( .set_local_branch_target(new_branch, ref_target); tx.mut_repo() .set_local_branch_target(old_branch, RefTarget::absent()); - tx.finish( - ui, - format!( - "rename {} to {}", - make_branch_term(&[old_branch]), - make_branch_term(&[new_branch]), - ), - )?; + tx.finish(ui, format!("rename branch {old_branch} to {new_branch}"))?; let view = workspace_command.repo().view(); if view diff --git a/cli/src/commands/branch/set.rs b/cli/src/commands/branch/set.rs index 3757fc8106..2dfbf4d942 100644 --- a/cli/src/commands/branch/set.rs +++ b/cli/src/commands/branch/set.rs @@ -16,7 +16,7 @@ use clap::builder::NonEmptyStringValueParser; use jj_lib::object_id::ObjectId as _; use jj_lib::op_store::RefTarget; -use super::{is_fast_forward, make_branch_term}; +use super::is_fast_forward; use crate::cli_util::{CommandHelper, RevisionArg}; use crate::command_error::{user_error_with_hint, CommandError}; use crate::ui::Ui; @@ -77,9 +77,9 @@ pub fn cmd_branch_set( tx.finish( ui, format!( - "point {} to commit {}", - make_branch_term(branch_names), - target_commit.id().hex() + "point branch {names} to commit {id}", + names = branch_names.join(", "), + id = target_commit.id().hex() ), )?; diff --git a/cli/src/commands/branch/track.rs b/cli/src/commands/branch/track.rs index 3e0a4b77d8..5dab3a29d0 100644 --- a/cli/src/commands/branch/track.rs +++ b/cli/src/commands/branch/track.rs @@ -14,7 +14,9 @@ use std::collections::HashMap; -use super::{find_remote_branches, make_branch_term}; +use itertools::Itertools as _; + +use super::find_remote_branches; use crate::cli_util::{CommandHelper, RemoteBranchNamePattern}; use crate::command_error::CommandError; use crate::commit_templater::{CommitTemplateLanguage, RefName}; @@ -61,7 +63,10 @@ pub fn cmd_branch_track( tx.mut_repo() .track_remote_branch(&name.branch, &name.remote); } - tx.finish(ui, format!("track remote {}", make_branch_term(&names)))?; + tx.finish( + ui, + format!("track remote branch {}", names.iter().join(", ")), + )?; if names.len() > 1 { writeln!( ui.status(), diff --git a/cli/src/commands/branch/untrack.rs b/cli/src/commands/branch/untrack.rs index 44f5fe0fd8..498ecfe29b 100644 --- a/cli/src/commands/branch/untrack.rs +++ b/cli/src/commands/branch/untrack.rs @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; use jj_lib::git; -use super::{find_remote_branches, make_branch_term}; +use super::find_remote_branches; use crate::cli_util::{CommandHelper, RemoteBranchNamePattern}; use crate::command_error::CommandError; use crate::ui::Ui; @@ -65,7 +66,10 @@ pub fn cmd_branch_untrack( tx.mut_repo() .untrack_remote_branch(&name.branch, &name.remote); } - tx.finish(ui, format!("untrack remote {}", make_branch_term(&names)))?; + tx.finish( + ui, + format!("untrack remote branch {}", names.iter().join(", ")), + )?; if names.len() > 1 { writeln!( ui.status(), From 98715574a20bd0a182fa6e3c55eb4962ac9b9607 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 24 Jun 2024 16:44:42 +0900 Subject: [PATCH 3/4] cli: branch: let find_*_branches() return (name, target) pairs This will help inline view.remove_branch() in cmd_branch_forget(). I don't care much about owned (String, _) vs (&str, _), but we can't simplify the lifetime issue in find_forgettable_branches() anyway. So I made all callers pass cloned Arc and borrow (name, target) pairs from there. --- cli/src/commands/branch/delete.rs | 21 +++++++++++++------- cli/src/commands/branch/forget.rs | 32 ++++++++++++++++++------------- cli/src/commands/branch/mod.rs | 27 +++++++++++++------------- cli/src/commands/branch/move.rs | 28 +++++++++++++-------------- 4 files changed, 60 insertions(+), 48 deletions(-) diff --git a/cli/src/commands/branch/delete.rs b/cli/src/commands/branch/delete.rs index 3e8afed456..c9d5700766 100644 --- a/cli/src/commands/branch/delete.rs +++ b/cli/src/commands/branch/delete.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; use jj_lib::op_store::RefTarget; use jj_lib::str_util::StringPattern; @@ -39,16 +40,22 @@ pub fn cmd_branch_delete( args: &BranchDeleteArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let view = workspace_command.repo().view(); - let names = find_local_branches(view, &args.names)?; + let repo = workspace_command.repo().clone(); + let matched_branches = find_local_branches(repo.view(), &args.names)?; let mut tx = workspace_command.start_transaction(); - for branch_name in names.iter() { + for (name, _) in &matched_branches { tx.mut_repo() - .set_local_branch_target(branch_name, RefTarget::absent()); + .set_local_branch_target(name, RefTarget::absent()); } - tx.finish(ui, format!("delete branch {}", names.join(", ")))?; - if names.len() > 1 { - writeln!(ui.status(), "Deleted {} branches.", names.len())?; + tx.finish( + ui, + format!( + "delete branch {}", + matched_branches.iter().map(|(name, _)| name).join(", ") + ), + )?; + if matched_branches.len() > 1 { + writeln!(ui.status(), "Deleted {} branches.", matched_branches.len())?; } Ok(()) } diff --git a/cli/src/commands/branch/forget.rs b/cli/src/commands/branch/forget.rs index c6a264d1eb..4e778a2fae 100644 --- a/cli/src/commands/branch/forget.rs +++ b/cli/src/commands/branch/forget.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; +use jj_lib::op_store::BranchTarget; use jj_lib::str_util::StringPattern; use jj_lib::view::View; @@ -42,26 +44,30 @@ pub fn cmd_branch_forget( args: &BranchForgetArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let view = workspace_command.repo().view(); - let names = find_forgettable_branches(view, &args.names)?; + let repo = workspace_command.repo().clone(); + let matched_branches = find_forgettable_branches(repo.view(), &args.names)?; let mut tx = workspace_command.start_transaction(); - for branch_name in names.iter() { - tx.mut_repo().remove_branch(branch_name); + for (name, _) in &matched_branches { + tx.mut_repo().remove_branch(name); } - tx.finish(ui, format!("forget branch {}", names.join(", ")))?; - if names.len() > 1 { - writeln!(ui.status(), "Forgot {} branches.", names.len())?; + tx.finish( + ui, + format!( + "forget branch {}", + matched_branches.iter().map(|(name, _)| name).join(", ") + ), + )?; + if matched_branches.len() > 1 { + writeln!(ui.status(), "Forgot {} branches.", matched_branches.len())?; } Ok(()) } -fn find_forgettable_branches( - view: &View, +fn find_forgettable_branches<'a>( + view: &'a View, name_patterns: &[StringPattern], -) -> Result, CommandError> { +) -> Result)>, CommandError> { find_branches_with(name_patterns, |pattern| { - view.branches() - .filter(|(name, _)| pattern.matches(name)) - .map(|(name, _)| name.to_owned()) + view.branches().filter(|(name, _)| pattern.matches(name)) }) } diff --git a/cli/src/commands/branch/mod.rs b/cli/src/commands/branch/mod.rs index b61444def5..71e3d7d71b 100644 --- a/cli/src/commands/branch/mod.rs +++ b/cli/src/commands/branch/mod.rs @@ -85,33 +85,32 @@ pub fn cmd_branch( } } -fn find_local_branches( - view: &View, +fn find_local_branches<'a>( + view: &'a View, name_patterns: &[StringPattern], -) -> Result, CommandError> { +) -> Result, CommandError> { find_branches_with(name_patterns, |pattern| { view.local_branches_matching(pattern) - .map(|(name, _)| name.to_owned()) }) } -fn find_branches_with<'a, I: Iterator>( - name_patterns: &'a [StringPattern], - mut find_matches: impl FnMut(&'a StringPattern) -> I, -) -> Result, CommandError> { - let mut matching_branches: Vec = vec![]; +fn find_branches_with<'a, 'b, V, I: Iterator>( + name_patterns: &'b [StringPattern], + mut find_matches: impl FnMut(&'b StringPattern) -> I, +) -> Result, CommandError> { + let mut matching_branches: Vec = vec![]; let mut unmatched_patterns = vec![]; for pattern in name_patterns { - let mut names = find_matches(pattern).peekable(); - if names.peek().is_none() { + let mut matches = find_matches(pattern).peekable(); + if matches.peek().is_none() { unmatched_patterns.push(pattern); } - matching_branches.extend(names); + matching_branches.extend(matches); } match &unmatched_patterns[..] { [] => { - matching_branches.sort_unstable(); - matching_branches.dedup(); + matching_branches.sort_unstable_by_key(|(name, _)| *name); + matching_branches.dedup_by_key(|(name, _)| *name); Ok(matching_branches) } [pattern] if pattern.is_exact() => Err(user_error(format!("No such branch: {pattern}"))), diff --git a/cli/src/commands/branch/move.rs b/cli/src/commands/branch/move.rs index 8fc9c5c9ad..ac5b414212 100644 --- a/cli/src/commands/branch/move.rs +++ b/cli/src/commands/branch/move.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; use jj_lib::backend::CommitId; use jj_lib::object_id::ObjectId as _; use jj_lib::op_store::RefTarget; @@ -63,10 +64,9 @@ pub fn cmd_branch_move( args: &BranchMoveArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let repo = workspace_command.repo().as_ref(); - let view = repo.view(); + let repo = workspace_command.repo().clone(); - let branch_names = { + let matched_branches = { let is_source_commit = if !args.from.is_empty() { workspace_command .parse_union_revsets(&args.from)? @@ -77,28 +77,28 @@ pub fn cmd_branch_move( }; if !args.names.is_empty() { find_branches_with(&args.names, |pattern| { - view.local_branches_matching(pattern) + repo.view() + .local_branches_matching(pattern) .filter(|(_, target)| target.added_ids().any(&is_source_commit)) - .map(|(name, _)| name.to_owned()) })? } else { - view.local_branches() + repo.view() + .local_branches() .filter(|(_, target)| target.added_ids().any(&is_source_commit)) - .map(|(name, _)| name.to_owned()) .collect() } }; let target_commit = workspace_command.resolve_single_rev(&args.to)?; - if branch_names.is_empty() { + if matched_branches.is_empty() { writeln!(ui.status(), "No branches to update.")?; return Ok(()); } - if branch_names.len() > 1 { + if matched_branches.len() > 1 { writeln!( ui.warning_default(), "Updating multiple branches: {}", - branch_names.join(", "), + matched_branches.iter().map(|(name, _)| name).join(", "), )?; if args.names.is_empty() { writeln!(ui.hint_default(), "Specify branch by name to update one.")?; @@ -106,9 +106,9 @@ pub fn cmd_branch_move( } if !args.allow_backwards { - if let Some(name) = branch_names + if let Some((name, _)) = matched_branches .iter() - .find(|name| !is_fast_forward(repo, view.get_local_branch(name), target_commit.id())) + .find(|(_, old_target)| !is_fast_forward(repo.as_ref(), old_target, target_commit.id())) { return Err(user_error_with_hint( format!("Refusing to move branch backwards or sideways: {name}"), @@ -118,7 +118,7 @@ pub fn cmd_branch_move( } let mut tx = workspace_command.start_transaction(); - for name in &branch_names { + for (name, _) in &matched_branches { tx.mut_repo() .set_local_branch_target(name, RefTarget::normal(target_commit.id().clone())); } @@ -126,7 +126,7 @@ pub fn cmd_branch_move( ui, format!( "point branch {names} to commit {id}", - names = branch_names.join(", "), + names = matched_branches.iter().map(|(name, _)| name).join(", "), id = target_commit.id().hex() ), )?; From a3fc12f8b2a6eaa1cbeec2eaaee77a5cdaa5e991 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 24 Jun 2024 17:02:42 +0900 Subject: [PATCH 4/4] cli: branch: inline view.remove_branch() in cmd_branch_forget() This API no longer makes sense, and we'll probably add some flags to forget only tracked remotes for example. --- cli/src/commands/branch/forget.rs | 11 ++++++++--- lib/src/repo.rs | 4 ---- lib/src/view.rs | 8 -------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/cli/src/commands/branch/forget.rs b/cli/src/commands/branch/forget.rs index 4e778a2fae..a0b3b05a56 100644 --- a/cli/src/commands/branch/forget.rs +++ b/cli/src/commands/branch/forget.rs @@ -13,7 +13,7 @@ // limitations under the License. use itertools::Itertools as _; -use jj_lib::op_store::BranchTarget; +use jj_lib::op_store::{BranchTarget, RefTarget, RemoteRef}; use jj_lib::str_util::StringPattern; use jj_lib::view::View; @@ -47,8 +47,13 @@ pub fn cmd_branch_forget( let repo = workspace_command.repo().clone(); let matched_branches = find_forgettable_branches(repo.view(), &args.names)?; let mut tx = workspace_command.start_transaction(); - for (name, _) in &matched_branches { - tx.mut_repo().remove_branch(name); + for (name, branch_target) in &matched_branches { + tx.mut_repo() + .set_local_branch_target(name, RefTarget::absent()); + for (remote_name, _) in &branch_target.remote_refs { + tx.mut_repo() + .set_remote_branch(name, remote_name, RemoteRef::absent()); + } } tx.finish( ui, diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 7a742b6590..e90158284d 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -1428,10 +1428,6 @@ impl MutableRepo { self.view.mark_dirty(); } - pub fn remove_branch(&mut self, name: &str) { - self.view_mut().remove_branch(name); - } - pub fn get_local_branch(&self, name: &str) -> RefTarget { self.view.with_ref(|v| v.get_local_branch(name).clone()) } diff --git a/lib/src/view.rs b/lib/src/view.rs index 8ce37f5299..4906de0aa0 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -96,14 +96,6 @@ impl View { self.data.head_ids.remove(head_id); } - // TODO: maybe rename to forget_branch() because this seems unusual operation? - pub fn remove_branch(&mut self, name: &str) { - self.data.local_branches.remove(name); - for remote_view in self.data.remote_views.values_mut() { - remote_view.branches.remove(name); - } - } - /// Iterates local branch `(name, target)`s in lexicographical order. pub fn local_branches(&self) -> impl Iterator { self.data