Skip to content

Commit

Permalink
next/prev: make --edit implied when already on non-head commit
Browse files Browse the repository at this point in the history
Users who edit non-head commits usually expect `jj next/prev` to
continue to edit the next/previous commit, so let's make that the
default behavior. This should not confuse users who don't edit
non-head commits since they will simply not be in this state. My main
concern is that doing `jj next; jj prev` will now usually take you
back to the previous commit, but not if you started on the parent of a
head commit.
  • Loading branch information
martinvonz committed Feb 12, 2024
1 parent c63d4ff commit ffcc05f
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* In the templating language, Timestamps now have a `.local()` method for
converting to the local timezone.

* `jj next/prev` now infer `--edit` when you're already editing a non-head
commit (a commit with children).

### Fixed bugs

* On Windows, symlinks in the repo are now materialized as regular files in the
Expand Down
10 changes: 9 additions & 1 deletion cli/src/commands/next.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ use crate::ui::Ui;
/// B => @
/// | |
/// @ A
///
/// If your working-copy commit already has visible children, then `--edit` is
/// implied.
/// ```
#[derive(clap::Args, Clone, Debug)]
#[command(verbatim_doc_comment)]
Expand Down Expand Up @@ -100,11 +103,16 @@ pub(crate) fn cmd_next(
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 edit = args.edit
|| !workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let current_wc = workspace_command.repo().store().get_commit(current_wc_id)?;
// If we're editing, start at the working-copy commit.
// Otherwise start from our direct parent.
Expand Down
12 changes: 10 additions & 2 deletions cli/src/commands/prev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::ui::Ui;
/// D @ D
/// |/ |
/// A => A @
/// | | /
/// | |/
/// B B
/// ```
///
Expand All @@ -44,6 +44,9 @@ use crate::ui::Ui;
/// B B
/// | |
/// A A
///
/// If your working-copy commit already has visible children, then `--edit` is
/// implied.
/// ```
// TODO(#2126): Handle multiple parents, e.g merges.
#[derive(clap::Args, Clone, Debug)]
Expand All @@ -63,11 +66,16 @@ pub(crate) fn cmd_prev(
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 edit = args.edit
|| !workspace_command
.repo()
.view()
.heads()
.contains(current_wc_id);
let current_wc = workspace_command.repo().store().get_commit(current_wc_id)?;
let start_id = if edit {
current_wc_id
Expand Down
8 changes: 7 additions & 1 deletion cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,9 @@ C C
B => @
| |
@ A
If your working-copy commit is already a non-head commit (i.e. if it has
visible children), then `--edit` is implied.
```
**Usage:** `jj next [OPTIONS] [AMOUNT]`
Expand Down Expand Up @@ -1330,7 +1333,7 @@ The command moves you to the parent in a linear fashion.
D @ D
|/ |
A => A @
| | /
| |/
B B
```
Expand All @@ -1345,6 +1348,9 @@ C => @
B B
| |
A A
If your working-copy commit is already a non-head commit (i.e. if it has
visible children), then `--edit` is implied.
```
**Usage:** `jj prev [OPTIONS] [AMOUNT]`
Expand Down
16 changes: 15 additions & 1 deletion cli/tests/test_next_prev_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ fn test_prev_editing() {
Working copy now at: zsuskuln 009f88bf (empty) fourth
Parent commit : kkmpptxz 3fa8931e (empty) third
"###);
// --edit is implied when already editing a non-head commit
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: yqosqzyt d2edc95b (empty) (no description set)
Parent commit : rlvkpnrz 5c52832c (empty) second
"###);
}

#[test]
Expand All @@ -296,10 +303,17 @@ fn test_next_editing() {
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fourth"]);
test_env.jj_cmd_ok(&repo_path, &["edit", "@--"]);
test_env.jj_cmd_ok(&repo_path, &["edit", "@---"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--edit"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kkmpptxz 3fa8931e (empty) third
Parent commit : rlvkpnrz 5c52832c (empty) second
"###);
// --edit is implied when already editing a non-head commit
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: zsuskuln 009f88bf (empty) fourth
Parent commit : kkmpptxz 3fa8931e (empty) third
"###);
Expand Down

0 comments on commit ffcc05f

Please sign in to comment.