From e0385ce677e038c7657098aace530b60cad2903e Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Sun, 14 Jul 2024 18:20:30 -0700 Subject: [PATCH] cli `split`: add experimental --restore-from AKA --from argument This is useful when you accidentally put some changes in the wrong commit. In the future, we could add some shortcuts for common uses. For example, we could define `current(X)` to be the current revision with the same change id as `X` (which would usually be a hidden commit) and have a shortcut for `jj split --from X -r current(X)` (only valid if `current(X)` is one commit). Or, we could have a similar operation for `obscurrent(X)`, defined as `obsheads(obsdescendants(X))` , based on the `jj obslog` graph (see also #4129 for a more focused discussion about implementing such operation). However, let's have the basic operation first. It should be useful with the default value of `-r @`. --- cli/src/commands/split.rs | 52 ++++- cli/tests/cli-reference@.md.snap | 9 +- cli/tests/test_split_command.rs | 339 +++++++++++++++++++++++++++++++ docs/FAQ.md | 6 +- 4 files changed, 396 insertions(+), 10 deletions(-) diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 2cb8908093..82fa06b915 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -45,7 +45,7 @@ use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] pub(crate) struct SplitArgs { /// Interactively choose which parts to split. This is the default if no - /// paths are provided. + /// paths are provided and `--from` is not used. #[arg(long, short)] interactive: bool, /// Specify diff editor to be used (implies --interactive) @@ -54,6 +54,31 @@ pub(crate) struct SplitArgs { /// The revision to split #[arg(long, short, default_value = "@")] revision: RevisionArg, + /// The revision to copy as the first part of the split (Experimental) + /// + /// This option's behavior may change in the future. Experience reports and + /// feedback are appreciated. + /// + /// With this option, the first part of the split will contain the changes + /// between the parent of the REVISION and the FROM revision. The second + /// part of the split will contain the changes between the FROM revision and + /// the REVISION revision. + /// + /// This is especially useful if the FROM revision is a past version of + /// REVISION, with its commit id obtained via `jj obslog` or `jj log + /// --at-operation`. + // + // TODO(ilyagr): We could allow `--interactive --from`. It's unclear how + // useful that would be. It would mostly require writing tests and + // JJ-INSTRUCTIONS. More ambitiously, we could have a 3-pane interactive view + // with the FROM commit in the middle and the REVISION commit on the RHS. + #[arg( + long, + conflicts_with = "interactive", + visible_alias = "from", + value_name = "FROM" + )] + restore_from: Option, /// Split the revision into two parallel revisions instead of a parent and /// child. // TODO: Delete `--siblings` alias in jj 0.25+ @@ -78,6 +103,11 @@ pub(crate) fn cmd_split( "Use `jj new` if you want to create another empty commit.", )); } + let from_revision = args + .restore_from + .as_ref() + .map(|revstr| workspace_command.resolve_single_rev(ui, revstr)) + .transpose()?; workspace_command.check_rewritable([commit.id()])?; let matcher = workspace_command @@ -86,11 +116,12 @@ pub(crate) fn cmd_split( let diff_selector = workspace_command.diff_selector( ui, args.tool.as_deref(), - args.interactive || args.paths.is_empty(), + args.interactive || (args.paths.is_empty() && args.restore_from.is_none()), )?; let mut tx = workspace_command.start_transaction(); let end_tree = commit.tree()?; let base_tree = commit.parent_tree(tx.repo())?; + // Note: --from --interactive is currently forbidden, ensured by `clap` let format_instructions = || { format!( "\ @@ -105,9 +136,15 @@ The remainder will be in the second commit. ) }; - // 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(), format_instructions)?; + // Figure out what changes should go into the first commit (possibly + // interactively) + let from_revision_tree = from_revision.as_ref().map(|rev| rev.tree()).transpose()?; + let selected_tree_id = diff_selector.select( + &base_tree, + from_revision_tree.as_ref().unwrap_or(&end_tree), + matcher.as_ref(), + format_instructions, + )?; if &selected_tree_id == commit.tree_id() { // The user selected everything from the original commit. writeln!( @@ -130,6 +167,11 @@ The remainder will be in the second commit. .rewrite_commit(command.settings(), &commit) .detach(); commit_builder.set_tree_id(selected_tree_id); + // TODO(ilyagr): When --from is used, we could show either both descriptions or + // one of the descriptions and a diff. + if let Some(from_revision) = from_revision { + commit_builder.set_description(from_revision.description()); + }; if commit_builder.description().is_empty() { commit_builder.set_description(command.settings().default_description()); } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 07d888757c..c1f6506858 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1903,11 +1903,18 @@ Splitting an empty commit is not supported because the same effect can be achiev ###### **Options:** -* `-i`, `--interactive` — Interactively choose which parts to split. This is the default if no paths are provided +* `-i`, `--interactive` — Interactively choose which parts to split. This is the default if no paths are provided and `--from` is not used * `--tool ` — Specify diff editor to be used (implies --interactive) * `-r`, `--revision ` — The revision to split Default value: `@` +* `--restore-from ` — The revision to copy as the first part of the split (Experimental) + + This option's behavior may change in the future. Experience reports and feedback are appreciated. + + With this option, the first part of the split will contain the changes between the parent of the REVISION and the FROM revision. The second part of the split will contain the changes between the FROM revision and the REVISION revision. + + This is especially useful if the FROM revision is a past version of REVISION, with its commit id obtained via `jj obslog` or `jj log --at-operation`. * `-p`, `--parallel` — Split the revision into two parallel revisions instead of a parent and child diff --git a/cli/tests/test_split_command.rs b/cli/tests/test_split_command.rs index c5f3318873..c0828dec68 100644 --- a/cli/tests/test_split_command.rs +++ b/cli/tests/test_split_command.rs @@ -22,6 +22,11 @@ fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { test_env.jj_cmd_success(cwd, &["log", "-T", template]) } +fn get_log_output_with_patch(test_env: &TestEnvironment, cwd: &Path) -> String { + let template = r#"separate(" ", change_id.short(), empty, description, local_bookmarks)"#; + test_env.jj_cmd_success(cwd, &["log", "-p", "--git", "-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]) @@ -522,6 +527,340 @@ fn test_split_siblings_with_merge_child() { "###); } +#[test] +fn test_split_from_one_file() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=older"]); + std::fs::write(workspace_path.join("file1"), "older\n").unwrap(); + std::fs::write(workspace_path.join("file2"), "older\n").unwrap(); + test_env.jj_cmd_ok(&workspace_path, &["new", "root()", "-m=base"]); + std::fs::write(workspace_path.join("file1"), "base\n").unwrap(); + std::fs::write(workspace_path.join("file2"), "base\n").unwrap(); + test_env.jj_cmd_ok(&workspace_path, &["new", "-m=pre-split"]); + std::fs::write(workspace_path.join("file1"), "newer\n").unwrap(); + std::fs::write(workspace_path.join("file2"), "newer\n").unwrap(); + + // In many use-cases, the `older` commit will be an older version of the + // `pre-split` commit and might be hidden. + insta::assert_snapshot!(get_log_output_with_patch(&test_env, &workspace_path), @r###" + @ zsuskulnrvyr false pre-split + │ diff --git a/file1 b/file1 + │ index df967b96a5..d58ed19c91 100644 + │ --- a/file1 + │ +++ b/file1 + │ @@ -1,1 +1,1 @@ + │ -base + │ +newer + │ diff --git a/file2 b/file2 + │ index df967b96a5..d58ed19c91 100644 + │ --- a/file2 + │ +++ b/file2 + │ @@ -1,1 +1,1 @@ + │ -base + │ +newer + ○ kkmpptxzrspx false base + │ diff --git a/file1 b/file1 + │ new file mode 100644 + │ index 0000000000..df967b96a5 + │ --- /dev/null + │ +++ b/file1 + │ @@ -1,0 +1,1 @@ + │ +base + │ diff --git a/file2 b/file2 + │ new file mode 100644 + │ index 0000000000..df967b96a5 + │ --- /dev/null + │ +++ b/file2 + │ @@ -1,0 +1,1 @@ + │ +base + │ ○ qpvuntsmwlqt false older + ├─╯ diff --git a/file1 b/file1 + │ new file mode 100644 + │ index 0000000000..b074d105d8 + │ --- /dev/null + │ +++ b/file1 + │ @@ -1,0 +1,1 @@ + │ +older + │ diff --git a/file2 b/file2 + │ new file mode 100644 + │ index 0000000000..b074d105d8 + │ --- /dev/null + │ +++ b/file2 + │ @@ -1,0 +1,1 @@ + │ +older + ◆ zzzzzzzzzzzz true + "###); + + let edit_script = test_env.set_up_fake_editor(); + std::fs::write( + edit_script, + [ + // Invocations for test 2 + "dump editor1", + "write\npart 1", + "next invocation\n", + "dump editor2", + "write\npart 2", + "next invocation\n", + // Repeat the same invocations for test 3 + "dump editor1", + "write\npart 1", + "next invocation\n", + "dump editor2", + "write\npart 2", + ] + .join("\0"), + ) + .unwrap(); + + // SETUP ENDS HERE + let setup_operation_id = test_env.current_operation_id(&workspace_path); + + // Test 1: --from --interactive is not (yet) implemented, see TODOs in split.rs + let stderr = test_env.jj_cmd_cli_error( + &workspace_path, + &["split", "--from=description(older)", "--interactive"], + ); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--restore-from ' cannot be used with '--interactive' + + Usage: jj split --restore-from [PATHS]... + + For more information, try '--help'. + "###); + + // Test 2: --from without a file argument (restores the entire older commit + // non-interactively) + let (stdout, stderr) = + test_env.jj_cmd_ok(&workspace_path, &["split", "--from=description(older)"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + First part: zsuskuln b20629fa part 1 + Second part: vruxwmqv d4b39657 part 2 + Working copy now at: vruxwmqv d4b39657 part 2 + Parent commit : zsuskuln b20629fa part 1 + "###); + + // The files are listed as modified, not added, since they are modified relative + // to the parent of the commit being split. + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(), @r###" + JJ: Enter a description for the first commit. + older + + JJ: This commit contains the following changes: + JJ: M file1 + JJ: M file2 + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###" + JJ: Enter a description for the second commit. + pre-split + + JJ: This commit contains the following changes: + JJ: M file1 + JJ: M file2 + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + + // The new parent commit is identical to the `older` commit, as after `jj + // restore --from older`, but its description was edited successfully + let stdout = test_env.jj_cmd_success(&workspace_path, &["show", "--git", "-r", "@-"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: b20629fa59453e41cb28d1a09c3954d29767e900 + Change ID: zsuskulnrvyrovkzqrwmxqlsskqntxvp + Author: Test User (2001-02-03 08:05:10) + Committer: Test User (2001-02-03 08:05:14) + + part 1 + + diff --git a/file1 b/file1 + index df967b96a5..b074d105d8 100644 + --- a/file1 + +++ b/file1 + @@ -1,1 +1,1 @@ + -base + +older + diff --git a/file2 b/file2 + index df967b96a5..b074d105d8 100644 + --- a/file2 + +++ b/file2 + @@ -1,1 +1,1 @@ + -base + +older + "###); + // The new child commit is the same as the pre-split commit as a snapshot + let stdout = test_env.jj_cmd_success(&workspace_path, &["show", "--git", "-r", "@"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: d4b396576002b46b435b260dd30473675bd74d8a + Change ID: vruxwmqvtpmxqkrrksmzyrvxysqqlsxp + Author: Test User (2001-02-03 08:05:10) + Committer: Test User (2001-02-03 08:05:14) + + part 2 + + diff --git a/file1 b/file1 + index b074d105d8..d58ed19c91 100644 + --- a/file1 + +++ b/file1 + @@ -1,1 +1,1 @@ + -older + +newer + diff --git a/file2 b/file2 + index b074d105d8..d58ed19c91 100644 + --- a/file2 + +++ b/file2 + @@ -1,1 +1,1 @@ + -older + +newer + "###); + + // Test 3: --from with a file argument, preliminaries + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_path, + &["operation", "restore", &setup_operation_id], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Restored to operation f6864a0edc14 2001-02-03 04:05:11.000 +07:00 - 2001-02-03 04:05:11.000 +07:00 snapshot working copy + Working copy now at: zsuskuln 0ca028a6 pre-split + Parent commit : kkmpptxz e4a752c8 base + "#); + // Reminder of the setup + insta::assert_snapshot!(get_log_output_with_patch(&test_env, &workspace_path), @r###" + @ zsuskulnrvyr false pre-split + │ diff --git a/file1 b/file1 + │ index df967b96a5..d58ed19c91 100644 + │ --- a/file1 + │ +++ b/file1 + │ @@ -1,1 +1,1 @@ + │ -base + │ +newer + │ diff --git a/file2 b/file2 + │ index df967b96a5..d58ed19c91 100644 + │ --- a/file2 + │ +++ b/file2 + │ @@ -1,1 +1,1 @@ + │ -base + │ +newer + ○ kkmpptxzrspx false base + │ diff --git a/file1 b/file1 + │ new file mode 100644 + │ index 0000000000..df967b96a5 + │ --- /dev/null + │ +++ b/file1 + │ @@ -1,0 +1,1 @@ + │ +base + │ diff --git a/file2 b/file2 + │ new file mode 100644 + │ index 0000000000..df967b96a5 + │ --- /dev/null + │ +++ b/file2 + │ @@ -1,0 +1,1 @@ + │ +base + │ ○ qpvuntsmwlqt false older + ├─╯ diff --git a/file1 b/file1 + │ new file mode 100644 + │ index 0000000000..b074d105d8 + │ --- /dev/null + │ +++ b/file1 + │ @@ -1,0 +1,1 @@ + │ +older + │ diff --git a/file2 b/file2 + │ new file mode 100644 + │ index 0000000000..b074d105d8 + │ --- /dev/null + │ +++ b/file2 + │ @@ -1,0 +1,1 @@ + │ +older + ◆ zzzzzzzzzzzz true + "###); + + // Test 3: --from with a file argument + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_path, + &["split", "--from=description(older)", "file1"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + First part: zsuskuln dc949368 part 1 + Second part: wqnwkozp f22c2886 part 2 + Working copy now at: wqnwkozp f22c2886 part 2 + Parent commit : zsuskuln dc949368 part 1 + "###); + + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor1")).unwrap(), @r###" + JJ: Enter a description for the first commit. + older + + JJ: This commit contains the following changes: + JJ: M file1 + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###" + JJ: Enter a description for the second commit. + pre-split + + JJ: This commit contains the following changes: + JJ: M file1 + JJ: M file2 + + JJ: Lines starting with "JJ: " (like this one) will be removed. + "###); + + // The new parent commit restored one file from the `older` commit + let stdout = test_env.jj_cmd_success(&workspace_path, &["show", "--git", "-r", "@-"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: dc9493680b1791877e47cf7186bd8e1d1b1e7964 + Change ID: zsuskulnrvyrovkzqrwmxqlsskqntxvp + Author: Test User (2001-02-03 08:05:10) + Committer: Test User (2001-02-03 08:05:19) + + part 1 + + diff --git a/file1 b/file1 + index df967b96a5..b074d105d8 100644 + --- a/file1 + +++ b/file1 + @@ -1,1 +1,1 @@ + -base + +older + "###); + // The new child commit is the same as the pre-split commit as a snapshot + let stdout = test_env.jj_cmd_success(&workspace_path, &["show", "--git", "-r", "@"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: f22c2886b7c96ccedeec11f6f1321f38303edb57 + Change ID: wqnwkozpkustnxypnnntnykwrqrkrpvv + Author: Test User (2001-02-03 08:05:10) + Committer: Test User (2001-02-03 08:05:19) + + part 2 + + diff --git a/file1 b/file1 + index b074d105d8..d58ed19c91 100644 + --- a/file1 + +++ b/file1 + @@ -1,1 +1,1 @@ + -older + +newer + diff --git a/file2 b/file2 + index df967b96a5..d58ed19c91 100644 + --- a/file2 + +++ b/file2 + @@ -1,1 +1,1 @@ + -base + +newer + "###); +} + // Make sure `jj split` would refuse to split an empty commit. #[test] fn test_split_empty() { diff --git a/docs/FAQ.md b/docs/FAQ.md index 4d290a3e5f..752f4b32f7 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -243,10 +243,8 @@ Use `jj evolog -p` to see how your working-copy commit has evolved. Find the commit you want to restore the contents to. Let's say the current commit (with the changes intended for a new commit) are in commit X and the state you wanted is in commit Y. Note the commit id (normally in blue at the end of the line in -the log output) of each of them. Now use `jj new` to create a new working-copy -commit, then run `jj restore --from Y --to @-` to restore the parent commit -to the old state, and `jj restore --from X` to restore the new working-copy -commit to the new state. +the log output) of each of them. Now use `jj split --restore-from Y` to split +the current commit into its old version and the changes since then. ### How do I resume working on an existing change?