From cbbb91dcaf41805add3f51fd31a687c4bf8b7511 Mon Sep 17 00:00:00 2001 From: Philip Metzger Date: Sun, 22 Jan 2023 00:07:42 +0100 Subject: [PATCH] commands: Implement `next` and `prev` This is a naive implementation, which cannot deal with multiple children or parents stemming from merges. Note: I currently gave each command separate a separate argument struct for extensibility. Fixes #878 --- CHANGELOG.md | 4 + src/commands/mod.rs | 365 +++++++++++++++++++++++++++++++ tests/test_next_prev_commands.rs | 194 ++++++++++++++++ 3 files changed, 563 insertions(+) create mode 100644 tests/test_next_prev_commands.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 559f1fa5bc..76aa058624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -121,6 +121,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj branch list` can now be filtered by revset. +* `jj next` and `jj prev` are added, these allow you to traverse the history + in a linear style, see [#NNN](https://github.com/martinvonz/jj/issues/NNN) + for further pending improvements. + ### Fixed bugs * Modify/delete conflicts now include context lines diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 337c63e823..0e926a2156 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -106,10 +106,12 @@ enum Commands { Merge(NewArgs), Move(MoveArgs), New(NewArgs), + Next(NextArgs), Obslog(ObslogArgs), #[command(subcommand)] #[command(visible_alias = "op")] Operation(operation::OperationCommands), + Prev(PrevArgs), Rebase(RebaseArgs), Resolve(ResolveArgs), Restore(RestoreArgs), @@ -535,6 +537,81 @@ struct NewArgs { insert_before: bool, } +/// Move the current working copy commit to the next child revision in the +/// repository. +/// +/// +/// The command moves you to the next child in a linear fashion. +/// +/// +/// F F @ +/// | | / +/// C @ => C +/// | / | +/// B B +/// +/// +/// If `edit` is passed as an argument, it will move you directly to the child +/// revision. +/// +/// +/// F F +/// | | +/// C C +/// | | +/// B @ => @ +/// | / | +/// A A +// TODO(#NNN): Handle multiple child revisions properly. +#[derive(clap::Args, Clone, Debug)] +#[command(verbatim_doc_comment)] +struct NextArgs { + /// How many revisions to move forward. By default advances to the next + /// child. + #[arg(default_value = "1")] + amount: usize, + /// Instead of creating a new working-copy commit on top of the target commit + /// (like `jj new`), edit the target commit directly (like `jj edit`). + #[arg(long)] + edit: bool, +} + +/// Move the working copy commit to the parent of the current revision. +/// +/// +/// The command moves you to the parent in a linear fashion. +/// +/// +/// F @ F +/// |/ | +/// A => A @ +/// | | / +/// B B +/// +/// +/// If `edit` is passed as an argument, it will move the working copy commit +/// directly to the parent. +/// +/// +/// F @ F +/// |/ | +/// C => C +/// | | +/// B @ +/// | | +/// A A +// TODO(#NNN): Handle multiple parents, e.g merges. +#[derive(clap::Args, Clone, Debug)] +#[command(verbatim_doc_comment)] +struct PrevArgs { + /// How many revisions to move backward. By default moves to the parent. + #[arg(default_value = "1")] + amount: usize, + /// Edit the parent directly, instead of moving the working-copy commit. + #[arg(long)] + edit: bool, +} + /// Move changes from one revision into another /// /// Use `--interactive` to move only part of the source revision into the @@ -2298,6 +2375,292 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C Ok(()) } +fn cmd_next(ui: &mut Ui, command: &CommandHelper, args: &NextArgs) -> Result<(), CommandError> { + let mut workspace_command = command.workspace_helper(ui)?; + let edit = args.edit; + let amount = args.amount; + assert!(amount >= 1); + let current_wc_id = workspace_command + .get_wc_commit_id() + .ok_or_else(|| user_error("This command requires a working copy"))?; + let current_wc = workspace_command.repo().store().get_commit(current_wc_id)?; + // TODO(#NNN): This currently depends on order in which the parents are + // returned, make it configurable. + let parents = current_wc.parents(); + let parent = parents + .first() + .ok_or_else(|| user_error("unable to determine parent"))?; + let parent_commit = RevsetExpression::commit(parent.id().clone()); + // Collect all descendants of our parent. + // TODO(#NNN) We currently cannot deal with multiple children, which result + // from branches. Fix it when --interactive is implemented. + let mut children_revset = RevsetExpression::children(&parent_commit); + // let children: Vec = RevsetExpression::descendants(&parent_commit) + // .resolve(workspace_command.repo().as_ref())? + // .evaluate(workspace_command.repo().as_ref())? + // .iter() + // .commits(workspace_command.repo().store()) + // .try_collect()?; + + let current_id = current_wc.id(); + let current_short = short_commit_hash(current_id); + // Handle the simple `jj next` call. + if amount == 1 { + let children: Vec = children_revset + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + // TODO(#NNN): Immediate descendant is a branch. + if children.len() > 1 { + return Err(user_error( + r#"next cannot resolve the immediate child as there are multiple candidates + see www.github.com/martinvonz/#NNN for more information"#, + )); + } + // If the parent is the last commit in the repository and we're not editing, + // just move the working-copy commit to the end. This is just a wrapped `jj + // new`. + if children.is_empty() { + let mut tx = workspace_command + .start_transaction(&format!("next: {current_short} -> new commit")); + // As stated above, we can only move the working-copy commit if we're not + // editing. + if edit { + return Err(user_error( + "next cannot edit the next commit at the end of the history".to_string(), + )); + } + let merged_tree = merge_commit_trees(tx.repo(), &[parent.clone()])?; + // Move the working-copy commit. + let new_wc_revision = tx + .mut_repo() + .new_commit( + command.settings(), + vec![parent.id().clone()], + merged_tree.id().clone(), + ) + .write()?; + tx.edit(&new_wc_revision).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + assert!(children.len() == 1, "simple next expects a single child"); + // The target is the first child. + let target = children.first().unwrap(); + workspace_command.check_rewritable(target)?; + let mut tx = workspace_command.start_transaction(""); + let target_id = target.id(); + let target_short = short_commit_hash(target_id); + + // Move to the revision. + if edit { + tx.set_description(&format!("next: {current_short} -> editing {target_short}")); + tx.edit(target).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + tx.set_description(&format!("next: {current_short} -> {target_short}")); + let merged_tree = merge_commit_trees(tx.repo(), &[target.clone()])?; + let new_wc_revision = tx + .mut_repo() + .new_commit( + command.settings(), + vec![target.id().clone()], + merged_tree.id().clone(), + ) + .write()?; + tx.edit(&new_wc_revision).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + assert!(amount > 1, "Expected to descend to further children"); + // Get the N'th child. + for _ in 0..amount { + // Does the revset optimizer turn this into a `descendants(id)`? + children_revset = children_revset.children(); + } + let children: Vec<_> = children_revset + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + // TODO(#NNN): Handle branching children. + if children.len() > 1 { + return Err(user_error(format!( + r#"next failed to resolve to a single commit {amount} generations back, + see www.github.com/martinvonz/#NNN for more information"# + ))); + } + // Why does this crash tests? + // assert!(children.len() == 1, "expected a single child"); + let target = children.first().ok_or_else(|| { + user_error(format!( + "failed to resolve revision as {amount} is larger then remaining repository history" + )) + })?; + let target_id = target.id(); + let target_short = short_commit_hash(target_id); + let mut tx = workspace_command.start_transaction(""); + + // Move to the target. + if edit { + tx.set_description(&format!("next: {current_short} -> editing {target_short}")); + tx.edit(target).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + tx.set_description(&format!("next: {current_short} -> {target_short}")); + // Make the target the parent of the new working-copy commit. + let merged_tree = merge_commit_trees(tx.repo(), &[target.clone()])?; + let new_wc_revision = tx + .mut_repo() + .new_commit( + command.settings(), + vec![target.id().clone()], + merged_tree.id().clone(), + ) + .write()?; + tx.edit(&new_wc_revision).unwrap(); + tx.finish(ui)?; + Ok(()) +} + +fn cmd_prev(ui: &mut Ui, command: &CommandHelper, args: &PrevArgs) -> Result<(), CommandError> { + let mut workspace_command = command.workspace_helper(ui)?; + let edit = args.edit; + let amount = args.amount; + assert!(amount >= 1); + let current_wc_id = workspace_command + .get_wc_commit_id() + .ok_or_else(|| user_error("This command requires a working copy".to_string()))?; + let current_wc = workspace_command.repo().store().get_commit(current_wc_id)?; + let current_id = current_wc.id(); + let current_short = short_commit_hash(current_id); + let mut parent_revset = RevsetExpression::commit(current_wc_id.clone()).parents(); + // Collect all ancestors up until the root commit. + // let all_ancestors: Vec = parents + // .ancestors() + // .resolve(workspace_command.repo().as_ref())? + // .evaluate(workspace_command.repo().as_ref())? + // .iter() + // .commits(workspace_command.repo().store()) + // .try_collect()?; + // Handle the simple case of a basic `prev` call. + if amount == 1 { + let possible_parents: Vec<_> = parent_revset + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + // TODO(#NNN): Handle multiple parents correctly, e.g prompt if we're + // interactive. + if possible_parents.len() > 1 { + return Err(user_error( + r#"prev failed to resolve direct parent, see + www.github/martinvonz/jj/#NNN for more informatin."#, + )); + } + // The direct parent is the first ancestor. + let parent = possible_parents + .first() + .ok_or_else(|| user_error("failed to determine parent"))?; + workspace_command.check_rewritable(parent)?; + let mut tx = workspace_command.start_transaction(""); + let parent_id = parent.id(); + let parent_short = short_commit_hash(parent_id); + // Omit the "moved N commits" from the message. + let root_commit = tx.base_repo().store().root_commit(); + // If we're editing, just move to the revision directly. + if edit { + if parent_id == root_commit.id() { + return Err(user_error("Editing the root commit is not allowed.")); + } + tx.set_description(&format!("prev: {current_short} -> editing {parent_short}")); + tx.edit(parent).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + tx.set_description(&format!("prev: {current_short} -> {parent_short}",)); + let merged_tree = merge_commit_trees(tx.repo(), &[parent.clone()])?; + // Make the workspace commit a descendant of the parent. + let new_wc_revision = tx + .mut_repo() + .new_commit( + command.settings(), + vec![parent_id.clone()], + merged_tree.id().clone(), + ) + .write()?; + tx.edit(&new_wc_revision).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + // Why does this crash tests? + // assert!(amount > 1, "Expected more parents to traverse"); + for _ in 0..amount { + // Does the revset optimizer turn this into `ancestors(id)`? + parent_revset = parent_revset.parents(); + } + let possible_parents: Vec<_> = parent_revset + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + + if possible_parents.is_empty() { + return Err(user_error(format!(""))); + } + + // TODO(#NNN): Handle a merge commit N generations back correctly. + if possible_parents.len() > 1 { + return Err(user_error(format!( + r#"commit resolved to multiple revisions {amount} steps back + see www.github.com/martinvonz/jj/#NNN for more information + "#))); + } + let target = possible_parents.first().ok_or_else( + || user_error("failed to resolve parent"))?; + let target_id = target.id(); + let target_short = short_commit_hash(target_id); + let mut tx = workspace_command.start_transaction(""); + let root_commit = tx.base_repo().store().root_commit(); + // You still cannot edit the root commit. + if *target == root_commit { + return Err(user_error("unable to edit the root commit")); + } + // We're editing, just move to the commit. + if edit { + tx.set_description(&format!( + "prev: moved {amount} commits {current_short} -> editing {target_short}" + )); + tx.edit(target).unwrap(); + tx.finish(ui)?; + return Ok(()); + } + tx.set_description(&format!( + "prev: moved {amount} commits {current_short} -> {target_short}" + )); + // Create a child revision for our new working-copy commit. + let merged_tree = merge_commit_trees(tx.repo(), &[target.clone()])?; + // Make the working-copy commit a descendant of the target. + let new_wc_revision = tx + .mut_repo() + .new_commit( + command.settings(), + vec![target_id.clone()], + merged_tree.id().clone(), + ) + .write()?; + tx.edit(&new_wc_revision).unwrap(); + tx.finish(ui)?; + Ok(()) +} + fn combine_messages( repo: &ReadonlyRepo, source: &Commit, @@ -3651,6 +4014,8 @@ pub fn run_command(ui: &mut Ui, command_helper: &CommandHelper) -> Result<(), Co Commands::Duplicate(sub_args) => cmd_duplicate(ui, command_helper, sub_args), Commands::Abandon(sub_args) => cmd_abandon(ui, command_helper, sub_args), Commands::Edit(sub_args) => cmd_edit(ui, command_helper, sub_args), + Commands::Next(sub_args) => cmd_next(ui, command_helper, sub_args), + Commands::Prev(sub_args) => cmd_prev(ui, command_helper, sub_args), Commands::New(sub_args) => cmd_new(ui, command_helper, sub_args), Commands::Move(sub_args) => cmd_move(ui, command_helper, sub_args), Commands::Squash(sub_args) => cmd_squash(ui, command_helper, sub_args), diff --git a/tests/test_next_prev_commands.rs b/tests/test_next_prev_commands.rs new file mode 100644 index 0000000000..acce448b73 --- /dev/null +++ b/tests/test_next_prev_commands.rs @@ -0,0 +1,194 @@ +// Copyright 2023 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +use crate::common::TestEnvironment; + +pub mod common; + +#[test] +fn test_next_simple() { + // Move from first => second. + // first + // | + // second + // | + // third + // + let 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"); + // Create a simple linear history, which we'll traverse. + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + // Move to `first` + test_env.jj_cmd_success(&repo_path, &["edit", "-r", "@--"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["next"]); + insta::assert_snapshot!( + stdout, + @r###" + Working copy now at: 5647d685026f (no description set) + Parent commit : 5c52832c3483 second + "### + ); +} + +#[test] +fn test_next_multiple() { + // Move from first => fourth. + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "fourth"]); + test_env.jj_cmd_success(&repo_path, &["edit", "@--"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["next", "2"]); + // We should now be the child of the fourth commit. + insta::assert_snapshot!( + stdout, + @r###""" + """### + ); +} + +#[test] +fn test_prev_simple() { + // Move from third => second. + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["prev"]); + // The working copy commit is now a child of "second". + insta::assert_snapshot!( + stdout, + @r###" + Working copy now at: 973e9c36d3e4 (no description set) + Parent commit : 3fa8931e7b89 third + "### + ); +} + +#[test] +fn test_prev_multiple_without_root() { + // Move from fourth => second. + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "fourth"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["prev", "2"]); + insta::assert_snapshot!( + stdout, + @r###" + Working copy now at: 5647d685026f (no description set) + Parent commit : 5c52832c3483 second + "### + ); +} + +#[test] +fn test_next_fails_on_branching_children() { + // TODO(#NNN): Fix this behavior + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + // Create a branching child. + test_env.jj_cmd_success(&repo_path, &["branch", "c", "into-the-future"]); + test_env.jj_cmd_success(&repo_path, &["co", "into-the-future"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "42"]); + test_env.jj_cmd_success(&repo_path, &["co", "main"]); + // Try to advance the working copy commit. + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["next"]); + insta::assert_snapshot!(stderr,@r#""#); +} + +#[test] +fn test_prev_fails_on_multiple_parents() { + // TODO(#NNN): Fix this behavior + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + + // Try to access a parent commit. + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["prev"]); + insta::assert_snapshot!(stderr,@r#""#); +} + +#[test] +fn test_prev_onto_root_fails() { + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "fourth"]); + // The root commit is before "first". + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["prev", "4"]); + insta::assert_snapshot!(stderr,@r#""#); +} + +#[test] +fn test_prev_editing() { + // Edit the third commit. + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "fourth"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["prev", "--edit"]); + insta::assert_snapshot!( + stdout, + @r###" + Working copy now at: 009f88bf7141 fourth + Parent commit : 3fa8931e7b89 third + "### + ); +} + +#[test] +fn test_next_editing() { + // Edit the second commit. + let 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"); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "fourth"]); + test_env.jj_cmd_success(&repo_path, &["edit", "@--"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["next", "--edit"]); + insta::assert_snapshot!( + stdout, + @r###" + Nothing changed. + "### + ); +}