Skip to content

Commit

Permalink
commands: Teach next to create a new commit after an edit. WIP: C…
Browse files Browse the repository at this point in the history
…urrently not building

This makes the commands symmetrical again.  

Fixes jj-vcs#2209
  • Loading branch information
PhilipMetzger committed Jan 1, 2024
1 parent c9b5815 commit b7852f2
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 3 deletions.
36 changes: 33 additions & 3 deletions cli/src/commands/next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use itertools::Itertools;
use jj_lib::commit::Commit;
use jj_lib::repo::Repo;
use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::rewrite::merge_commit_trees;

use crate::cli_util::{short_commit_hash, user_error, CommandError, CommandHelper};
use crate::ui::Ui;
Expand Down Expand Up @@ -87,23 +88,47 @@ pub(crate) fn cmd_next(
let target_expression = if edit {
descendant_expression
} else {
descendant_expression.minus(&RevsetExpression::commit(current_wc_id.clone()).descendants())
// Jujutsu will always create a new commit for next, 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 back `.descendants()` to the revset below.
descendant_expression.minus(&RevsetExpression::commit(current_wc_id.clone()))
};
let targets: Vec<Commit> = target_expression
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
.take(2)
.try_collect()?;
// Do we create a commit to allow `next` onto new?
let mut created_commit = false;

let mut tx = workspace_command.start_transaction();
let target = match targets.as_slice() {
[target] => target,
[] => {
[] if edit => {
// We found no descendant.
return Err(user_error(format!(
"No descendant found {amount} commit{} forward",
if amount > 1 { "s" } else { "" }
)));
}
[] => {
// Create a new commit to get out of the forced `edit` flow.
let merged_tree = merge_commit_trees(tx.repo(), &[current_wc.clone()])?;
let new_commit = tx
.mut_repo()
.new_commit(
command.settings(),
vec![current_wc_id.clone()],
merged_tree.id(),
)
.write()
.unwrap();
created_commit = true;
&new_commit
}
_ => {
// TODO(#2126) We currently cannot deal with multiple children, which result
// from branches. Prompt the user for resolution.
Expand All @@ -124,8 +149,13 @@ pub(crate) fn cmd_next(
return Ok(());
}
let mut tx = workspace_command.start_transaction();
let message = if !created_commit {
format!("next: {current_short} -> {target_short}")
} else {
format!("next: {current_short} -> new commit")
};
// Move the working-copy commit to the new parent.
tx.check_out(target)?;
tx.finish(ui, format!("next: {current_short} -> {target_short}"))?;
tx.finish(ui, &message)?;
Ok(())
}
23 changes: 23 additions & 0 deletions cli/tests/test_next_prev_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,26 @@ fn test_next_editing() {
Parent commit : kkmpptxz 3fa8931e (empty) third
"###);
}

#[test]
fn test_next_after_edit_new_commit() {
// Move from first => second.
// While creating a new 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");
// 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", "@--"]);
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
"###
);
}

0 comments on commit b7852f2

Please sign in to comment.