From 57911bbbae0837015534ab0c27d67bce622ddb9e Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Mon, 18 Mar 2024 23:35:10 -0400 Subject: [PATCH] Implement advance-branches for jj new --- cli/src/commands/new.rs | 30 ++++++- cli/tests/test_advance_branches.rs | 129 +++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index a94d80769e..e3ab4a94ce 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -22,7 +22,9 @@ use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; use jj_lib::rewrite::{merge_commit_trees, rebase_commit}; use tracing::instrument; -use crate::cli_util::{self, short_commit_hash, CommandHelper, RevisionArg}; +use crate::cli_util::{ + self, short_commit_hash, AdvanceableBranch, CommandHelper, RevisionArg, WorkspaceCommandHelper, +}; use crate::command_error::{user_error, CommandError}; use crate::description_util::join_message_paragraphs; use crate::ui::Ui; @@ -102,6 +104,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", .into_iter() .collect_vec(); let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec(); + let advanceable_branches = get_advanceable_branches(args, &workspace_command, &target_commits); let mut tx = workspace_command.start_transaction(); let mut num_rebased; let new_commit; @@ -145,11 +148,11 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", .set_description(join_message_paragraphs(&args.message_paragraphs)) .write()?; num_rebased = target_ids.len(); - for child_commit in target_commits { + for child_commit in &target_commits { rebase_commit( command.settings(), tx.mut_repo(), - &child_commit, + child_commit, &[new_commit.clone()], )?; } @@ -206,6 +209,27 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", if num_rebased > 0 { writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; } + + if !advanceable_branches.is_empty() { + tx.advance_branches(advanceable_branches, target_commits[0].id()); + } + tx.finish(ui, "new empty commit")?; Ok(()) } + +// Branches are only advanced if `jj new` has a single target commit and the +// new commit is not being inserted before or after the target. +fn get_advanceable_branches( + args: &NewArgs, + ws: &WorkspaceCommandHelper, + target_commits: &[Commit], +) -> Vec { + let should_advance_branches = + target_commits.len() == 1 && !args.insert_before && !args.insert_after; + if should_advance_branches { + ws.get_advanceable_branches(&**ws.repo(), target_commits[0].parent_ids()) + } else { + Vec::new() + } +} diff --git a/cli/tests/test_advance_branches.rs b/cli/tests/test_advance_branches.rs index f68d7c286b..a0fec3ce4f 100644 --- a/cli/tests/test_advance_branches.rs +++ b/cli/tests/test_advance_branches.rs @@ -52,6 +52,12 @@ fn commit_cmd(env: &TestEnvironment, workspace_path: &Path, commit_message: &str env.jj_cmd_ok(workspace_path, &["commit", "-m", commit_message]); } +// Implements CommitFn using the `jj describe` and `jj new`. +fn describe_new_cmd(env: &TestEnvironment, workspace_path: &Path, commit_message: &str) { + env.jj_cmd_ok(workspace_path, &["describe", "-m", commit_message]); + env.jj_cmd_ok(workspace_path, &["new"]); +} + macro_rules! parameterized_tests{ ($($test_name:ident: $case_fn:ident($commit_fn:ident),)*) => { $( @@ -69,6 +75,11 @@ parameterized_tests! { test_commit_advance_branches_overrides: case_advance_branches_overrides(commit_cmd), test_commit_advance_branches_multiple_branches: case_advance_branches_multiple_branches(commit_cmd), + test_new_advance_branches_enabled: case_advance_branches_enabled(describe_new_cmd), + test_new_advance_branches_at_minus: case_advance_branches_at_minus(describe_new_cmd), + test_new_advance_branches_overrides: case_advance_branches_overrides(describe_new_cmd), + test_new_advance_branches_multiple_branches: + case_advance_branches_multiple_branches(describe_new_cmd), } // Check that enabling and disabling advance-branches works as expected. @@ -292,3 +303,121 @@ fn case_advance_branches_multiple_branches(make_commit: CommitFn) { "###); } } + +// Call `jj new` on an interior commit and see that the branch pointing to its +// parent's parent is advanced. +#[test] +fn test_new_advance_branches_interior() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + // First, test with advance-branches enabled. Start by creating a branch on the + // root commit. + set_advance_branches(&test_env, &workspace_path, true); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: + "###); + + // Create a gap in the commits for us to insert our new commit with --before. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "third"]); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@---", "test_branch"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: third + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["new", "-r", "@--"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + │ ◉ branches{} desc: third + ├─╯ + ◉ branches{test_branch} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: + "###); +} + +// If the `--before` flag is passed to `jj new`, branches are not advanced. +#[test] +fn test_new_advance_branches_before() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + // First, test with advance-branches enabled. Start by creating a branch on the + // root commit. + set_advance_branches(&test_env, &workspace_path, true); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: + "###); + + // Create a gap in the commits for us to insert our new commit with --before. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "third"]); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@---", "test_branch"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: third + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["new", "--before", "-r", "@-"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + ◉ branches{} desc: third + @ branches{} desc: + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); +} + +// If the `--after` flag is passed to `jj new`, branches are not advanced. +#[test] +fn test_new_advance_branches_after() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + // First, test with advance-branches enabled. Start by creating a branch on the + // root commit. + set_advance_branches(&test_env, &workspace_path, true); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@-", "test_branch"], + ); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{test_branch} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["describe", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["new", "--after"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: first + ◉ branches{test_branch} desc: + "###); +}