clean up remainder of old branch API, with minor behavior changes in CLI output

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(())

cli/src/commands/branch/delete.rs

@@ -12,10 +12,11 @@
 // 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;
 
-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;
@@ -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 {}", make_branch_term(&names)))?;
-    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(())
 }

cli/src/commands/branch/forget.rs

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use itertools::Itertools as _;
+use jj_lib::op_store::{BranchTarget, RefTarget, RemoteRef};
 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;
@@ -42,26 +44,35 @@ 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, 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, format!("forget {}", make_branch_term(&names)))?;
-    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<Vec<String>, CommandError> {
+) -> Result<Vec<(&'a str, BranchTarget<'a>)>, 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))
     })
 }

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,40 +85,32 @@ 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,
+fn find_local_branches<'a>(
+    view: &'a View,
     name_patterns: &[StringPattern],
-) -> Result<Vec<String>, CommandError> {
+) -> Result<Vec<(&'a str, &'a RefTarget)>, CommandError> {
     find_branches_with(name_patterns, |pattern| {
         view.local_branches_matching(pattern)
-            .map(|(name, _)| name.to_owned())
     })
 }
 
-fn find_branches_with<'a, I: Iterator<Item = String>>(
-    name_patterns: &'a [StringPattern],
-    mut find_matches: impl FnMut(&'a StringPattern) -> I,
-) -> Result<Vec<String>, CommandError> {
-    let mut matching_branches: Vec<String> = vec![];
+fn find_branches_with<'a, 'b, V, I: Iterator<Item = (&'a str, V)>>(
+    name_patterns: &'b [StringPattern],
+    mut find_matches: impl FnMut(&'b StringPattern) -> I,
+) -> Result<Vec<I::Item>, CommandError> {
+    let mut matching_branches: Vec<I::Item> = 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}"))),

cli/src/commands/branch/move.rs

@@ -12,12 +12,13 @@
 // 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;
 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;
@@ -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,38 +77,38 @@ 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.")?;
         }
     }
 
     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,16 +118,16 @@ 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()));
     }
     tx.finish(
         ui,
         format!(
-            "point {} to commit {}",
-            make_branch_term(&branch_names),
-            target_commit.id().hex()
+            "point branch {names} to commit {id}",
+            names = matched_branches.iter().map(|(name, _)| name).join(", "),
+            id = target_commit.id().hex()
         ),
     )?;
 

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

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()
         ),
     )?;
 

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(),

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(),

lib/src/repo.rs

@@ -1428,16 +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);
-    }
-
     pub fn get_local_branch(&self, name: &str) -> RefTarget {
         self.view.with_ref(|v| v.get_local_branch(name).clone())
     }