next/prev: Add config flag to control prev/next edit behaviour.

* We started with a tristate flag where:
    - Auto - Maintain current behaviour. This edits if
      the wc parent is not a head commit. Else, it will
      create a new commit on the parent of the wc in
      the direction of movement.
    - Always - Always edit
    - Never - Never edit, prefer the new+squash workflow.
  However, consensus the review thread is that `auto` mode where we try to infer when to
  switch to `edit mode`, should be removed. So `ui.movement.edit` is a boolean flag now.
    - true: edit mode
    - false: new+squash mode
* Also add a `--no-edit` flag as the explicit inverse of `--edit` and
  ensure both flags take precedence over the config.
* Update tests that assumed edit mode inference, to specify `--edit` explicitly.

NOTE: #4302 was squashed into this commit, so see that closed PR for review history.

Part of #3947

CHANGELOG.md

@@ -10,10 +10,18 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Breaking changes
 
+* `next/prev` will no longer infer when to go into edit mode when moving from
+  commit to commit. It now either follows the flags `--edit|--no-edit` or it
+  gets the mode from `ui.movement.edit`.
+
 ### Deprecations
 
 ### New features
 
+* Add new boolean config knob, `ui.movement.edit` for controlling the behaviour
+  of `prev/next`. The flag turns `edit` mode `on` and `off` permanently when set
+  respectively to `true` or `false`.
+
 * The following diff formats now include information about copies and moves:
   `--color-words`, `--git`, `--stat`, `--summary`, `--types`, and external diff
   tools in file-by-file mode.

cli/src/commands/next.rs

@@ -51,15 +51,24 @@ use crate::ui::Ui;
 #[command(verbatim_doc_comment)]
 pub(crate) struct NextArgs {
     /// How many revisions to move forward. Advances to the next child by
-    /// default.
+    /// default
     #[arg(default_value = "1")]
     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`).
+    /// edit`)
+    ///
+    /// Takes precedence over config in `ui.movement.edit`; i.e.
+    /// will negate `ui.movement.edit = false`
     #[arg(long, short)]
     edit: bool,
-    /// Jump to the next conflicted descendant.
+    /// The inverse of `--edit`
+    ///
+    /// Takes precedence over config in `ui.movement.edit`; i.e.
+    /// will negate `ui.movement.edit = true`
+    #[arg(long, short, conflicts_with = "edit")]
+    no_edit: bool,
+    /// Jump to the next conflicted descendant
     #[arg(long, conflicts_with = "offset")]
     conflict: bool,
 }
@@ -69,6 +78,7 @@ impl From<&NextArgs> for MovementArgs {
         MovementArgs {
             offset: val.offset,
             edit: val.edit,
+            no_edit: val.no_edit,
             conflict: val.conflict,
         }
     }
@@ -79,11 +89,5 @@ pub(crate) fn cmd_next(
     command: &CommandHelper,
     args: &NextArgs,
 ) -> Result<(), CommandError> {
-    let mut workspace_command = command.workspace_helper(ui)?;
-    move_to_commit(
-        ui,
-        &mut workspace_command,
-        Direction::Next,
-        &MovementArgs::from(args),
-    )
+    move_to_commit(ui, command, Direction::Next, &MovementArgs::from(args))
 }

cli/src/commands/prev.rs

@@ -45,17 +45,26 @@ use crate::ui::Ui;
 /// A        A
 /// ```
 /// If the working copy revision already has visible children, then `--edit` is
-/// implied.
+/// implied
 #[derive(clap::Args, Clone, Debug)]
 #[command(verbatim_doc_comment)]
 pub(crate) struct PrevArgs {
-    /// How many revisions to move backward. Moves to the parent by default.
+    /// How many revisions to move backward. Moves to the parent by default
     #[arg(default_value = "1")]
     offset: u64,
-    /// Edit the parent directly, instead of moving the working-copy commit.
+    /// 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 = false`
     #[arg(long, short)]
     edit: bool,
-    /// Jump to the previous conflicted ancestor.
+    /// The inverse of `--edit`
+    ///
+    /// Takes precedence over config in `ui.movement.edit`; i.e.
+    /// will negate `ui.movement.edit = true`
+    #[arg(long, short, conflicts_with = "edit")]
+    no_edit: bool,
+    /// Jump to the previous conflicted ancestor
     #[arg(long, conflicts_with = "offset")]
     conflict: bool,
 }
@@ -65,6 +74,7 @@ impl From<&PrevArgs> for MovementArgs {
         MovementArgs {
             offset: val.offset,
             edit: val.edit,
+            no_edit: val.no_edit,
             conflict: val.conflict,
         }
     }
@@ -75,11 +85,5 @@ pub(crate) fn cmd_prev(
     command: &CommandHelper,
     args: &PrevArgs,
 ) -> Result<(), CommandError> {
-    let mut workspace_command = command.workspace_helper(ui)?;
-    move_to_commit(
-        ui,
-        &mut workspace_command,
-        Direction::Prev,
-        &MovementArgs::from(args),
-    )
+    move_to_commit(ui, command, Direction::Prev, &MovementArgs::from(args))
 }

cli/src/config/misc.toml

@@ -15,5 +15,8 @@ pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
 log-word-wrap = false
 log-synthetic-elided-nodes = true
 
+[ui.movement]
+edit = false
+
 [snapshot]
 max-new-file-size = "1MiB"

cli/src/movement_util.rs

@@ -21,17 +21,25 @@ 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::cli_util::{short_commit_hash, CommandHelper, WorkspaceCommandHelper};
 use crate::command_error::{user_error, CommandError};
 use crate::ui::Ui;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(crate) struct MovementArgs {
     pub offset: u64,
     pub edit: bool,
+    pub no_edit: bool,
     pub conflict: bool,
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+struct MovementArgsInternal {
+    offset: u64,
+    should_edit: bool,
+    conflict: bool,
+}
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub(crate) enum Direction {
     Next,
@@ -56,12 +64,12 @@ impl Direction {
     fn get_target_revset(
         &self,
         working_commit_id: &CommitId,
-        args: &MovementArgs,
+        args: &MovementArgsInternal,
     ) -> Result<Rc<RevsetExpression>, CommandError> {
         let wc_revset = RevsetExpression::commit(working_commit_id.clone());
         // If we're editing, start at the working-copy commit. Otherwise, start from
         // its direct parent(s).
-        let start_revset = if args.edit {
+        let start_revset = if args.should_edit {
             wc_revset.clone()
         } else {
             wc_revset.parents()
@@ -103,7 +111,7 @@ fn get_target_commit(
     workspace_command: &WorkspaceCommandHelper,
     direction: Direction,
     working_commit_id: &CommitId,
-    args: &MovementArgs,
+    args: &MovementArgsInternal,
 ) -> Result<Commit, CommandError> {
     let target_revset = direction.get_target_revset(working_commit_id, args)?;
     let targets: Vec<Commit> = target_revset
@@ -162,29 +170,29 @@ fn choose_commit<'a>(
 
 pub(crate) fn move_to_commit(
     ui: &mut Ui,
-    workspace_command: &mut WorkspaceCommandHelper,
+    command: &CommandHelper,
     direction: Direction,
     args: &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"))?;
 
-    let mut args = args.clone();
-    args.edit = args.edit
-        || !&workspace_command
-            .repo()
-            .view()
-            .heads()
-            .contains(current_wc_id);
+    let config_edit_flag = command.settings().config().get_bool("ui.movement.edit")?;
+    let args = MovementArgsInternal {
+        should_edit: args.edit || (!args.no_edit && config_edit_flag),
+        offset: args.offset,
+        conflict: args.conflict,
+    };
 
-    let target = get_target_commit(ui, workspace_command, direction, current_wc_id, &args)?;
+    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 {
+    if args.should_edit {
         // We're editing, the target must be rewritable.
         workspace_command.check_rewritable([target.id()])?;
         let mut tx = workspace_command.start_transaction();

cli/tests/cli-reference@.md.snap

@@ -1226,6 +1226,11 @@ 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`)
+
+   Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = false`
+* `-n`, `--no-edit` — The inverse of `--edit`
+
+   Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = true`
 * `--conflict` — Jump to the next conflicted descendant
 
 
@@ -1517,7 +1522,7 @@ B        B
 A        A
 ```
 If the working copy revision already has visible children, then `--edit` is
-implied.
+implied
 
 **Usage:** `jj prev [OPTIONS] [OFFSET]`
 
@@ -1530,6 +1535,11 @@ implied.
 ###### **Options:**
 
 * `-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 = false`
+* `-n`, `--no-edit` — The inverse of `--edit`
+
+   Takes precedence over config in `ui.movement.edit`; i.e. will negate `ui.movement.edit = true`
 * `--conflict` — Jump to the previous conflicted ancestor