Add a --siblings option to the jj split command

If the --siblings option is used, the target commit is split into two sibling
commits instead of parent and child commits. Any children of the original
commit will have both siblings as their new parents.


#2274

CHANGELOG.md

@@ -60,6 +60,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * There is a new global `--quiet` flag to silence commands' non-primary output.
 
+* `jj split` now supports a `--siblings/-s` option that splits the target
+  revision into siblings with the same parents and children.
+
 ### Fixed bugs
 
 ## [0.15.1] - 2024-03-06

cli/src/commands/split.rs

@@ -13,13 +13,17 @@
 // limitations under the License.
 use std::io::Write;
 
+use itertools::Itertools;
+use jj_lib::commit::Commit;
 use jj_lib::object_id::ObjectId;
 use jj_lib::repo::Repo;
+use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 use jj_lib::rewrite::merge_commit_trees;
 use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
 use crate::command_error::CommandError;
+use crate::commands::rebase::rebase_descendants;
 use crate::description_util::{description_template_for_commit, edit_description};
 use crate::ui::Ui;
 
@@ -47,6 +51,9 @@ pub(crate) struct SplitArgs {
     /// The revision to split
     #[arg(long, short, default_value = "@")]
     revision: RevisionArg,
+    /// Split the revision into two siblings instead of a parent and child.
+    #[arg(long, short)]
+    siblings: bool,
     /// Put these paths in the first commit
     #[arg(value_hint = clap::ValueHint::AnyPath)]
     paths: Vec<String>,
@@ -72,13 +79,13 @@ pub(crate) fn cmd_split(
     let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
     let instructions = format!(
         "\
-You are splitting a commit in two: {}
+You are splitting a commit into two: {}
 
 The diff initially shows the changes in the commit you're splitting.
 
-Adjust the right side until it shows the contents you want for the first
-(parent) commit. The remainder will be in the second commit. If you
-don't make any changes, then the operation will be aborted.
+Adjust the right side until it shows the contents you want for the first commit.
+The remainder will be in the second commit. If you don't make any changes, then
+the operation will be aborted.
 ",
         tx.format_commit_summary(&commit)
     );
@@ -106,7 +113,7 @@ don't make any changes, then the operation will be aborted.
         ui,
         command.settings(),
         tx.base_workspace_helper(),
-        "Enter commit description for the first part (parent).",
+        "Enter a description for the first commit.",
         commit.description(),
         &base_tree,
         &selected_tree,
@@ -119,7 +126,21 @@ don't make any changes, then the operation will be aborted.
         .set_description(first_description)
         .write()?;
 
-    // Create the second commit, which includes everything the user didn't select.
+    // Create the second commit, which includes everything the user didn't
+    // select.
+    let (second_tree, second_base_tree) = if args.siblings {
+        // Merge the original commit tree with its parent using the tree
+        // containing the user selected changes as the base for the merge.
+        // This results in a tree with the changes the user didn't select.
+        (end_tree.merge(&selected_tree, &base_tree)?, &base_tree)
+    } else {
+        (end_tree, &selected_tree)
+    };
+    let second_commit_parents = if args.siblings {
+        commit.parent_ids().to_vec()
+    } else {
+        vec![first_commit.id().clone()]
+    };
     let second_description = if commit.description().is_empty() {
         // If there was no description before, don't ask for one for the second commit.
         "".to_string()
@@ -128,27 +149,50 @@ don't make any changes, then the operation will be aborted.
             ui,
             command.settings(),
             tx.base_workspace_helper(),
-            "Enter commit description for the second part (child).",
+            "Enter a description for the second commit.",
             commit.description(),
-            &selected_tree,
-            &end_tree,
+            second_base_tree,
+            &second_tree,
         )?;
         edit_description(tx.base_repo(), &second_template, command.settings())?
     };
     let second_commit = tx
         .mut_repo()
         .rewrite_commit(command.settings(), &commit)
-        .set_parents(vec![first_commit.id().clone()])
-        .set_tree_id(commit.tree_id().clone())
+        .set_parents(second_commit_parents)
+        .set_tree_id(second_tree.id())
+        // Generate a new change id so that the commit being split doesn't
+        // become divergent.
         .generate_new_change_id()
         .set_description(second_description)
         .write()?;
 
-    // We want only the `second_commit` to inherit `commit`'s branches and
-    // descendants.
+    // 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
+    // branches pointing to the commit being split are moved to the second
+    // commit.
     tx.mut_repo()
         .set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
-    let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
+    // Rebase descendants of the commit being split.
+    let new_parents = if args.siblings {
+        vec![first_commit.clone(), second_commit.clone()]
+    } else {
+        vec![second_commit.clone()]
+    };
+    let children: Vec<Commit> = RevsetExpression::commit(commit.id().clone())
+        .children()
+        .evaluate_programmatic(tx.base_repo().as_ref())?
+        .iter()
+        .commits(tx.base_repo().store())
+        .try_collect()?;
+    let num_rebased = rebase_descendants(
+        &mut tx,
+        command.settings(),
+        &new_parents,
+        &children,
+        Default::default(),
+    )?;
     if let Some(mut formatter) = ui.status_formatter() {
         if num_rebased > 0 {
             writeln!(formatter, "Rebased {num_rebased} descendant commits")?;

cli/tests/cli-reference@.md.snap

@@ -1690,6 +1690,10 @@ If the change you split had a description, you will be asked to enter a change d
 * `-r`, `--revision <REVISION>` — The revision to split
 
   Default value: `@`
+* `-s`, `--siblings` — Split the revision into two siblings instead of a parent and child
+
+  Possible values: `true`, `false`
+
 
 
 

cli/tests/test_split_command.rs

@@ -16,6 +16,16 @@ use std::path::Path;
 
 use crate::common::TestEnvironment;
 
+fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
+    let template = r#"separate(" ", change_id.short(), empty, description)"#;
+    test_env.jj_cmd_success(cwd, &["log", "-T", template])
+}
+
+fn get_recorded_dates(test_env: &TestEnvironment, cwd: &Path, revset: &str) -> String {
+    let template = r#"separate("\n", "Author date:  " ++ author.timestamp(), "Committer date: " ++ committer.timestamp())"#;
+    test_env.jj_cmd_success(cwd, &["log", "--no-graph", "-T", template, "-r", revset])
+}
+
 #[test]
 fn test_split_by_paths() {
     let mut test_env = TestEnvironment::default();
@@ -51,7 +61,7 @@ fn test_split_by_paths() {
     "###);
     insta::assert_snapshot!(
         std::fs::read_to_string(test_env.env_root().join("editor0")).unwrap(), @r###"
-    JJ: Enter commit description for the first part (parent).
+    JJ: Enter a description for the first commit.
 
     JJ: This commit contains the following changes:
     JJ:     A file2
@@ -167,7 +177,7 @@ fn test_split_with_non_empty_description() {
 
     assert_eq!(
         std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(),
-        r#"JJ: Enter commit description for the first part (parent).
+        r#"JJ: Enter a description for the first commit.
 test
 
 JJ: This commit contains the following changes:
@@ -178,7 +188,7 @@ JJ: Lines starting with "JJ: " (like this one) will be removed.
     );
     assert_eq!(
         std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(),
-        r#"JJ: Enter commit description for the second part (child).
+        r#"JJ: Enter a description for the second commit.
 test
 
 JJ: This commit contains the following changes:
@@ -211,9 +221,13 @@ fn test_split_with_default_description() {
     .unwrap();
     test_env.jj_cmd_ok(&workspace_path, &["split", "file1"]);
 
+    // Since the commit being split has no description, the user will only be
+    // prompted to add a description to the first commit, which will use the
+    // default value we set. The second commit will inherit the empty
+    // description from the commit being split.
     assert_eq!(
         std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(),
-        r#"JJ: Enter commit description for the first part (parent).
+        r#"JJ: Enter a description for the first commit.
 
 
 TESTED=TODO
@@ -231,12 +245,121 @@ JJ: Lines starting with "JJ: " (like this one) will be removed.
     "###);
 }
 
-fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
-    let template = r#"separate(" ", change_id.short(), empty, description)"#;
-    test_env.jj_cmd_success(cwd, &["log", "-T", template])
+#[test]
+// Split a commit with no descendants into siblings. Also tests that the default
+// description is set correctly on the first commit.
+fn test_split_siblings_no_descendants() {
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    test_env.add_config(r#"ui.default-description = "\n\nTESTED=TODO""#);
+    let workspace_path = test_env.env_root().join("repo");
+
+    std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
+    std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();
+    let edit_script = test_env.set_up_fake_editor();
+    std::fs::write(
+        edit_script,
+        ["dump editor1", "next invocation\n", "dump editor2"].join("\0"),
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&workspace_path, &["split", "--siblings", "file1"]);
+
+    // Since the commit being split has no description, the user will only be
+    // prompted to add a description to the first commit, which will use the
+    // default value we set. The second commit will inherit the empty
+    // description from the commit being split.
+    assert_eq!(
+        std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(),
+        r#"JJ: Enter a description for the first commit.
+
+
+TESTED=TODO
+JJ: This commit contains the following changes:
+JJ:     A file1
+
+JJ: Lines starting with "JJ: " (like this one) will be removed.
+"#
+    );
+    assert!(!test_env.env_root().join("editor2").exists());
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @  rlvkpnrzqnoo false
+    │ ◉  qpvuntsmwlqt false TESTED=TODO
+    ├─╯
+    ◉  zzzzzzzzzzzz true
+    "###);
 }
 
-fn get_recorded_dates(test_env: &TestEnvironment, cwd: &Path, revset: &str) -> String {
-    let template = r#"separate("\n", "Author date:  " ++ author.timestamp(), "Committer date: " ++ committer.timestamp())"#;
-    test_env.jj_cmd_success(cwd, &["log", "--no-graph", "-T", template, "-r", revset])
+#[test]
+fn test_split_siblings_with_descendants() {
+    // Configure the environment and make the initial commits.
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    // test_env.add_config(r#"ui.default-description = "\n\nTESTED=TODO""#);
+    let workspace_path = test_env.env_root().join("repo");
+
+    // First commit. This is the one we will split later.
+    std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
+    std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();
+    test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "Add file1 & file2"]);
+    // Second commit. This will be the child of the sibling commits after the split.
+    std::fs::write(workspace_path.join("file3"), "baz\n").unwrap();
+    test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "Add file3"]);
+    // Third commit.
+    std::fs::write(workspace_path.join("file4"), "foobarbaz\n").unwrap();
+    test_env.jj_cmd_ok(&workspace_path, &["describe", "-m", "Add file4"]);
+    // Move back to the previous commit so that we don't have to pass a revision
+    // to the split command.
+    test_env.jj_cmd_ok(&workspace_path, &["prev", "--edit"]);
+    test_env.jj_cmd_ok(&workspace_path, &["prev", "--edit"]);
+
+    // Set up the editor and do the split.
+    let edit_script = test_env.set_up_fake_editor();
+    std::fs::write(
+        edit_script,
+        [
+            "dump editor1",
+            "write\nAdd file1",
+            "next invocation\n",
+            "dump editor2",
+            "write\nAdd file2",
+        ]
+        .join("\0"),
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&workspace_path, &["split", "--siblings", "file1"]);
+
+    // The commit we're splitting has a description, so the user will be
+    // prompted to enter a description for each of the sibling commits.
+    assert_eq!(
+        std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(),
+        r#"JJ: Enter a description for the first commit.
+Add file1 & file2
+
+JJ: This commit contains the following changes:
+JJ:     A file1
+
+JJ: Lines starting with "JJ: " (like this one) will be removed.
+"#
+    );
+    assert_eq!(
+        std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(),
+        r#"JJ: Enter a description for the second commit.
+Add file1 & file2
+
+JJ: This commit contains the following changes:
+JJ:     A file2
+
+JJ: Lines starting with "JJ: " (like this one) will be removed.
+"#
+    );
+
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    ◉  kkmpptxzrspx false Add file4
+    ◉    rlvkpnrzqnoo false Add file3
+    ├─╮
+    │ @  yqosqzytrlsw false Add file2
+    ◉ │  qpvuntsmwlqt false Add file1
+    ├─╯
+    ◉  zzzzzzzzzzzz true
+    "###);
 }