diff --git a/CHANGELOG.md b/CHANGELOG.md index f8b4e7fa8dd..43ca8720b3a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 01465f5ac5f..401f494b3ea 100644 --- a/cli/src/commands/split.rs +++ b/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, @@ -70,22 +77,18 @@ pub(crate) fn cmd_split( let mut tx = workspace_command.start_transaction(); let end_tree = commit.tree()?; let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?; - let instructions = format!( - "\ -You are splitting a commit in two: {} + let instructions = "\ +You are splitting a commit in two commits. 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. -", - tx.format_commit_summary(&commit) - ); - +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. +"; // Prompt the user to select the changes they want for the first commit. let selected_tree_id = - diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), Some(&instructions))?; + diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), Some(instructions))?; if &selected_tree_id == commit.tree_id() && diff_selector.is_interactive() { // The user selected everything from the original commit. writeln!(ui.status(), "Nothing changed.")?; @@ -106,7 +109,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 +122,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 +145,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 = 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")?; diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index f73c3fea4ce..4957f73fb41 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/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 ` — The revision to split Default value: `@` +* `-s`, `--siblings` — Split the revision into two siblings instead of a parent and child + + Possible values: `true`, `false` + diff --git a/cli/tests/test_split_command.rs b/cli/tests/test_split_command.rs index 03799c42659..f091914bba7 100644 --- a/cli/tests/test_split_command.rs +++ b/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 + "###); }