From ac8313f119bf277e4923e0276562d9b62de036fe Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 2 Jun 2023 23:03:14 -0700 Subject: [PATCH] cli: in `jj git push`, create transaction in a single place I added a function for updating the description on an existing transaction. That way we can create the transaction earlier. I'll try to make `--change` and `--branch` not mutually exclusive next. --- lib/src/transaction.rs | 4 +++ src/cli_util.rs | 4 +++ src/commands/git.rs | 61 +++++++++++++++++++++--------------------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 42953ab51..59835b3f5 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -52,6 +52,10 @@ impl Transaction { self.mut_repo.base_repo() } + pub fn set_description(&mut self, description: &str) { + self.op_metadata.description = description.to_string(); + } + pub fn set_tag(&mut self, key: String, value: String) { self.op_metadata.tags.insert(key, value); } diff --git a/src/cli_util.rs b/src/cli_util.rs index caf24faf3..9ddb388d7 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -1221,6 +1221,10 @@ impl WorkspaceCommandTransaction<'_> { self.tx.mut_repo() } + pub fn set_description(&mut self, description: &str) { + self.tx.set_description(description) + } + pub fn check_out(&mut self, commit: &Commit) -> Result { let workspace_id = self.helper.workspace_id().to_owned(); let settings = &self.helper.settings; diff --git a/src/commands/git.rs b/src/commands/git.rs index 744a99646..c597f2c3f 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -624,12 +624,20 @@ fn cmd_git_push( get_default_push_remote(ui, command, &git_repo)? }; - let mut tx; + let repo = workspace_command.repo().clone(); + let wc_commit_id = workspace_command.get_wc_commit_id().cloned(); + let change_commits: Vec<_> = args + .change + .iter() + .map(|change_str| workspace_command.resolve_single_rev(change_str)) + .try_collect()?; + let mut tx = workspace_command.start_transaction(""); + let tx_description; let mut branch_updates = vec![]; let mut seen_branches = hashset! {}; if args.all || args.deleted { // TODO: Is it useful to warn about conflicted branches? - for (branch_name, branch_target) in workspace_command.repo().view().branches() { + for (branch_name, branch_target) in repo.view().branches() { if !seen_branches.insert(branch_name.clone()) { continue; } @@ -645,19 +653,17 @@ fn cmd_git_push( } } } - tx = workspace_command.start_transaction(&format!( + tx_description = format!( "push all {}branches to git remote {}", if args.deleted { "deleted " } else { "" }, &remote - )); + ); } else if !args.branch.is_empty() { for branch_name in &args.branch { if !seen_branches.insert(branch_name.clone()) { continue; } - if let Some(update) = - branch_updates_for_push(workspace_command.repo().as_ref(), &remote, branch_name)? - { + if let Some(update) = branch_updates_for_push(repo.as_ref(), &remote, branch_name)? { branch_updates.push((branch_name.clone(), update)); } else { writeln!( @@ -667,29 +673,27 @@ fn cmd_git_push( )?; } } - tx = workspace_command.start_transaction(&format!( + tx_description = format!( "push {} to git remote {}", make_branch_term(&args.branch), &remote - )); + ); } else if !args.change.is_empty() { // TODO: Allow specifying --branch and --change at the same time - let commits: Vec<_> = args - .change - .iter() - .map(|change_str| workspace_command.resolve_single_rev(change_str)) - .try_collect()?; - tx = workspace_command.start_transaction(&format!( + tx_description = format!( "push {} {} to git remote {}", - if commits.len() > 1 { + if change_commits.len() > 1 { "changes" } else { "change" }, - commits.iter().map(|c| c.change_id().hex()).join(", "), + change_commits + .iter() + .map(|c| c.change_id().hex()) + .join(", "), &remote - )); - for (change_str, commit) in std::iter::zip(args.change.iter(), commits) { + ); + for (change_str, commit) in std::iter::zip(args.change.iter(), change_commits) { let mut branch_name = format!( "{}{}", command.settings().push_branch_prefix(), @@ -737,7 +741,7 @@ fn cmd_git_push( } } } else { - match workspace_command.get_wc_commit_id() { + match wc_commit_id { None => { return Err(user_error("Nothing checked out in this workspace")); } @@ -755,17 +759,15 @@ fn cmd_git_push( } // Search for branches targeting @ - let mut branches = find_branches_targeting( - workspace_command.repo().view(), - &RefTarget::Normal(wc_commit.clone()), - ); + let mut branches = + find_branches_targeting(repo.view(), &RefTarget::Normal(wc_commit.clone())); if branches.is_empty() { // Try @- instead if @ is discardable - let commit = workspace_command.repo().store().get_commit(wc_commit)?; + let commit = repo.store().get_commit(&wc_commit)?; if commit.is_discardable() { if let [parent_commit_id] = commit.parent_ids() { branches = find_branches_targeting( - workspace_command.repo().view(), + repo.view(), &RefTarget::Normal(parent_commit_id.clone()), ); } @@ -790,10 +792,7 @@ fn cmd_git_push( } } } - tx = workspace_command.start_transaction(&format!( - "push current branch(es) to git remote {}", - &remote - )); + tx_description = format!("push current branch(es) to git remote {}", &remote); } drop(seen_branches); @@ -802,7 +801,7 @@ fn cmd_git_push( return Ok(()); } - let repo = tx.base_repo(); + tx.set_description(&tx_description); let mut ref_updates = vec![]; let mut new_heads = vec![];