From 1a8ad4465f701da1b209ed844069405e452b8cb5 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 | 115 +++++++++++++++++++++++++++++ cli/tests/cli-reference@.md.snap | 32 ++++++++ cli/tests/test_branch_command.rs | 122 +++++++++++++++++++++++++++++++ 4 files changed, 272 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c7d216365..26ce0775aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj file` now groups commands for working with files. +* 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 15fdf490d6..a9cd70e4c1 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), @@ -391,6 +429,83 @@ 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.names.is_empty() { + writeln!(ui.hint_default(), "Specify branch by name to update one.")?; + } + } + + 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 291b93d893..d0d36cf2ac 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) @@ -236,6 +237,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 @@ -321,6 +323,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..f4d5147469 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,124 @@ 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 + Hint: Specify branch by name to update one. + "###); + 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]