ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

cli: make move/squash/unsquash keep empty working-copy commit

Browse source 
If the source commit becomes empty as a result of
`move/squash/unsquash`, we abandon it. However, perhaps we shouldn't
do that if the source commit is a working-copy commit because
working-copy commits are often work-in-progress commits.

The background for this change is that @arxanas had just started a new
change and had set a description on it, and then decided to make some
changes in the working copy that should be in the parent
commit. Running `jj squash` then abandoned the working-copy commit,
resuling in the description getting lost.
This commit is contained in:
Martin von Zweigbergk 2022-05-29 17:18:56 -07:00 committed by Martin von Zweigbergk
parent 3b47b00ed0
commit 8ae9540f2c
6 changed files with 35 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -142,6 +142,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* When checking out a commit, the previous commit is no longer abandoned if it
has a non-empty description.
* When using `jj move/squash/unsquash` results in the source commit becoming
empty, it is no longer abandoned if it is checked out (in any workspace).
## [0.4.0] - 2022-04-02
### Breaking changes

6
lib/src/view.rs
View file

@ -14,6 +14,8 @@
use std::collections::{BTreeMap, HashMap, HashSet};
use itertools::Itertools;
use crate::backend::CommitId;
use crate::index::IndexRef;
use crate::op_store;
@ -48,6 +50,10 @@ impl View {
self.data.checkouts.get(workspace_id)
}
pub fn is_checkout(&self, commit_id: &CommitId) -> bool {
self.data.checkouts.values().contains(commit_id)
}
pub fn heads(&self) -> &HashSet<CommitId> {
&self.data.head_ids
}

22
src/commands.rs
View file

@ -1400,7 +1400,8 @@ struct NewArgs {
/// destination. The selected changes (or all the changes in the source revision
/// if not using `--interactive`) will be moved into the destination. The
/// changes will be removed from the source. If that means that the source is
/// now empty compared to its parent, it will be abandoned.
/// now empty compared to its parent, it will be abandoned unless it's checked
/// out. Without `--interactive`, the source change will always be empty.
#[derive(clap::Args, Clone, Debug)]
#[clap(group(ArgGroup::new("to_move").args(&["from", "to"]).multiple(true).required(true)))]
struct MoveArgs {
@ -1422,8 +1423,8 @@ struct MoveArgs {
///
/// After moving the changes into the parent, the child revision will have the
/// same content state as before. If that means that the change is now empty
/// compared to its parent, it will be abandoned. This will always be the case
/// without `--interactive`.
/// compared to its parent, it will be abandoned unless it's checked out.
/// Without `--interactive`, the child change will always be empty.
#[derive(clap::Args, Clone, Debug)]
#[clap(visible_alias = "amend")]
struct SquashArgs {
@ -1438,6 +1439,12 @@ struct SquashArgs {
}
/// Move changes from a revision's parent into the revision
///
/// After moving the changes out of the parent, the child revision will have the
/// same content state as before. If moving the change out of the parent change
/// made it empty compared to its parent, it will be abandoned unless it's
/// checked out. Without `--interactive`, the parent change will always become
/// empty.
#[derive(clap::Args, Clone, Debug)]
#[clap(visible_alias = "unamend")]
struct UnsquashArgs {
@ -1445,7 +1452,7 @@ struct UnsquashArgs {
revision: String,
/// Interactively choose which parts to unsquash
// TODO: It doesn't make much sense to run this without -i. We should make that
// the default. We should also abandon the parent commit if that becomes empty.
// the default.
#[clap(long, short)]
interactive: bool,
}
@ -3423,7 +3430,7 @@ from the source will be moved into the destination.
.get_tree(&RepoPath::root(), &new_parent_tree_id)?;
// Apply the reverse of the selected changes onto the source
let new_source_tree_id = merge_trees(&source_tree, &new_parent_tree, &parent_tree)?;
if new_source_tree_id == *parent_tree.id() {
if new_source_tree_id == *parent_tree.id() && !repo.view().is_checkout(source.id()) {
mut_repo.record_abandoned_commit(source.id().clone());
} else {
CommitBuilder::for_rewrite_from(ui.settings(), repo.store(), &source)
@ -3495,7 +3502,8 @@ from the source will be moved into the parent.
}
// Abandon the child if the parent now has all the content from the child
// (always the case in the non-interactive case).
let abandon_child = &new_parent_tree_id == commit.tree_id();
let abandon_child =
&new_parent_tree_id == commit.tree_id() && !repo.view().is_checkout(commit.id());
let new_parent = CommitBuilder::for_rewrite_from(ui.settings(), repo.store(), parent)
.set_tree(new_parent_tree_id)
.set_predecessors(vec![parent.id().clone(), commit.id().clone()])
@ -3560,7 +3568,7 @@ aborted.
}
// Abandon the parent if it is now empty (always the case in the non-interactive
// case).
if &new_parent_tree_id == parent_base_tree.id() {
if &new_parent_tree_id == parent_base_tree.id() && !repo.view().is_checkout(parent.id()) {
mut_repo.record_abandoned_commit(parent.id().clone());
// Commit the new child on top of the parent's parents.
CommitBuilder::for_rewrite_from(ui.settings(), repo.store(), &commit)

2
tests/test_edit_command.rs
View file

@ -130,7 +130,7 @@ fn test_edit_merge() {
insta::assert_snapshot!(stdout, @r###"
Created 608f32ad9e19 merge
Rebased 1 descendant commits
Working copy now at: a791bdbda05c (no description set)
Working copy now at: 2eca803962db (no description set)
Added 0 files, modified 0 files, removed 1 files
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "-r", "@-"]);

9
tests/test_squash_command.rs
View file

@ -43,10 +43,11 @@ fn test_squash() {
// Squashes the working copy into the parent by default
let stdout = test_env.jj_cmd_success(&repo_path, &["squash"]);
insta::assert_snapshot!(stdout, @r###"
Working copy now at: 6ca29c9d2e7c (no description set)
Working copy now at: 5f56e6899bce (no description set)
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 6ca29c9d2e7c b c
@ 5f56e6899bce c
o 6ca29c9d2e7c b
o 90aeefd03044 a
o 000000000000
"###);
@ -105,10 +106,10 @@ fn test_squash() {
std::fs::write(repo_path.join("file1"), "e\n").unwrap();
let stdout = test_env.jj_cmd_success(&repo_path, &["squash"]);
insta::assert_snapshot!(stdout, @r###"
Working copy now at: 626d78245205 (no description set)
Working copy now at: b4e9307669d0 (no description set)
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 626d78245205
@ b4e9307669d0
o 2a25465aba5f e
|\
o | 9a18f8da1e69 d

10
tests/test_workspaces.rs
View file

@ -101,13 +101,13 @@ fn test_workspaces_conflicting_edits() {
let stdout = test_env.jj_cmd_success(&main_path, &["squash"]);
insta::assert_snapshot!(stdout, @r###"
Rebased 1 descendant commits
Working copy now at: 86bef7fee095 (no description set)
Working copy now at: 6d004761e813 (no description set)
"###);
// The secondary workspace's checkout was updated
insta::assert_snapshot!(get_log_output(&test_env, &main_path), @r###"
@ 86bef7fee095bb5626d853c222764fc7c9fb88ac default@
| o 8d8269a323a01a287236c4fd5f64dc9737febb5b secondary@
o 8d8269a323a01a287236c4fd5f64dc9737febb5b secondary@
| @ 6d004761e81306cf8b2168a18868fbc84f182556 default@
|/
o 52601f748bf6cb00ad5389922f530f20a7ecffaa
o 0000000000000000000000000000000000000000
@ -118,8 +118,8 @@ fn test_workspaces_conflicting_edits() {
// have been committed first (causing divergence)
assert!(stdout.starts_with("The working copy is stale"));
insta::assert_snapshot!(stdout.lines().skip(1).join("\n"), @r###"
o 86bef7fee095bb5626d853c222764fc7c9fb88ac default@
| @ 8d8269a323a01a287236c4fd5f64dc9737febb5b secondary@
@ 8d8269a323a01a287236c4fd5f64dc9737febb5b secondary@
| o 6d004761e81306cf8b2168a18868fbc84f182556 default@
|/
o 52601f748bf6cb00ad5389922f530f20a7ecffaa
o 0000000000000000000000000000000000000000