From 2cb3ee5260b6acec6655953500bb49d5d1b07a65 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 4 Jan 2025 15:53:24 +0900 Subject: [PATCH] cli: restore: add --interactive flag As Martin suggested, this is the reverse operation of "jj diffedit", and supports fileset arguments. https://github.com/jj-vcs/jj/issues/3012#issuecomment-1937353458 Closes #3012 --- CHANGELOG.md | 2 + cli/src/commands/restore.rs | 39 ++++- cli/tests/cli-reference@.md.snap | 2 + cli/tests/test_restore_command.rs | 239 ++++++++++++++++++++++++++++++ 4 files changed, 276 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 559769a77..07d1f4b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj evolog` now accepts `--reversed`. +* `jj restore` now supports `-i`/`--interactive` selection. + * Add templater support for rendering commit signatures. ### Fixed bugs diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index 0f00d14e2..1fe33549a 100644 --- a/cli/src/commands/restore.rs +++ b/cli/src/commands/restore.rs @@ -16,8 +16,9 @@ use std::io::Write; use clap_complete::ArgValueCandidates; use clap_complete::ArgValueCompleter; +use indoc::formatdoc; +use itertools::Itertools as _; use jj_lib::object_id::ObjectId; -use jj_lib::rewrite::restore_tree; use tracing::instrument; use crate::cli_util::CommandHelper; @@ -92,6 +93,12 @@ pub(crate) struct RestoreArgs { /// the user might not even realize something went wrong. #[arg(long, short, hide = true)] revision: Option, + /// Interactively choose which parts to restore + #[arg(long, short)] + interactive: bool, + /// Specify diff editor to be used (implies --interactive) + #[arg(long, value_name = "NAME")] + tool: Option, /// Preserve the content (not the diff) when rebasing descendants #[arg(long)] restore_descendants: bool, @@ -104,7 +111,7 @@ pub(crate) fn cmd_restore( args: &RestoreArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let (from_tree, to_commit); + let (from_commits, from_tree, to_commit); if args.revision.is_some() { return Err(user_error( "`jj restore` does not have a `--revision`/`-r` option. If you'd like to modify\nthe \ @@ -115,21 +122,41 @@ pub(crate) fn cmd_restore( if args.from.is_some() || args.to.is_some() { to_commit = workspace_command .resolve_single_rev(ui, args.to.as_ref().unwrap_or(&RevisionArg::AT))?; - from_tree = workspace_command - .resolve_single_rev(ui, args.from.as_ref().unwrap_or(&RevisionArg::AT))? - .tree()?; + let from_commit = workspace_command + .resolve_single_rev(ui, args.from.as_ref().unwrap_or(&RevisionArg::AT))?; + from_tree = from_commit.tree()?; + from_commits = vec![from_commit]; } else { to_commit = workspace_command .resolve_single_rev(ui, args.changes_in.as_ref().unwrap_or(&RevisionArg::AT))?; from_tree = to_commit.parent_tree(workspace_command.repo().as_ref())?; + from_commits = to_commit.parents().try_collect()?; } workspace_command.check_rewritable([to_commit.id()])?; let matcher = workspace_command .parse_file_patterns(ui, &args.paths)? .to_matcher(); + let diff_selector = + workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?; let to_tree = to_commit.tree()?; - let new_tree_id = restore_tree(&from_tree, &to_tree, matcher.as_ref())?; + let format_instructions = || { + formatdoc! {" + You are restoring changes from: {from_commits} + to commit: {to_commit} + + The diff initially shows all changes restored. Adjust the right side until it + shows the contents you want for the destination commit. + ", + from_commits = from_commits + .iter() + .map(|commit| workspace_command.format_commit_summary(commit)) + // "You are restoring changes from: " + .join("\n "), + to_commit = workspace_command.format_commit_summary(&to_commit), + } + }; + let new_tree_id = diff_selector.select(&to_tree, &from_tree, &matcher, format_instructions)?; if &new_tree_id == to_commit.tree_id() { writeln!(ui.status(), "Nothing changed.")?; } else { diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 99b7b571d..47a060f66 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1934,6 +1934,8 @@ See `jj diffedit` if you'd like to restore portions of files rather than entire This undoes the changes that can be seen with `jj diff -r REVSET`. If `REVSET` only has a single parent, this option is equivalent to `jj restore --to REVSET --from REVSET-`. The default behavior of `jj restore` is equivalent to `jj restore --changes-in @`. +* `-i`, `--interactive` — Interactively choose which parts to restore +* `--tool ` — Specify diff editor to be used (implies --interactive) * `--restore-descendants` — Preserve the content (not the diff) when rebasing descendants diff --git a/cli/tests/test_restore_command.rs b/cli/tests/test_restore_command.rs index 7f496cf43..d044592b7 100644 --- a/cli/tests/test_restore_command.rs +++ b/cli/tests/test_restore_command.rs @@ -342,6 +342,245 @@ fn test_restore_restore_descendants() { "#); } +#[test] +fn test_restore_interactive() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + create_commit( + &test_env, + &repo_path, + "a", + &[], + &[("file1", "a1\n"), ("file2", "a2\n")], + ); + create_commit( + &test_env, + &repo_path, + "b", + &["a"], + &[("file1", "b1\n"), ("file2", "b2\n"), ("file3", "b3\n")], + ); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ zsuskuln test.user@example.com 2001-02-03 08:05:11 b c0745ce2 + │ b + │ M file1 + │ M file2 + │ A file3 + ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 186caaef + │ a + │ A file1 + │ A file2 + ◆ zzzzzzzz root() 00000000 + "); + + let diff_editor = test_env.set_up_fake_diff_editor(); + let diff_script = [ + "files-before file1 file2 file3", + "files-after JJ-INSTRUCTIONS file1 file2", + "reset file2", + "dump JJ-INSTRUCTIONS instrs", + ] + .join("\0"); + std::fs::write(diff_editor, diff_script).unwrap(); + + // Restore file1 and file3 + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["restore", "-i", "--from=@-"]); + insta::assert_snapshot!(stderr, @r" + Created zsuskuln bccde490 b | b + Working copy now at: zsuskuln bccde490 b | b + Parent commit : rlvkpnrz 186caaef a | a + Added 0 files, modified 1 files, removed 1 files + "); + + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r" + You are restoring changes from: rlvkpnrz 186caaef a | a + to commit: zsuskuln c0745ce2 b | b + + The diff initially shows all changes restored. Adjust the right side until it + shows the contents you want for the destination commit. + "); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ zsuskuln test.user@example.com 2001-02-03 08:05:13 b bccde490 + │ b + │ M file2 + ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 186caaef + │ a + │ A file1 + │ A file2 + ◆ zzzzzzzz root() 00000000 + "); + + // Try again with --tool, which should imply --interactive + test_env.jj_cmd_ok(&repo_path, &["undo"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["restore", "--tool=fake-diff-editor"]); + insta::assert_snapshot!(stderr, @r" + Created zsuskuln 5921de19 b | b + Working copy now at: zsuskuln 5921de19 b | b + Parent commit : rlvkpnrz 186caaef a | a + Added 0 files, modified 1 files, removed 1 files + "); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ zsuskuln test.user@example.com 2001-02-03 08:05:16 b 5921de19 + │ b + │ M file2 + ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 186caaef + │ a + │ A file1 + │ A file2 + ◆ zzzzzzzz root() 00000000 + "); +} + +#[test] +fn test_restore_interactive_merge() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + create_commit(&test_env, &repo_path, "a", &[], &[("file1", "a1\n")]); + create_commit(&test_env, &repo_path, "b", &[], &[("file2", "b1\n")]); + create_commit( + &test_env, + &repo_path, + "c", + &["a", "b"], + &[("file1", "c1\n"), ("file2", "c2\n"), ("file3", "c3\n")], + ); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ royxmykx test.user@example.com 2001-02-03 08:05:13 c 34042291 + ├─╮ c + │ │ M file1 + │ │ M file2 + │ │ A file3 + │ ○ zsuskuln test.user@example.com 2001-02-03 08:05:11 b 29e70804 + │ │ b + │ │ A file2 + ○ │ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 79c1b823 + ├─╯ a + │ A file1 + ◆ zzzzzzzz root() 00000000 + "); + + let diff_editor = test_env.set_up_fake_diff_editor(); + let diff_script = [ + "files-before file1 file2 file3", + "files-after JJ-INSTRUCTIONS file1 file2", + "reset file2", + "dump JJ-INSTRUCTIONS instrs", + ] + .join("\0"); + std::fs::write(diff_editor, diff_script).unwrap(); + + // Restore file1 and file3 + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["restore", "-i"]); + insta::assert_snapshot!(stderr, @r" + Created royxmykx 72e0cbf4 c | c + Working copy now at: royxmykx 72e0cbf4 c | c + Parent commit : rlvkpnrz 79c1b823 a | a + Parent commit : zsuskuln 29e70804 b | b + Added 0 files, modified 1 files, removed 1 files + "); + + insta::assert_snapshot!( + std::fs::read_to_string(test_env.env_root().join("instrs")).unwrap(), @r" + You are restoring changes from: rlvkpnrz 79c1b823 a | a + zsuskuln 29e70804 b | b + to commit: royxmykx 34042291 c | c + + The diff initially shows all changes restored. Adjust the right side until it + shows the contents you want for the destination commit. + "); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ royxmykx test.user@example.com 2001-02-03 08:05:15 c 72e0cbf4 + ├─╮ c + │ │ M file2 + │ ○ zsuskuln test.user@example.com 2001-02-03 08:05:11 b 29e70804 + │ │ b + │ │ A file2 + ○ │ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 79c1b823 + ├─╯ a + │ A file1 + ◆ zzzzzzzz root() 00000000 + "); +} + +#[test] +fn test_restore_interactive_with_paths() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + create_commit( + &test_env, + &repo_path, + "a", + &[], + &[("file1", "a1\n"), ("file2", "a2\n")], + ); + create_commit( + &test_env, + &repo_path, + "b", + &["a"], + &[("file1", "b1\n"), ("file2", "b2\n"), ("file3", "b3\n")], + ); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ zsuskuln test.user@example.com 2001-02-03 08:05:11 b c0745ce2 + │ b + │ M file1 + │ M file2 + │ A file3 + ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 186caaef + │ a + │ A file1 + │ A file2 + ◆ zzzzzzzz root() 00000000 + "); + + let diff_editor = test_env.set_up_fake_diff_editor(); + let diff_script = [ + "files-before file1 file2", + "files-after JJ-INSTRUCTIONS file1 file2", + "reset file2", + ] + .join("\0"); + std::fs::write(diff_editor, diff_script).unwrap(); + + // Restore file1 (file2 is reset by interactive editor) + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["restore", "-i", "file1", "file2"]); + insta::assert_snapshot!(stderr, @r" + Created zsuskuln 7187da33 b | b + Working copy now at: zsuskuln 7187da33 b | b + Parent commit : rlvkpnrz 186caaef a | a + Added 0 files, modified 1 files, removed 0 files + "); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--summary"]); + insta::assert_snapshot!(stdout, @r" + @ zsuskuln test.user@example.com 2001-02-03 08:05:13 b 7187da33 + │ b + │ M file2 + │ A file3 + ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 a 186caaef + │ a + │ A file1 + │ A file2 + ◆ zzzzzzzz root() 00000000 + "); +} + fn create_commit( test_env: &TestEnvironment, repo_path: &Path,