From 5a66afc5db7699ef0378ff269c58cd596561d9e4 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 gave each command separate a separate argument struct for extensibility. Fixes #878 --- CHANGELOG.md | 5 + cli/src/commands/mod.rs | 210 ++++++++++++++++++++++++ cli/tests/test_alias.rs | 2 + cli/tests/test_next_prev_commands.rs | 228 +++++++++++++++++++++++++++ 4 files changed, 445 insertions(+) create mode 100644 cli/tests/test_next_prev_commands.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 21fe76f285..4195c22244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * 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 + further pending improvements. + ### Fixed bugs * Fix issues related to .gitignore handling of untracked directories diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index fe1e5ebf13..d9fc603886 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -108,10 +108,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), @@ -545,6 +547,82 @@ 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. +/// +/// +/// D D @ +/// | |/ +/// C @ => C +/// |/ | +/// B B +/// +/// +/// If `--edit` is passed, it will move you directly to the child +/// revision. +/// +/// +/// D D +/// | | +/// C C +/// | | +/// B => @ +/// | | +/// @ A +// TODO(#2126): 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: u64, + /// 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. +/// +/// +/// D @ D +/// |/ | +/// A => A @ +/// | | / +/// B B +/// +/// +/// If `--edit` is passed, it will move the working copy commit +/// directly to the parent. +/// +/// +/// D @ D +/// |/ | +/// C => @ +/// | | +/// B B +/// | | +/// A A +// TODO(#2126): 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: u64, + /// 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 @@ -2348,6 +2426,136 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", 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; + 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)?; + let current_short = short_commit_hash(current_wc.id()); + // If we're editing, start at the working-copy commit. + // Otherwise start from our direct parent. + let start_id = if edit { + current_wc_id + } else { + match current_wc.parent_ids() { + [parent_id] => parent_id, + _ => return Err(user_error("Cannot run `jj next` on a merge commit")), + } + }; + let descendant_expression = RevsetExpression::commit(start_id.clone()).descendants_at(amount); + let target_expression = if edit { + descendant_expression + } else { + descendant_expression.minus(&RevsetExpression::commit(current_wc_id.clone()).descendants()) + }; + let targets: Vec = target_expression + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .take(2) + .try_collect()?; + let target = match targets.as_slice() { + [target] => target, + [] => { + // We found no descendant. + return Err(user_error(format!( + "No descendant found {amount} commit{} forward", + if amount > 1 { "s" } else { "" } + ))); + } + _ => { + // TODO(#2126) We currently cannot deal with multiple children, which result + // from branches. Prompt the user for resolution. + return Err(user_error("Ambiguous target commit")); + } + }; + let target_short = short_commit_hash(target.id()); + // We're editing, just move to the target commit. + if edit { + // We're editing, the target must be rewritable. + workspace_command.check_rewritable(target)?; + let mut tx = workspace_command + .start_transaction(&format!("next: {current_short} -> editing {target_short}")); + tx.edit(target)?; + tx.finish(ui)?; + return Ok(()); + } + let mut tx = + workspace_command.start_transaction(&format!("next: {current_short} -> {target_short}")); + // Move the working-copy commit to the new parent. + tx.check_out(target)?; + 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; + 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)?; + let current_short = short_commit_hash(current_wc.id()); + let start_id = if edit { + current_wc_id + } else { + match current_wc.parent_ids() { + [parent_id] => parent_id, + _ => return Err(user_error("Cannot run `jj prev` on a merge commit")), + } + }; + let ancestor_expression = RevsetExpression::commit(start_id.clone()).ancestors_at(amount); + let target_revset = if edit { + ancestor_expression + } else { + // Jujutsu will always create a new commit for prev, even where Mercurial cannot and fails. + // The decision and all discussion around it are available here: + // https://github.com/martinvonz/jj/pull/1200#discussion_r1298623933 + // + // If users ever request erroring out, add `.ancestors()` to the revset below. + ancestor_expression.minus(&RevsetExpression::commit(current_wc_id.clone())) + }; + let targets: Vec<_> = target_revset + .resolve(workspace_command.repo().as_ref())? + .evaluate(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .take(2) + .try_collect()?; + let target = match targets.as_slice() { + [target] => target, + [] => { + return Err(user_error(format!( + "No ancestor found {amount} commit{} back", + if amount > 1 { "s" } else { "" } + ))) + } + _ => return Err(user_error("Ambiguous target commit")), + }; + // Generate a short commit hash, to make it readable in the op log. + let target_short = short_commit_hash(target.id()); + // If we're editing, just move to the revision directly. + if edit { + // The target must be rewritable if we're editing. + workspace_command.check_rewritable(target)?; + let mut tx = workspace_command + .start_transaction(&format!("prev: {current_short} -> editing {target_short}")); + tx.edit(target)?; + tx.finish(ui)?; + return Ok(()); + } + let mut tx = + workspace_command.start_transaction(&format!("prev: {current_short} -> {target_short}")); + tx.check_out(target)?; + tx.finish(ui)?; + Ok(()) +} + fn combine_messages( repo: &ReadonlyRepo, source: &Commit, @@ -3731,6 +3939,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/cli/tests/test_alias.rs b/cli/tests/test_alias.rs index 4288c26b79..7c2f2d8b7b 100644 --- a/cli/tests/test_alias.rs +++ b/cli/tests/test_alias.rs @@ -86,6 +86,8 @@ fn test_alias_calls_unknown_command() { insta::assert_snapshot!(stderr, @r###" error: unrecognized subcommand 'nonexistent' + tip: a similar subcommand exists: 'next' + Usage: jj [OPTIONS] [COMMAND] For more information, try '--help'. diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs new file mode 100644 index 0000000000..64910b59a2 --- /dev/null +++ b/cli/tests/test_next_prev_commands.rs @@ -0,0 +1,228 @@ +// 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, &["new", "@--"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["next"]); + insta::assert_snapshot!( + stdout, + @r###" + Working copy now at: royxmykx f039cf03 (empty) (no description set) + Parent commit : kkmpptxz 3fa8931e (empty) third + "### + ); +} + +#[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, &["new", "@---"]); + 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###" + Working copy now at: yqosqzyt 52a2e8c2 (empty) (no description set) + Parent commit : zsuskuln 009f88bf (empty) fourth + "### + ); +} + +#[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: mzvwutvl f7a0f876 (empty) (no description set) + Parent commit : rlvkpnrz 5c52832c (empty) second + "### + ); +} + +#[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: royxmykx 5647d685 (empty) (no description set) + Parent commit : rlvkpnrz 5c52832c (empty) second + "### + ); +} + +#[test] +fn test_next_exceeding_history() { + // Try to step beyond the current repos history. + 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, &["edit", "-r", "@--"]); + let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "3"]); + // `jj next` beyond existing history fails. + insta::assert_snapshot!(stderr, @r###" + Error: No descendant found 3 commits forward + "###); +} + +#[test] +fn test_next_fails_on_branching_children() { + // TODO(#2126): 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"); + // Create a main branch for this test + test_env.jj_cmd_success(&repo_path, &["branch", "set", "main"]); + 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"]); + // Make the default branch have two possible children. + test_env.jj_cmd_success(&repo_path, &["new", "into-the-future", "@-"]); + // Try to advance the working copy commit. + let stderr = test_env.jj_cmd_failure(&repo_path, &["next"]); + insta::assert_snapshot!(stderr,@r###" + Error: Cannot run `jj next` on a merge commit + "###); +} + +#[test] +fn test_prev_fails_on_multiple_parents() { + // TODO(#2126): 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"]); + test_env.jj_cmd_success(&repo_path, &["branch", "c", "all-about"]); + test_env.jj_cmd_success(&repo_path, &["co", "all-about"]); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "soname"]); + // Create a merge commit, which has two parents. + test_env.jj_cmd_success(&repo_path, &["new", "@-", "@--"]); + // We have more than one parent, prev fails. + let stderr = test_env.jj_cmd_failure(&repo_path, &["prev"]); + insta::assert_snapshot!(stderr,@r###" + Error: Cannot run `jj prev` on a merge commit + "###); +} + +#[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_failure(&repo_path, &["prev", "6"]); + insta::assert_snapshot!(stderr,@r###" + Error: No ancestor found 6 commits back + "###); +} + +#[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: zsuskuln 009f88bf (empty) fourth + Parent commit : kkmpptxz 3fa8931e (empty) 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###" + Working copy now at: zsuskuln 009f88bf (empty) fourth + Parent commit : kkmpptxz 3fa8931e (empty) third + "### + ); +}