diff --git a/CHANGELOG.md b/CHANGELOG.md index fc45958b25..601666d146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Breaking changes +* `jj split` no longer moves bookmarks to the second revision created by the + split. Local bookmarks associated with the target revision will point to the + first revision (which inherits the target revision's change id) created by the + split. [#3419](https://github.com/jj-vcs/jj/issues/3419) + ### Deprecations ### New features diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 06e8e57c94..e556a41979 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -43,6 +43,9 @@ use crate::ui::Ui; /// description, the second part will not get a description, and you will be /// asked for a description only for the first part. /// +/// The first revision created by the split inherits the change id of the target +/// revision and any local bookmarks associated with it. +/// /// Splitting an empty commit is not supported because the same effect can be /// achieved with `jj new`. #[derive(clap::Args, Clone, Debug)] @@ -83,6 +86,7 @@ pub(crate) fn cmd_split( args: &SplitArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let at = workspace_command.resolve_single_rev(ui, &RevisionArg::AT)?; let commit = workspace_command.resolve_single_rev(ui, &args.revision)?; if commit.is_empty(workspace_command.repo().as_ref())? { return Err(user_error_with_hint( @@ -196,23 +200,21 @@ The remainder will be in the second commit. commit_builder.write(tx.repo_mut())? }; - // Mark the commit being split as rewritten to the second commit. As a - // result, if @ points to the commit being split, it will point to the - // second commit after the command finishes. This also means that any - // bookmarks pointing to the commit being split are moved to the second - // commit. + // Mark the first commit as the rewrite of the target commit. Since the + // first commit inherits the change id of the target commit, this ensures + // bookmarks continue pointing to the same change id before and after the + // split. tx.repo_mut() - .set_rewritten_commit(commit.id().clone(), second_commit.id().clone()); + .set_rewritten_commit(commit.id().clone(), first_commit.id().clone()); let mut num_rebased = 0; tx.repo_mut() .transform_descendants(vec![commit.id().clone()], |mut rewriter| { num_rebased += 1; if args.parallel { - rewriter - .replace_parent(second_commit.id(), [first_commit.id(), second_commit.id()]); + rewriter.replace_parent(first_commit.id(), [first_commit.id(), second_commit.id()]); + } else { + rewriter.replace_parent(first_commit.id(), [second_commit.id()]); } - // We don't need to do anything special for the non-parallel case - // since we already marked the original commit as rewritten. rewriter.rebase()?.write()?; Ok(()) })?; @@ -227,6 +229,10 @@ The remainder will be in the second commit. tx.write_commit_summary(formatter.as_mut(), &second_commit)?; writeln!(formatter)?; } + // If @ refers to the target commit, move it to the second commit. + if at.id() == commit.id() { + tx.edit(&second_commit)?; + } tx.finish(ui, format!("split commit {}", commit.id().hex()))?; Ok(()) } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 5791a09ec5..b31c84def8 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -2278,6 +2278,8 @@ Starts a [diff editor] on the changes in the revision. Edit the right side of th If the change you split had a description, you will be asked to enter a change description for each commit. If the change did not have a description, the second part will not get a description, and you will be asked for a description only for the first part. +The first revision created by the split inherits the change id of the target revision and any local bookmarks associated with it. + Splitting an empty commit is not supported because the same effect can be achieved with `jj new`. **Usage:** `jj split [OPTIONS] [FILESETS]...` diff --git a/cli/tests/test_split_command.rs b/cli/tests/test_split_command.rs index dfede90b10..4fb6da8a65 100644 --- a/cli/tests/test_split_command.rs +++ b/cli/tests/test_split_command.rs @@ -221,8 +221,7 @@ fn test_split_with_default_description() { std::fs::write(workspace_path.join("file1"), "foo\n").unwrap(); std::fs::write(workspace_path.join("file2"), "bar\n").unwrap(); - // Create a bookmark pointing to the commit. It will be moved to the second - // commit after the split. + // Create a bookmark pointing to the commit. It will not move during the split. test_env.jj_cmd_ok(&workspace_path, &["bookmark", "create", "test_bookmark"]); let edit_script = test_env.set_up_fake_editor(); @@ -234,10 +233,10 @@ fn test_split_with_default_description() { let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_path, &["split", "file1"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - First part: qpvuntsm 48018df6 TESTED=TODO - Second part: kkmpptxz 350b4c13 test_bookmark | (no description set) - Working copy now at: kkmpptxz 350b4c13 test_bookmark | (no description set) - Parent commit : qpvuntsm 48018df6 TESTED=TODO + First part: qpvuntsm 48018df6 test_bookmark | TESTED=TODO + Second part: kkmpptxz 350b4c13 (no description set) + Working copy now at: kkmpptxz 350b4c13 (no description set) + Parent commit : qpvuntsm 48018df6 test_bookmark | TESTED=TODO "###); // Since the commit being split has no description, the user will only be @@ -258,8 +257,9 @@ fn test_split_with_default_description() { "###); assert!(!test_env.env_root().join("editor2").exists()); insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###" - @ kkmpptxzrspx false test_bookmark + @ kkmpptxzrspx false ○ qpvuntsmwlqt false TESTED=TODO + │ test_bookmark ◆ zzzzzzzzzzzz true "###); } @@ -329,8 +329,7 @@ fn test_split_siblings_no_descendants() { std::fs::write(workspace_path.join("file1"), "foo\n").unwrap(); std::fs::write(workspace_path.join("file2"), "bar\n").unwrap(); - // Create a bookmark pointing to the commit. It will be moved to the second - // commit after the split. + // Create a bookmark pointing to the commit. It will not move during the split. test_env.jj_cmd_ok(&workspace_path, &["bookmark", "create", "test_bookmark"]); insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###" @ qpvuntsmwlqt false test_bookmark @@ -346,16 +345,16 @@ fn test_split_siblings_no_descendants() { let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_path, &["split", "--parallel", "file1"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - First part: qpvuntsm 0dced07a TESTED=TODO - Second part: zsuskuln 0473f014 test_bookmark | (no description set) - Working copy now at: zsuskuln 0473f014 test_bookmark | (no description set) + First part: qpvuntsm 0dced07a test_bookmark | TESTED=TODO + Second part: zsuskuln 0473f014 (no description set) + Working copy now at: zsuskuln 0473f014 (no description set) Parent commit : zzzzzzzz 00000000 (empty) (no description set) Added 0 files, modified 0 files, removed 1 files "###); insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###" - @ zsuskulnrvyr false test_bookmark + @ zsuskulnrvyr false │ ○ qpvuntsmwlqt false TESTED=TODO - ├─╯ + ├─╯ test_bookmark ◆ zzzzzzzzzzzz true "###);