diff --git a/CHANGELOG.md b/CHANGELOG.md index 347216fa856..d01aad85d55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New features +* Add new config knob, `ui.movement.edit` for controlling the behaviour of `prev/next`. + `auto` maintains existing behaviour while `always` and `never` turn `edit` mode + permanently `on` and `off` respectively. + * Define `immutable_heads()` revset alias in terms of a new `builtin_immutable_heads()`. This enables users to redefine `immutable_heads()` as they wish, but still have `builtin_immutable_heads()` which should not be redefined. diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs index 0a48be34e86..d04effa6ac4 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -56,9 +56,15 @@ pub(crate) struct NextArgs { offset: 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, short)] + /// edit`). Takes precedence over config in `ui.movement.edit`; i.e. + /// will negate `ui.movement.edit = "never"` + #[arg(long, short, conflicts_with = "no_edit")] edit: bool, + /// The inverse of `--edit`. + /// Takes precedence over config in `ui.movement.edit`; i.e. + /// will negate `ui.movement.edit = "always"` + #[arg(long, short, conflicts_with = "edit")] + no_edit: bool, /// Jump to the next conflicted descendant. #[arg(long, conflicts_with = "offset")] conflict: bool, @@ -69,6 +75,7 @@ impl From<&NextArgs> for MovementArgs { MovementArgs { offset: val.offset, edit: val.edit, + no_edit: val.no_edit, conflict: val.conflict, } } @@ -80,10 +87,5 @@ pub(crate) fn cmd_next( args: &NextArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - move_to_commit( - ui, - &mut workspace_command, - &Direction::Next, - &mut MovementArgs::from(args), - ) + move_to_commit(ui, command, &Direction::Next, &mut MovementArgs::from(args)) } diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index fa4088732fb..4ab9638341d 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -53,8 +53,15 @@ pub(crate) struct PrevArgs { #[arg(default_value_t = 1)] offset: u64, /// Edit the parent directly, instead of moving the working-copy commit. + /// Takes precedence over config in `ui.movement.edit`; i.e. + /// will negate `ui.movement.edit = "never"` #[arg(long, short)] edit: bool, + /// The inverse of `--edit`. + /// Takes precedence over config in `ui.movement.edit`; i.e. + /// will negate `ui.movement.edit = "always"` + #[arg(long, short, conflicts_with = "edit")] + no_edit: bool, /// Jump to the previous conflicted ancestor. #[arg(long, conflicts_with = "offset")] conflict: bool, @@ -65,6 +72,7 @@ impl From<&PrevArgs> for MovementArgs { MovementArgs { offset: val.offset, edit: val.edit, + no_edit: val.no_edit, conflict: val.conflict, } } @@ -76,10 +84,5 @@ pub(crate) fn cmd_prev( args: &PrevArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - move_to_commit( - ui, - &mut workspace_command, - &Direction::Prev, - &mut MovementArgs::from(args), - ) + move_to_commit(ui, command, &Direction::Prev, &mut MovementArgs::from(args)) } diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index 5aeda8c3d8d..3208393cf5d 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -18,5 +18,8 @@ pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } } log-word-wrap = false log-synthetic-elided-nodes = true +[ui.movement] +edit = "auto" + [snapshot] max-new-file-size = "1MiB" diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs index 5be3c825bb0..6b820c52d12 100644 --- a/cli/src/movement_util.rs +++ b/cli/src/movement_util.rs @@ -15,14 +15,15 @@ use std::io::Write; use std::rc::Rc; +use config::Config; use itertools::Itertools; use jj_lib::backend::CommitId; use jj_lib::commit::Commit; use jj_lib::repo::Repo; use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; -use crate::cli_util::{short_commit_hash, WorkspaceCommandHelper}; -use crate::command_error::{user_error, CommandError}; +use crate::cli_util::{short_commit_hash, WorkspaceCommandHelper, CommandHelper}; +use crate::command_error::{config_error_with_message, user_error, CommandError}; use crate::ui::Ui; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -162,25 +163,34 @@ fn choose_commit<'a>( pub(crate) fn move_to_commit( ui: &mut Ui, - workspace_command: &mut WorkspaceCommandHelper, + command: &CommandHelper, + config: &config::Config, direction: &Direction, args: &mut MovementArgs, ) -> Result<(), CommandError> { + let mut workspace_command = command.workspace_helper(ui)?; + let current_wc_id = workspace_command .get_wc_commit_id() .ok_or_else(|| user_error("This command requires a working copy"))?; - args.edit = args.edit - || !&workspace_command + + let config_edit_flag = match MovementSettings::try_from(command.settings().config())?.edit_mode() { + MovementEditMode::Always => true, + MovementEditMode::Never => false, + MovementEditMode::Auto => !&workspace_command .repo() .view() .heads() - .contains(current_wc_id); + .contains(current_wc_id), + }; + + let args.edit = args.edit || (!no_edit && config_edit_flag); + let target = get_target_commit(ui, workspace_command, direction, current_wc_id, args)?; let current_short = short_commit_hash(current_wc_id); let target_short = short_commit_hash(target.id()); let cmd = direction.cmd(); - // We're editing, just move to the target commit. if args.edit { // We're editing, the target must be rewritable. @@ -199,3 +209,34 @@ pub(crate) fn move_to_commit( tx.finish(ui, format!("{cmd}: {current_short} -> {target_short}"))?; Ok(()) } + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)] +#[serde(rename_all(deserialize = "kebab-case"))] +pub enum MovementEditMode { + #[default] + Auto, + Always, + Never, +} + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)] +#[serde(rename_all(deserialize = "kebab-case"))] +pub struct MovementSettings { + edit: MovementEditMode, +} + +impl MovementSettings { + fn edit_mode(&self) -> MovementEditMode { + self.edit + } +} + +impl TryFrom<&Config> for MovementSettings { + type Error = CommandError; + + fn try_from(config: &Config) -> Result { + config + .get::("ui.movement") + .map_err(|err| config_error_with_message("Invalid `ui.movement`", err)) + } +} diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index fd81686e87a..ed2f06ce5b5 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1225,7 +1225,8 @@ 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`) +* `-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`). Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "never"` +* `-n`, `--no-edit` — The inverse of `--edit`. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "always"` * `--conflict` — Jump to the next conflicted descendant @@ -1529,7 +1530,8 @@ implied. ###### **Options:** -* `-e`, `--edit` — Edit the parent directly, instead of moving the working-copy commit +* `-e`, `--edit` — Edit the parent directly, instead of moving the working-copy commit. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "never"` +* `-n`, `--no-edit` — The inverse of `--edit`. Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = "always"` * `--conflict` — Jump to the previous conflicted ancestor diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs index bea7c3911c0..0e63eaa545a 100644 --- a/cli/tests/test_next_prev_commands.rs +++ b/cli/tests/test_next_prev_commands.rs @@ -869,6 +869,360 @@ fn test_next_conflict_head() { "###); } +#[test] +fn test_movement_edit_mode_auto() { + let test_env = TestEnvironment::default(); + test_env.add_config(r#"ui.movement.edit = 'auto'"#); + + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ zsuskulnrvyr + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + test_env.jj_cmd_ok(&repo_path, &["prev"]); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ royxmykxtrkr + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: vruxwmqv 087a65b1 (empty) (no description set) + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ vruxwmqvtpmx + │ ○ kkmpptxzrspx third + │ ○ rlvkpnrzqnoo second + ├─╯ + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: znkkpsqq a8419fd6 (empty) (no description set) + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ znkkpsqqskkl + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kmkuslsw 8c4d85ef (empty) (no description set) + Parent commit : kkmpptxz 30056b0c (empty) third + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kmkuslswpqwq + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--edit", "2"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: rlvkpnrz 9ed53a4a (empty) second + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ kkmpptxzrspx third + @ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + // next --no-edit will fail on a linear tree. + let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--no-edit"]); + insta::assert_snapshot!(stderr, @r###" + Error: No descendant found 1 commit(s) forward + "###); + + // to make it pass, first create a new commit + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["new"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: uyznsvlq db8f04ea (empty) (no description set) + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ uyznsvlquzzm + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + // next --no-edit will pass now + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: xtnwkqum e9fdcaec (empty) (no description set) + Parent commit : kkmpptxz 30056b0c (empty) third + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ xtnwkqumpolk + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); +} + +#[test] +fn test_movement_edit_mode_always() { + let test_env = TestEnvironment::default(); + test_env.add_config(r#"ui.movement.edit = 'always'"#); + + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ zsuskulnrvyr + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + test_env.jj_cmd_ok(&repo_path, &["prev"]); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: rlvkpnrz 9ed53a4a (empty) second + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ kkmpptxzrspx third + @ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: qpvuntsm fa15625b (empty) first + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + @ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: rlvkpnrz 9ed53a4a (empty) second + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ kkmpptxzrspx third + @ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kkmpptxz 30056b0c (empty) third + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: nkmrtpmo 67a001a6 (empty) (no description set) + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ nkmrtpmomlro + │ ○ kkmpptxzrspx third + │ ○ rlvkpnrzqnoo second + ├─╯ + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: xznxytkn 22287813 (empty) (no description set) + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ xznxytknoqwo + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); +} + +#[test] +fn test_movement_edit_mode_never() { + let test_env = TestEnvironment::default(); + test_env.add_config(r#"ui.movement.edit = 'never'"#); + + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ zsuskulnrvyr + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + test_env.jj_cmd_ok(&repo_path, &["prev"]); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ royxmykxtrkr + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: vruxwmqv 087a65b1 (empty) (no description set) + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ vruxwmqvtpmx + │ ○ kkmpptxzrspx third + │ ○ rlvkpnrzqnoo second + ├─╯ + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: znkkpsqq a8419fd6 (empty) (no description set) + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ znkkpsqqskkl + │ ○ kkmpptxzrspx third + ├─╯ + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--no-edit"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kmkuslsw 8c4d85ef (empty) (no description set) + Parent commit : kkmpptxz 30056b0c (empty) third + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kmkuslswpqwq + ○ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--edit", "2"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: rlvkpnrz 9ed53a4a (empty) second + Parent commit : qpvuntsm fa15625b (empty) first + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ kkmpptxzrspx third + @ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); + + 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 30056b0c (empty) third + Parent commit : rlvkpnrz 9ed53a4a (empty) second + "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kkmpptxzrspx third + ○ rlvkpnrzqnoo second + ○ qpvuntsmwlqt first + ◆ zzzzzzzzzzzz + "###); +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"separate(" ", change_id.short(), local_branches, if(conflict, "conflict"), description)"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])