Skip to content

Commit

Permalink
Implement advance-branches for jj new
Browse files Browse the repository at this point in the history
  • Loading branch information
emesterhazy committed Mar 20, 2024
1 parent 93f8f3d commit 57911bb
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 3 deletions.
30 changes: 27 additions & 3 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()],
)?;
}
Expand Down Expand Up @@ -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<AdvanceableBranch> {
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()
}
}
129 changes: 129 additions & 0 deletions cli/tests/test_advance_branches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),)*) => {
$(
Expand All @@ -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.
Expand Down Expand Up @@ -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:
"###);
}

0 comments on commit 57911bb

Please sign in to comment.