Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
commands: Implement next and prev
Browse files Browse the repository at this point in the history
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
PhilipMetzger committed Sep 5, 2023
1 parent 31ad674 commit 5a66afc
Showing 4 changed files with 445 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
210 changes: 210 additions & 0 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -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<Commit> = 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),
2 changes: 2 additions & 0 deletions cli/tests/test_alias.rs
Original file line number Diff line number Diff line change
@@ -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'.
228 changes: 228 additions & 0 deletions cli/tests/test_next_prev_commands.rs
Original file line number Diff line number Diff line change
@@ -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
"###
);
}

0 comments on commit 5a66afc

Please sign in to comment.