Skip to content

Commit

Permalink
cli: teach jj move to move only changes to specified paths
Browse files Browse the repository at this point in the history
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
  • Loading branch information
martinvonz committed Apr 9, 2022
1 parent 80d54ea commit 27d5ceb
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 17 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* The new `jj print` command prints the contents of a file in a revision.

* `jj move` now lets you limit the set of changes to move by specifying paths
on the command line (in addition to the `--interactive` mode). For example,
use `jj move --to @-- foo` to move the changes to file (or directory) `foo` in
the working copy to the grandparent commit.

### Fixed bugs

* Errors are now printed to stderr (they used to be printed to stdout).
Expand Down
66 changes: 56 additions & 10 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,46 @@ impl WorkspaceCommandHelper {
)
}

fn select_diff(
&self,
ui: &Ui,
left_tree: &Tree,
right_tree: &Tree,
instructions: &str,
interactive: bool,
paths: &[String],
) -> Result<TreeId, CommandError> {
if interactive {
Ok(crate::diff_edit::edit_diff(
&self.settings,
left_tree,
right_tree,
instructions,
self.base_ignores(),
)?)
} else if paths.is_empty() {
// Optimization for a common case
Ok(right_tree.id().clone())
} else {
// TODO: It's probably better to have the caller pass in the matcher, but then
// we'll want to be able to check if it matches everything so we do
// the optimization above.
let matcher = matcher_from_values(ui, self.workspace_root(), paths)?;
let mut tree_builder = self.repo().store().tree_builder(left_tree.id().clone());
for (repo_path, diff) in left_tree.diff(right_tree, matcher.as_ref()) {
match diff.into_options().1 {
Some(value) => {
tree_builder.set(repo_path, value);
}
None => {
tree_builder.remove(repo_path);
}
}
}
Ok(tree_builder.write_tree())
}
}

fn start_transaction(&self, description: &str) -> Transaction {
let mut tx = self.repo.start_transaction(description);
// TODO: Either do better shell-escaping here or store the values in some list
Expand Down Expand Up @@ -1233,6 +1273,9 @@ struct MoveArgs {
/// Interactively choose which parts to move
#[clap(long, short)]
interactive: bool,
/// Move only changes to these paths (instead of all paths)
#[clap(conflicts_with = "interactive")]
paths: Vec<String>,
}

/// Move changes from a revision into its parent
Expand Down Expand Up @@ -3125,9 +3168,8 @@ fn cmd_move(ui: &mut Ui, command: &CommandHelper, args: &MoveArgs) -> Result<(),
let repo = workspace_command.repo();
let parent_tree = merge_commit_trees(repo.as_repo_ref(), &source.parents());
let source_tree = source.tree();
let new_parent_tree_id = if args.interactive {
let instructions = format!(
"\
let instructions = format!(
"\
You are moving changes from: {}
into commit: {}
Expand All @@ -3139,13 +3181,17 @@ Adjust the right side until the diff shows the changes you want to move
to the destination. If you don't make any changes, then all the changes
from the source will be moved into the destination.
",
short_commit_description(&source),
short_commit_description(&destination)
);
workspace_command.edit_diff(&parent_tree, &source_tree, &instructions)?
} else {
source_tree.id().clone()
};
short_commit_description(&source),
short_commit_description(&destination)
);
let new_parent_tree_id = workspace_command.select_diff(
ui,
&parent_tree,
&source_tree,
&instructions,
args.interactive,
&args.paths,
)?;
if &new_parent_tree_id == parent_tree.id() {
return Err(CommandError::UserError(String::from("No changes to move")));
}
Expand Down
42 changes: 35 additions & 7 deletions tests/test_move_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn test_move() {
}

#[test]
fn test_move_interactive() {
fn test_move_partial() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
Expand All @@ -156,9 +156,6 @@ fn test_move_interactive() {
// D B
// |/
// A
//
// When moving changes between e.g. C and F, we should not get unrelated changes
// from B and D.
test_env.jj_cmd_success(&repo_path, &["branch", "a"]);
std::fs::write(repo_path.join("file1"), "a\n").unwrap();
std::fs::write(repo_path.join("file2"), "a\n").unwrap();
Expand Down Expand Up @@ -188,7 +185,7 @@ fn test_move_interactive() {

let edit_script = test_env.set_up_fake_diff_editor();

// If we don't make any changes, the whole change is moved
// If we don't make any changes in the diff-editor, the whole change is moved
std::fs::write(&edit_script, "").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["move", "-i", "--from", "c"]);
insta::assert_snapshot!(stdout, @r###"
Expand All @@ -215,7 +212,7 @@ fn test_move_interactive() {
insta::assert_snapshot!(stdout, @"d
");

// Can move only part of the change
// Can move only part of the change in interactive mode
test_env.jj_cmd_success(&repo_path, &["undo"]);
std::fs::write(&edit_script, "reset file2").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["move", "-i", "--from", "c"]);
Expand All @@ -240,7 +237,38 @@ fn test_move_interactive() {
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file2"]);
insta::assert_snapshot!(stdout, @"a
");
// File `file2`, which was not changed in source, is unchanged
// File `file3`, which was changed in source's parent, is unchanged
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file3"]);
insta::assert_snapshot!(stdout, @"d
");

// Can move only part of the change in non-interactive mode
test_env.jj_cmd_success(&repo_path, &["undo"]);
// Clear the script so we know it won't be used
std::fs::write(&edit_script, "").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["move", "--from", "c", "file1"]);
insta::assert_snapshot!(stdout, @r###"
Working copy now at: 17c2e6632cc5
Added 0 files, modified 1 files, removed 0 files
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", template]);
insta::assert_snapshot!(stdout, @r###"
@ 17c2e6632cc5 d
| o 6a3ae047a03e c
| o 55171e33db26 b
|/
o 3db0a2f5b535 a
o 000000000000
"###);
// The selected change from the source has been applied
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file1"]);
insta::assert_snapshot!(stdout, @"c
");
// The unselected change from the source has not been applied
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file2"]);
insta::assert_snapshot!(stdout, @"a
");
// File `file3`, which was changed in source's parent, is unchanged
let stdout = test_env.jj_cmd_success(&repo_path, &["print", "file3"]);
insta::assert_snapshot!(stdout, @"d
");
Expand Down

0 comments on commit 27d5ceb

Please sign in to comment.