cli split: Leave bookarks associated with the original change id

`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 afterward.

Since the first revision inherits the change id of the target revision, this
behavior is less surprising than moving the bookmark to the second revision.

This no-implicit-move behavior also aligns with how `jj abandon` drops
bookmarks instead of moving them to the parent revision.

fixes #3419

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

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(())
 }

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]...`

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
     "###);