From 0fb582ed8f8c45b183874bf479bb9086b95bfcdf 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 | 18 ++++ cli/tests/test_advance_branches.rs | 164 ++++++++++++++++++++++++++++- 2 files changed, 181 insertions(+), 1 deletion(-) diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 335c8b5aeb..9655b7bb57 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -99,6 +99,18 @@ 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().ids().cloned().collect_vec(); + + let should_advance_branches = + target_commits.len() == 1 && !args.insert_before && !args.insert_after; + let (advance_branches_target, advanceable_branches) = if should_advance_branches { + let ab_target = target_ids[0].clone(); + let ab_branches = + workspace_command.get_advanceable_branches(target_commits[0].parent_ids())?; + (Some(ab_target), ab_branches) + } else { + (None, Vec::new()) + }; + let mut tx = workspace_command.start_transaction(); let mut num_rebased; let new_commit; @@ -197,6 +209,12 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", if num_rebased > 0 { writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } + + // Does nothing if there's no branches to advance. + if let Some(target) = advance_branches_target { + tx.advance_branches(advanceable_branches, &target); + } + tx.finish(ui, "new empty commit")?; Ok(()) } diff --git a/cli/tests/test_advance_branches.rs b/cli/tests/test_advance_branches.rs index 4a53cd42eb..afa25c2f9d 100644 --- a/cli/tests/test_advance_branches.rs +++ b/cli/tests/test_advance_branches.rs @@ -35,7 +35,7 @@ fn set_advance_branches(test_env: &TestEnvironment, enabled: bool) { } else { test_env.add_config( r#"[experimental-advance-branches] - enabled-branches = [""] + enabled-branches = [] "#, ); } @@ -51,8 +51,15 @@ 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"]); +} + // Check that enabling and disabling advance-branches works as expected. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_enabled(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -100,6 +107,7 @@ fn test_advance_branches_enabled(make_commit: CommitFn) { // Check that only a branch pointing to @- advances. Branches pointing to @ are // not advanced. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_at_minus(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -141,6 +149,7 @@ fn test_advance_branches_at_minus(make_commit: CommitFn) { // Test that per-branch overrides invert the behavior of // experimental-advance-branches.enabled. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_overrides(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -251,6 +260,7 @@ fn test_advance_branches_overrides(make_commit: CommitFn) { // If multiple eligible branches point to @-, all of them will be advanced. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_multiple_branches(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -284,3 +294,155 @@ fn test_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"); + + set_advance_branches(&test_env, 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"); + + set_advance_branches(&test_env, 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"); + + set_advance_branches(&test_env, 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: + "###); +} + +#[test] +fn test_new_advance_branches_merge_children() { + 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"); + + set_advance_branches(&test_env, true); + test_env.jj_cmd_ok(&workspace_path, &["desc", "-m", "0"]); + test_env.jj_cmd_ok(&workspace_path, &["new", "-m", "1"]); + test_env.jj_cmd_ok(&workspace_path, &["new", "description(0)", "-m", "2"]); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "test_branch", "-r", "description(0)"], + ); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: 2 + │ ◉ branches{} desc: 1 + ├─╯ + ◉ branches{test_branch} desc: 0 + ◉ branches{} desc: + "###); + + // The branch won't advance because `jj new` had multiple targets. + test_env.jj_cmd_ok( + &workspace_path, + &["new", "description(1)", "description(2)"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ├─╮ + │ ◉ branches{} desc: 2 + ◉ │ branches{} desc: 1 + ├─╯ + ◉ branches{test_branch} desc: 0 + ◉ branches{} desc: + "###); +}