Skip to content

Commit

Permalink
Add a --siblings option to the jj split command
Browse files Browse the repository at this point in the history 
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 sibilings as their new parents.


ISSUE=#2274
  • Loading branch information
@emesterhazy
emesterhazy committed Mar 31, 2024
1 parent ffc7311 commit 71b65e5
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 22 deletions.
69 changes: 57 additions & 12 deletions cli/src/commands/split.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -77,12 +84,13 @@ You are splitting a commit in 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
{} commit. The remainder will be in the {} commit. If you
don't make any changes, then the operation will be aborted.
",
tx.format_commit_summary(&commit)
tx.format_commit_summary(&commit),
if args.siblings { "" } else { "(parent)" },
if args.siblings { "sibling" } else { "child" }
);

// 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))?;
Expand All @@ -106,7 +114,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,
Expand All @@ -120,6 +128,20 @@ don't make any changes, then the operation will be aborted.
.write()?;

// Create the second commit, which includes everything the user didn't select.
// TODO(emesterhazy): Maybe there's a prettier way to write this or we could
// move the commit creation code to helper functions.
let (second_tree, second_base_tree, second_commit_parents) = 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,
commit.parent_ids().to_vec(),
)
} else {
(end_tree, &selected_tree, 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()
Expand All @@ -128,30 +150,53 @@ 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.
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 num_rebased > 0 {
writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?;
}

write!(ui.stderr(), "First part: ")?;
tx.write_commit_summary(ui.stderr_formatter().as_mut(), &first_commit)?;
write!(ui.stderr(), "\nSecond part: ")?;
Expand Down
4 changes: 4 additions & 0 deletions cli/tests/[email protected]
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
143 changes: 133 additions & 10 deletions cli/tests/test_split_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
"###);
}

0 comments on commit 71b65e5

Please sign in to comment.