From a64542eb0a94148de749eef8c05c7cdfb0594aba Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 16 Jun 2024 15:40:53 +0900 Subject: [PATCH] cli: branch: add "move" command that can update branches by revset or name This basically supersedes the current "branch set" command. The plan is to turn "branch set" into an "upsert" command, and deprecate "branch create". (#3584) Maybe we can also add "branch set --new" flag to only allow creation of new branches. One reason behind this proposed change is that "set" usually allows both "creation" and "update". However, we also need a typo-safe version of "set" to not create new branches by accident. "jj branch move" is useful when advancing ancestor branches. Let's say you've added a couple of commits on top of an existing PR branch, you can advance the branch by "jj branch move --from 'heads(::@- & branches())' --to @-". If this pattern is super common, maybe we can add --advance flag for short. One drawback of this change is that "git branch --move" is equivalent to "jj branch rename". I personally don't find this is confusing, but it's true that "move" sometimes means "rename". --- CHANGELOG.md | 3 + cli/src/commands/branch.rs | 112 ++++++++++++++++++++++++++++ cli/tests/cli-reference@.md.snap | 32 ++++++++ cli/tests/test_branch_command.rs | 121 +++++++++++++++++++++++++++++++ 4 files changed, 268 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0401b87344..e2477ccc80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Conflicted files are individually simplified before being materialized. +* New command `jj branch move` let you update branches by name pattern or source + revision. + ### Fixed bugs ## [0.18.0] - 2024-06-05 diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index c2619a6d80..1a9b552009 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -46,6 +46,8 @@ pub enum BranchCommand { Forget(BranchForgetArgs), #[command(visible_alias("l"))] List(BranchListArgs), + #[command(visible_alias("m"))] + Move(BranchMoveArgs), #[command(visible_alias("r"))] Rename(BranchRenameArgs), #[command(visible_alias("s"))] @@ -183,6 +185,41 @@ pub struct BranchSetArgs { pub names: Vec, } +/// Move existing branches to target revision +/// +/// If branch names are given, the specified branches will be updated to point +/// to the target revision. +/// +/// If `--from` options are given, branches currently pointing to the specified +/// revisions will be updated. The branches can also be filtered by names. +/// +/// Example: pull up the nearest branches to the working-copy parent +/// +/// $ jj branch move --from 'heads(::@- & branches())' --to @- +#[derive(clap::Args, Clone, Debug)] +#[command(group(clap::ArgGroup::new("source").multiple(true).required(true)))] +pub struct BranchMoveArgs { + /// Move branches from the given revisions + #[arg(long, group = "source", value_name = "REVISIONS")] + from: Vec, + + /// Move branches to this revision + #[arg(long, default_value = "@", value_name = "REVISION")] + to: RevisionArg, + + /// Allow moving branches backwards or sideways + #[arg(long, short = 'B')] + allow_backwards: bool, + + /// Move branches matching the given name patterns + /// + /// By default, the specified name matches exactly. Use `glob:` prefix to + /// select branches by wildcard pattern. For details, see + /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. + #[arg(group = "source", value_parser = StringPattern::parse)] + names: Vec, +} + /// Start tracking given remote branches /// /// A tracking remote branch will be imported as a local branch of the same @@ -234,6 +271,7 @@ pub fn cmd_branch( BranchCommand::Create(sub_args) => cmd_branch_create(ui, command, sub_args), BranchCommand::Rename(sub_args) => cmd_branch_rename(ui, command, sub_args), BranchCommand::Set(sub_args) => cmd_branch_set(ui, command, sub_args), + BranchCommand::Move(sub_args) => cmd_branch_move(ui, command, sub_args), BranchCommand::Delete(sub_args) => cmd_branch_delete(ui, command, sub_args), BranchCommand::Forget(sub_args) => cmd_branch_forget(ui, command, sub_args), BranchCommand::Track(sub_args) => cmd_branch_track(ui, command, sub_args), @@ -393,6 +431,80 @@ fn cmd_branch_set( Ok(()) } +fn cmd_branch_move( + ui: &mut Ui, + command: &CommandHelper, + args: &BranchMoveArgs, +) -> Result<(), CommandError> { + let mut workspace_command = command.workspace_helper(ui)?; + let repo = workspace_command.repo().as_ref(); + let view = repo.view(); + + let branch_names = { + let is_source_commit = if !args.from.is_empty() { + workspace_command + .parse_union_revsets(&args.from)? + .evaluate()? + .containing_fn() + } else { + Box::new(|_: &CommitId| true) + }; + if !args.names.is_empty() { + find_branches_with(&args.names, |pattern| { + view.local_branches_matching(pattern) + .filter(|(_, target)| target.added_ids().any(&is_source_commit)) + .map(|(name, _)| name.to_owned()) + })? + } else { + 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() { + writeln!(ui.status(), "No branches to update.")?; + return Ok(()); + } + if branch_names.len() > 1 { + writeln!( + ui.warning_default(), + "Updating multiple branches: {}", + branch_names.join(", "), + )?; + } + + if !args.allow_backwards { + if let Some(name) = branch_names + .iter() + .find(|name| !is_fast_forward(repo, view.get_local_branch(name), target_commit.id())) + { + return Err(user_error_with_hint( + format!("Refusing to move branch backwards or sideways: {name}"), + "Use --allow-backwards to allow it.", + )); + } + } + + let mut tx = workspace_command.start_transaction(); + for name in &branch_names { + 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() + ), + )?; + + Ok(()) +} + fn find_local_branches( view: &View, name_patterns: &[StringPattern], diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index cc2f926482..cc88c44910 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -18,6 +18,7 @@ This document contains the help content for the `jj` command-line program. * [`jj branch delete`↴](#jj-branch-delete) * [`jj branch forget`↴](#jj-branch-forget) * [`jj branch list`↴](#jj-branch-list) +* [`jj branch move`↴](#jj-branch-move) * [`jj branch rename`↴](#jj-branch-rename) * [`jj branch set`↴](#jj-branch-set) * [`jj branch track`↴](#jj-branch-track) @@ -237,6 +238,7 @@ For information about branches, see https://github.com/martinvonz/jj/blob/main/d * `delete` — Delete an existing branch and propagate the deletion to remotes on the next push * `forget` — Forget everything about a branch, including its local and remote targets * `list` — List branches and their targets +* `move` — Move existing branches to target revision * `rename` — Rename `old` branch name to `new` branch name * `set` — Update an existing branch to point to a certain commit * `track` — Start tracking given remote branches @@ -322,6 +324,36 @@ For information about branches, see https://github.com/martinvonz/jj/blob/main/d +## `jj branch move` + +Move existing branches to target revision + +If branch names are given, the specified branches will be updated to point to the target revision. + +If `--from` options are given, branches currently pointing to the specified revisions will be updated. The branches can also be filtered by names. + +Example: pull up the nearest branches to the working-copy parent + +$ jj branch move --from 'heads(::@- & branches())' --to @- + +**Usage:** `jj branch move [OPTIONS] <--from |NAMES>` + +###### **Arguments:** + +* `` — Move branches matching the given name patterns + + By default, the specified name matches exactly. Use `glob:` prefix to select branches by wildcard pattern. For details, see https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. + +###### **Options:** + +* `--from ` — Move branches from the given revisions +* `--to ` — Move branches to this revision + + Default value: `@` +* `-B`, `--allow-backwards` — Allow moving branches backwards or sideways + + + ## `jj branch rename` Rename `old` branch name to `new` branch name. diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index fb13919981..9c0a75ba35 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -101,6 +101,10 @@ fn test_branch_move() { Error: No such branch: foo Hint: Use `jj branch create` to create it. "###); + let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "foo"]); + insta::assert_snapshot!(stderr, @r###" + Error: No such branch: foo + "###); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]); insta::assert_snapshot!(stderr, @""); @@ -126,6 +130,123 @@ fn test_branch_move() { &["branch", "set", "-r@-", "--allow-backwards", "foo"], ); insta::assert_snapshot!(stderr, @""); + + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "foo"]); + insta::assert_snapshot!(stderr, @""); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "--to=@-", "foo"]); + insta::assert_snapshot!(stderr, @r###" + Error: Refusing to move branch backwards or sideways: foo + Hint: Use --allow-backwards to allow it. + "###); + + let (_stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["branch", "move", "--to=@-", "--allow-backwards", "foo"], + ); + insta::assert_snapshot!(stderr, @""); +} + +#[test] +fn test_branch_move_matching() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a1", "a2"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-mhead1"]); + test_env.jj_cmd_ok(&repo_path, &["new", "root()"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b1"]); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "c1"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-mhead2"]); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ a2781dd9ee37 + ◉ c1 f4f38657a3dd + ◉ b1 f652c32197cf + │ ◉ 6b5e840ea72b + │ ◉ a1 a2 230dd059e1b0 + ├─╯ + ◉ 000000000000 + "###); + + // The default could be considered "--from=all() glob:*", but is disabled + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "move"]); + insta::assert_snapshot!(stderr, @r###" + error: the following required arguments were not provided: + <--from |NAMES> + + Usage: jj branch move <--from |NAMES> + + For more information, try '--help'. + "###); + + // No branches pointing to the source revisions + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "--from=none()"]); + insta::assert_snapshot!(stderr, @r###" + No branches to update. + "###); + + // No matching branches within the source revisions + let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "--from=::@", "glob:a?"]); + insta::assert_snapshot!(stderr, @r###" + Error: No matching branches for patterns: a? + "###); + + // Noop move + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "--to=a1", "a2"]); + insta::assert_snapshot!(stderr, @r###" + Nothing changed. + "###); + + // Move from multiple revisions + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "--from=::@"]); + insta::assert_snapshot!(stderr, @r###" + Warning: Updating multiple branches: b1, c1 + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ b1 c1 a2781dd9ee37 + ◉ f4f38657a3dd + ◉ f652c32197cf + │ ◉ 6b5e840ea72b + │ ◉ a1 a2 230dd059e1b0 + ├─╯ + ◉ 000000000000 + "###); + test_env.jj_cmd_ok(&repo_path, &["undo"]); + + // Try to move multiple branches, but one of them isn't fast-forward + let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "glob:?1"]); + insta::assert_snapshot!(stderr, @r###" + Warning: Updating multiple branches: a1, b1, c1 + Error: Refusing to move branch backwards or sideways: a1 + Hint: Use --allow-backwards to allow it. + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ a2781dd9ee37 + ◉ c1 f4f38657a3dd + ◉ b1 f652c32197cf + │ ◉ 6b5e840ea72b + │ ◉ a1 a2 230dd059e1b0 + ├─╯ + ◉ 000000000000 + "###); + + // Select by revision and name + let (_stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["branch", "move", "--from=::a1+", "--to=a1+", "glob:?1"], + ); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ a2781dd9ee37 + ◉ c1 f4f38657a3dd + ◉ b1 f652c32197cf + │ ◉ a1 6b5e840ea72b + │ ◉ a2 230dd059e1b0 + ├─╯ + ◉ 000000000000 + "###); } #[test]