From f942fe60ac498b886654ce5f4b5915f30d97b5ad 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 | 5 + cli/src/commands/mod.rs | 209 ++++++++++++++++++++++++ cli/tests/test_alias.rs | 2 + cli/tests/test_next_prev_commands.rs | 228 +++++++++++++++++++++++++++ 4 files changed, 444 insertions(+) create mode 100644 cli/tests/test_next_prev_commands.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index e1a1f2b487..264fc700e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj split` will now leave the description empty on the second part if the description was empty on the input commit. +* `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 * `jj config set --user` and `jj config edit --user` can now be used outside of any repository. diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index c4dce01bd1..42bf06f912 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -23,6 +23,7 @@ use std::collections::{BTreeMap, HashSet}; use std::fmt::Debug; use std::io::{BufRead, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::sync::Arc; use std::{fs, io}; @@ -110,10 +111,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), @@ -547,6 +550,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 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: 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(#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: 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 @@ -2351,6 +2430,134 @@ 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. + // Everything else starts 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 find_descendant_at = |amount: u64| -> Rc { + RevsetExpression::commit(start_id.clone()).descendants_at(amount) + }; + let target_expression = if edit { + find_descendant_at(amount) + } else { + find_descendant_at(amount).minus(&RevsetExpression::commit(current_wc_id.clone())) + }; + 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(#NNN) 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 branching parent")), + } + }; + let find_ancestor_at = |amount: u64| -> Rc { + RevsetExpression::commit(start_id.clone()).ancestors_at(amount) + }; + let target_revset = if edit { + find_ancestor_at(amount) + } else { + find_ancestor_at(amount).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()) + .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 shot 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, @@ -3743,6 +3950,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 cbd2f21fd1..b824c5a808 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..9be6438c83 --- /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, &["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###" + 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(#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"); + // 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(#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"]); + 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 branching parent + "###); +} + +#[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 + "### + ); +}