From 985c1f0d44680fba30d3545ffba9514d8020dc0c Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Wed, 7 Feb 2024 20:37:07 -0500 Subject: [PATCH] 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 sibilings as their new parents. ISSUE=#2274 --- CHANGELOG.md | 113 ++++++++++++------------ cli/src/commands/split.rs | 82 +++++++++++++----- cli/tests/cli-reference@.md.snap | 4 + cli/tests/test_split_command.rs | 143 ++++++++++++++++++++++++++++--- 4 files changed, 252 insertions(+), 90 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a45ad9c24b..4409dc9c70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,14 +24,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### New features -* Config now supports rgb hex colors (in the form `#rrggbb`) wherever existing color names are supported. +* Config now supports rgb hex colors (in the form `#rrggbb`) wherever existing color names are + supported. * `ui.default-command` now accepts multiple string arguments, for more complex default `jj` commands. * Graph node symbols are now configurable via templates - * `templates.log_node` - * `templates.op_log_node` + * `templates.log_node` + * `templates.op_log_node` * `jj log` now includes synthetic nodes in the graph where some revisions were elided. @@ -54,6 +55,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj duplicate` and `jj abandon` can now take more than a single `-r` argument, for consistency with other commands. +* `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 @@ -91,8 +95,8 @@ No code changes (fixing Rust `Cargo.toml` stuff). commits when they are created. This comes with out-of-the-box support for the following backends: - * GnuPG - * SSH + * GnuPG + * SSH Signature verification and an explicit sign command will hopefully come soon. @@ -138,7 +142,7 @@ No code changes (fixing Rust `Cargo.toml` stuff). * `jj commit`/`diffedit`/`move`/`resolve`/`split`/`squash`/`unsquash` now accept `--tool=` option to override the default. - [#2575](https://github.com/martinvonz/jj/issues/2575) + [#2575](https://github.com/martinvonz/jj/issues/2575) * Added completions for [Nushell](https://nushell.sh) to `jj util completion` @@ -158,7 +162,7 @@ No code changes (fixing Rust `Cargo.toml` stuff). When symlink support is unavailable, they will be materialized as regular files in the working copy (instead of resulting in a crash). [#2](https://github.com/martinvonz/jj/issues/2) - + * On Windows, the `:builtin` pager is now used by default, rather than being disabled entirely. @@ -192,7 +196,6 @@ Thanks to the people who made this release happen! * Vladimir (@0xdeafbeef) * Yuya Nishihara (@yuja) - ## [0.14.0] - 2024-02-07 ### Deprecations @@ -205,7 +208,7 @@ Thanks to the people who made this release happen! copy commit on top of a single specified revision, i.e. with one parent. `merge` creates a new working copy commit on top of *at least* two specified revisions, i.e. with two or more parents. - + The only difference between these commands and `jj new`, which *also* creates a new working copy commit, is that `new` can create a working copy commit on top of any arbitrary number of revisions, so it can handle both the previous @@ -226,7 +229,6 @@ Thanks to the people who made this release happen! Use `jj git init` instead. - ### Breaking changes * (Minor) Diff summaries (e.g. `jj diff -s`) now use `D` for "Deleted" instead @@ -318,7 +320,6 @@ Thanks to the people who made this release happen! * vwkd (@vwkd) * Yuya Nishihara (@yuja) - ## [0.13.0] - 2024-01-03 ### Breaking changes @@ -352,7 +353,6 @@ Thanks to the people who made this release happen! * Waleed Khan (@arxanas) * Yuya Nishihara (@yuja) - ## [0.12.0] - 2023-12-05 ### Breaking changes @@ -362,7 +362,7 @@ Thanks to the people who made this release happen! * `jj branch set` no longer creates a new branch. Use `jj branch create` instead. - + * `jj init --git` in an existing Git repository now errors and exits rather than creating a second Git store. @@ -413,7 +413,6 @@ Thanks to the people who made this release happen! * Waleed Khan (@arxanas) * Yuya Nishihara (@yuja) - ## [0.11.0] - 2023-11-01 ### Breaking changes @@ -437,9 +436,9 @@ Thanks to the people who made this release happen! `git.auto-local-branch` setting is applied only to newly fetched remote branches. Existing remote branches are migrated as follows: - * If local branch exists, the corresponding remote branches are considered - tracking branches. - * Otherwise, the remote branches are non-tracking branches. + * If local branch exists, the corresponding remote branches are considered + tracking branches. + * Otherwise, the remote branches are non-tracking branches. If the deduced tracking flags are wrong, use `jj branch track`/`untrack` commands to fix them up. @@ -461,7 +460,8 @@ Thanks to the people who made this release happen! ### New features -* `jj`'s stable release can now be installed with [`cargo binstall jj-cli`](https://github.com/cargo-bins/cargo-binstall). +* `jj`'s stable release can now be installed + with [`cargo binstall jj-cli`](https://github.com/cargo-bins/cargo-binstall). * `jj workspace add` now takes a `--revision` argument. @@ -514,7 +514,6 @@ Thanks to the people who made this release happen! * Waleed Khan (@arxanas) * Yuya Nishihara (@yuja) - ## [0.10.0] - 2023-10-04 ### Breaking changes @@ -526,8 +525,8 @@ Thanks to the people who made this release happen! ### New features -* The `ancestors()` revset function now takes an optional `depth` argument - to limit the depth of the ancestor set. For example, use `jj log -r +* The `ancestors()` revset function now takes an optional `depth` argument + to limit the depth of the ancestor set. For example, use `jj log -r 'ancestors(@, 5)` to view the last 5 commits. * Support for the Watchman filesystem monitor is now bundled by default. Set @@ -577,7 +576,6 @@ Thanks to the people who made this release happen! * Yuya Nishihara (@yuja) * Zachary Dremann (@Dr-Emann) - ## [0.9.0] - 2023-09-06 ### Breaking changes @@ -692,13 +690,13 @@ Thanks to the people who made this release happen! respectively. * `jj log` timestamp format now accepts `.utc()` to convert a timestamp to UTC. - + * templates now support additional string methods `.starts_with(x)`, `.ends_with(x)` `.remove_prefix(x)`, `.remove_suffix(x)`, and `.substr(start, end)`. * `jj next` and `jj prev` are added, these allow you to traverse the history in a linear style. For people coming from Sapling and `git-branchles` - see [#2126](https://github.com/martinvonz/jj/issues/2126) for + see [#2126](https://github.com/martinvonz/jj/issues/2126) for further pending improvements. * `jj diff --stat` has been implemented. It shows a histogram of the changes, @@ -754,7 +752,6 @@ Thanks to the people who made this release happen! * Yuya Nishihara (@yuja) * Zachary Dremann (@Dr-Emann) - ## [0.8.0] - 2023-07-09 ### Breaking changes @@ -955,7 +952,6 @@ Thanks to the people who made this release happen! * Waleed Khan (@arxanas) * Yuya Nishihara (@yuja) - ## [0.7.0] - 2023-02-16 ### Breaking changes @@ -1151,25 +1147,25 @@ Thanks to the people who made this release happen! Thanks to the people who made this release happen! - * Aleksandr Mikhailov (@AM5800) - * Augie Fackler (@durin42) - * Benjamin Saunders (@Ralith) - * Daniel Ploch (@torquestomp) - * Danny Hooper (@hooper) - * David Barnett (@dbarnett) - * Glen Choo (@chooglen) - * Herby Gillot (@herbygillot) - * Ilya Grigoriev (@ilyagr) - * Luke Granger-Brown (@lukegb) - * Martin von Zweigbergk (@martinvonz) - * Michael Forster (@MForster) - * Philip Metzger (@PhilipMetzger) - * Ruben Slabbert (@rslabbert) - * Samuel Tardieu (@samueltardieu) - * Tal Pressman (@talpr) - * Vamsi Avula (@avamsi) - * Waleed Khan (@arxanas) - * Yuya Nishihara (@yuja) +* Aleksandr Mikhailov (@AM5800) +* Augie Fackler (@durin42) +* Benjamin Saunders (@Ralith) +* Daniel Ploch (@torquestomp) +* Danny Hooper (@hooper) +* David Barnett (@dbarnett) +* Glen Choo (@chooglen) +* Herby Gillot (@herbygillot) +* Ilya Grigoriev (@ilyagr) +* Luke Granger-Brown (@lukegb) +* Martin von Zweigbergk (@martinvonz) +* Michael Forster (@MForster) +* Philip Metzger (@PhilipMetzger) +* Ruben Slabbert (@rslabbert) +* Samuel Tardieu (@samueltardieu) +* Tal Pressman (@talpr) +* Vamsi Avula (@avamsi) +* Waleed Khan (@arxanas) +* Yuya Nishihara (@yuja) ## [0.6.1] - 2022-12-05 @@ -1294,17 +1290,16 @@ No changes, only changed to a released version of the `thrift` crate dependency. Thanks to the people who made this release happen! - * Martin von Zweigbergk (@martinvonz) - * Benjamin Saunders (@Ralith) - * Yuya Nishihara (@yuja) - * Glen Choo (@chooglen) - * Ilya Grigoriev (@ilyagr) - * Ruben Slabbert (@rslabbert) - * Waleed Khan (@arxanas) - * Sean E. Russell (@xxxserxxx) - * Pranay Sashank (@pranaysashank) - * Luke Granger-Brown (@lukegb) - +* Martin von Zweigbergk (@martinvonz) +* Benjamin Saunders (@Ralith) +* Yuya Nishihara (@yuja) +* Glen Choo (@chooglen) +* Ilya Grigoriev (@ilyagr) +* Ruben Slabbert (@rslabbert) +* Waleed Khan (@arxanas) +* Sean E. Russell (@xxxserxxx) +* Pranay Sashank (@pranaysashank) +* Luke Granger-Brown (@lukegb) ## [0.5.1] - 2022-10-17 @@ -1569,9 +1564,9 @@ No changes, only trying to get the automated build to work. ### Fixed bugs - - Fixed crash when `core.excludesFile` pointed to nonexistent file, and made - leading `~/` in that config expand to `$HOME/` - [#131](https://github.com/martinvonz/jj/issues/131) +- Fixed crash when `core.excludesFile` pointed to nonexistent file, and made + leading `~/` in that config expand to `$HOME/` + [#131](https://github.com/martinvonz/jj/issues/131) ## [0.3.0] - 2022-03-12 diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 89e8cf3673..f8ee2c00b2 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.stderr(), "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,30 +145,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 = 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: ")?; diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index dc7d797e9f..05d6746903 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 03799c4265..f091914bba 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 + "###); }