diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c0450f72..c1d4b0e61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,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 b77d294dcf..6a0097c134 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -14,7 +14,7 @@ use crate::cli_util::{short_commit_hash, CommandHelper}; use crate::command_error::{user_error, CommandError}; -use crate::movement_util::{get_target_commit, Direction}; +use crate::movement_util::{get_movement_edit_mode, get_target_commit, Direction}; use crate::ui::Ui; /// Move the working-copy commit to the child revision @@ -73,12 +73,12 @@ pub(crate) fn cmd_next( 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 wc_is_head = workspace_command + .repo() + .view() + .heads() + .contains(current_wc_id); + let edit = get_movement_edit_mode(ui, args.edit, wc_is_head); let target = get_target_commit( ui, &workspace_command, diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index 266478186c..c7c875979c 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -14,7 +14,7 @@ use crate::cli_util::{short_commit_hash, CommandHelper}; use crate::command_error::{user_error, CommandError}; -use crate::movement_util::{get_target_commit, Direction}; +use crate::movement_util::{get_movement_edit_mode, get_target_commit, Direction}; use crate::ui::Ui; /// Change the working copy revision relative to the parent revision /// @@ -69,12 +69,12 @@ pub(crate) fn cmd_prev( 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 wc_is_head = workspace_command + .repo() + .view() + .heads() + .contains(current_wc_id); + let edit = get_movement_edit_mode(ui, args.edit, wc_is_head); let target = get_target_commit( ui, &workspace_command, diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index 5aeda8c3d8..3208393cf5 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 790c5c3a21..d699bfc371 100644 --- a/cli/src/movement_util.rs +++ b/cli/src/movement_util.rs @@ -23,7 +23,7 @@ use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt} use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::{user_error, CommandError}; -use crate::ui::Ui; +use crate::ui::{MovementEditMode, Ui}; pub enum Direction { Next, @@ -98,6 +98,14 @@ impl Direction { } } +pub fn get_movement_edit_mode(ui: &Ui, cli_edit_flag: bool, wc_is_head: bool) -> bool { + match ui.movement_edit_mode() { + MovementEditMode::Always => true, + MovementEditMode::Auto => cli_edit_flag || !wc_is_head, + MovementEditMode::Never => false, + } +} + pub fn get_target_commit( ui: &mut Ui, workspace_command: &WorkspaceCommandHelper, diff --git a/cli/src/ui.rs b/cli/src/ui.rs index c7c8d99a5e..cec162dc0f 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -236,6 +236,27 @@ impl Write for UiStderr<'_> { } } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)] +#[serde(rename_all(deserialize = "kebab-case"))] +pub enum MovementEditMode { + #[default] + Auto, + Always, + Never, +} + +fn movement_settings(config: &config::Config) -> Result { + config + .get::("ui.movement") + .map_err(|err| config_error_with_message("Invalid `ui.movement`", err)) +} + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)] +#[serde(rename_all(deserialize = "kebab-case"))] +pub struct MovementSettings { + edit: MovementEditMode, +} + pub struct Ui { quiet: bool, pager_cmd: CommandNameAndArgs, @@ -243,6 +264,7 @@ pub struct Ui { progress_indicator: bool, formatter_factory: FormatterFactory, output: UiOutput, + movement: MovementSettings, } fn progress_indicator_setting(config: &config::Config) -> bool { @@ -349,6 +371,7 @@ impl Ui { pager_cmd: pager_setting(config)?, paginate: pagination_setting(config)?, progress_indicator, + movement: movement_settings(config)?, output: UiOutput::new_terminal(), }) } @@ -359,6 +382,7 @@ impl Ui { self.pager_cmd = pager_setting(config)?; self.progress_indicator = progress_indicator_setting(config); self.formatter_factory = prepare_formatter_factory(config, &io::stdout())?; + self.movement = movement_settings(config)?; Ok(()) } @@ -397,6 +421,10 @@ impl Ui { } } + pub fn movement_edit_mode(&self) -> MovementEditMode { + self.movement.edit.to_owned() + } + pub fn color(&self) -> bool { self.formatter_factory.is_color() } diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs index 6c68f4ef76..31ea91db86 100644 --- a/cli/tests/test_next_prev_commands.rs +++ b/cli/tests/test_next_prev_commands.rs @@ -869,6 +869,247 @@ 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"]); + 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 + "###); +} + +#[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 + "###); +} + +#[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"]); + 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 + "###); +} + 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])