Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: Add prev/next --conflict, which allows you to jump to the next conflict. #3248

Merged
merged 2 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
revisions. This means that `jj git push -c xyz -c abc` is now equivalent to
`jj git push -c 'all:(xyz | abc)'`.

* `jj prev` and `jj next` have gained a `--conflict` flag which moves you
to the next conflict in a child commit.

### Fixed bugs

## [0.18.0] - 2024-06-05
Expand Down Expand Up @@ -94,6 +97,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
were global flags and specifying them once would insert the new commit before/
after all the specified commits.


### Deprecations

* Attempting to alias a built-in command now gives a warning, rather than being
Expand Down
5 changes: 2 additions & 3 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1408,9 +1408,8 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
// are millions of commits added to the repo, assuming the revset engine can
// efficiently skip non-conflicting commits. Filter out empty commits mostly so
// `jj new <conflicted commit>` doesn't result in a message about new conflicts.
let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict).intersection(
&RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all())),
);
let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict)
.filtered(RevsetFilterPredicate::File(FilesetExpression::all()));
let removed_conflicts_expr = new_heads.range(&old_heads).intersection(&conflicts);
let added_conflicts_expr = old_heads.range(&new_heads).intersection(&conflicts);

Expand Down
27 changes: 18 additions & 9 deletions cli/src/commands/next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::io::Write;
use itertools::Itertools;
use jj_lib::commit::Commit;
use jj_lib::repo::Repo;
use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};

use crate::cli_util::{short_commit_hash, CommandHelper, WorkspaceCommandHelper};
use crate::command_error::{user_error, CommandError};
Expand Down Expand Up @@ -65,6 +65,9 @@ pub(crate) struct NextArgs {
/// edit`).
#[arg(long, short)]
edit: bool,
/// Jump to the next conflicted descendant.
#[arg(long, conflicts_with = "offset")]
conflict: bool,
fowles marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn choose_commit<'a>(
Expand Down Expand Up @@ -117,21 +120,27 @@ pub(crate) fn cmd_next(
let wc_revset = RevsetExpression::commit(current_wc_id.clone());
// If we're editing, start at the working-copy commit. Otherwise, start from
// its direct parent(s).
let target_revset = if edit {
wc_revset.descendants_at(args.offset)
let start_revset = if edit {
wc_revset.clone()
} else {
wc_revset
.parents()
.descendants_at(args.offset)
// In previous versions we subtracted `wc_revset.descendants()`. That's
// unnecessary now that --edit is implied if `@` has descendants.
.minus(&wc_revset)
wc_revset.parents()
};

let target_revset = if args.conflict {
start_revset
.descendants()
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
.filtered(RevsetFilterPredicate::HasConflict)
.roots()
} else {
start_revset.descendants_at(args.offset).minus(&wc_revset)
};

let targets: Vec<Commit> = target_revset
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;

let target = match targets.as_slice() {
[target] => target,
[] => {
Expand Down
23 changes: 18 additions & 5 deletions cli/src/commands/prev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use itertools::Itertools;
use jj_lib::repo::Repo;
use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};

use crate::cli_util::{short_commit_hash, CommandHelper};
use crate::command_error::{user_error, CommandError};
Expand Down Expand Up @@ -59,6 +59,9 @@ pub(crate) struct PrevArgs {
/// Edit the parent directly, instead of moving the working-copy commit.
#[arg(long, short)]
edit: bool,
/// Jump to the previous conflicted ancestor.
#[arg(long, conflicts_with = "offset")]
conflict: bool,
}

pub(crate) fn cmd_prev(
Expand All @@ -79,11 +82,21 @@ pub(crate) fn cmd_prev(
// If we're editing, start at the working-copy commit. Otherwise, start from
// its direct parent(s).
let target_revset = if edit {
RevsetExpression::commit(current_wc_id.clone()).ancestors_at(args.offset)
} else {
RevsetExpression::commit(current_wc_id.clone())
.parents()
.ancestors_at(args.offset)
} else {
RevsetExpression::commit(current_wc_id.clone()).parents()
};
let target_revset = if args.conflict {
// If people desire to move to the root conflict, replace the `heads()` below
// with `roots(). But let's wait for feedback.
target_revset
.ancestors()
.filtered(RevsetFilterPredicate::HasConflict)
// We need to filter out empty commits to not land on empty working-copies lying around.
.minus(&RevsetExpression::is_empty())
.heads()
} else {
target_revset.ancestors_at(args.offset)
};
let targets: Vec<_> = target_revset
.evaluate_programmatic(workspace_command.repo().as_ref())?
Expand Down
6 changes: 4 additions & 2 deletions cli/src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,10 @@ pub(crate) fn cmd_status(
// Ancestors with conflicts, excluding the current working copy commit.
let ancestors_conflicts = workspace_command
.attach_revset_evaluator(
RevsetExpression::filter(RevsetFilterPredicate::HasConflict)
.intersection(&wc_revset.parents().ancestors())
wc_revset
.parents()
.ancestors()
.filtered(RevsetFilterPredicate::HasConflict)
.minus(&revset_util::parse_immutable_expression(
&workspace_command.revset_parse_context(),
)?),
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/cli-reference@.md.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,7 @@ implied.
###### **Options:**

* `-e`, `--edit` — 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`)
* `--conflict` — Jump to the next conflicted descendant



Expand Down Expand Up @@ -1399,6 +1400,7 @@ implied.
###### **Options:**

* `-e`, `--edit` — Edit the parent directly, instead of moving the working-copy commit
* `--conflict` — Jump to the previous conflicted ancestor



Expand Down
108 changes: 108 additions & 0 deletions cli/tests/test_next_prev_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,114 @@ fn test_next_editing() {
"###);
}

#[test]
fn test_prev_conflict() {
// Make the first commit our new parent.
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let file_path = repo_path.join("content.txt");
std::fs::write(&file_path, "first").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
std::fs::write(&file_path, "second").unwrap();
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
// Create a conflict in the first commit, where we'll jump to.
test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]);
std::fs::write(&file_path, "first+1").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "description(third)"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fourth"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--conflict"]);
// We now should be a child of `fourth`.
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: vruxwmqv b1ea981a (conflict) (empty) (no description set)
Parent commit : rlvkpnrz c26675ba (conflict) second
There are unresolved conflicts at these paths:
content.txt 2-sided conflict
"###);
}

#[test]
fn test_prev_conflict_editing() {
// Edit the third commit.
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let file_path = repo_path.join("content.txt");
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
std::fs::write(&file_path, "second").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
// Create a conflict in the third commit, where we'll jump to.
test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]);
std::fs::write(&file_path, "first text").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "description(third)"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--conflict", "--edit"]);
// We now should be editing the third commit.
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kkmpptxz 26b1439f (conflict) third
Parent commit : rlvkpnrz 55b5d11a (empty) second
There are unresolved conflicts at these paths:
content.txt 2-sided conflict
"###);
}

#[test]
fn test_next_conflict() {
// There is a conflict in the second commit, so after next it should be the new
// parent.
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let file_path = repo_path.join("content.txt");
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
std::fs::write(&file_path, "second").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
// Create a conflict in the second commit.
test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]);
std::fs::write(&file_path, "first").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "description(second)"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
test_env.jj_cmd_ok(&repo_path, &["new", "description(second)"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: vruxwmqv b69eca51 (conflict) (empty) (no description set)
Parent commit : rlvkpnrz fa43d820 (conflict) second
There are unresolved conflicts at these paths:
content.txt 2-sided conflict
"###);
}

#[test]
fn test_next_conflict_editing() {
// There is a conflict in the third commit, so after next it should be our
// working copy.
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let file_path = repo_path.join("content.txt");
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
std::fs::write(&file_path, "second").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
// Create a conflict in the third commit.
std::fs::write(&file_path, "third").unwrap();
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
test_env.jj_cmd_ok(&repo_path, &["edit", "description(second)"]);
std::fs::write(&file_path, "modified second").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "@+"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--conflict", "--edit"]);
// We now should be editing the third commit.
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: royxmykx 08fda952 (conflict) (empty) (no description set)
Parent commit : kkmpptxz 69ff337c (conflict) (no description set)
There are unresolved conflicts at these paths:
content.txt 2-sided conflict
"###);
}

fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
let template = r#"separate(" ", change_id.short(), local_branches, description)"#;
test_env.jj_cmd_success(cwd, &["log", "-T", template])
Expand Down
7 changes: 7 additions & 0 deletions lib/src/revset.rs
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
PhilipMetzger marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,13 @@ impl RevsetExpression {
})
}

/// Filter all commits by `predicate` in `self`.
pub fn filtered(
self: &Rc<RevsetExpression>,
predicate: RevsetFilterPredicate,
) -> Rc<RevsetExpression> {
self.intersection(&RevsetExpression::filter(predicate))
}
/// Commits that are descendants of `self` and ancestors of `heads`, both
/// inclusive.
pub fn dag_range_to(
Expand Down
Loading